From 0ad6d36049f015a076b70d48b5e949a6c8958309 Mon Sep 17 00:00:00 2001 From: Atrik Date: Sat, 7 Feb 2026 01:54:18 +0100 Subject: [PATCH] Fix Trainer-ProductionQueue sync OnOwnershipChange Use ProductionQueue.RemoveItem() instead of direct StopBatch() calls to ensure both components stay in sync when entity training lists change due to ownership changes. Fixes #8691 --- .../public/simulation/components/Trainer.js | 26 +++++++++++------ .../components/tests/test_Trainer.js | 28 +++++++++++++++++++ 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/binaries/data/mods/public/simulation/components/Trainer.js b/binaries/data/mods/public/simulation/components/Trainer.js index f85467e948..8664dc4cde 100644 --- a/binaries/data/mods/public/simulation/components/Trainer.js +++ b/binaries/data/mods/public/simulation/components/Trainer.js @@ -499,16 +499,23 @@ Trainer.prototype.CalculateEntitiesMap = function() * - replace the "{civ}" and "{native}" codes with the owner's civ ID and entity's civ ID * - remove disabled entities * - upgrade templates where necessary - * This also updates currently queued production (it's more convenient to do it here). + * This also updates currently queued production. */ + const cmpProductionQueue = Engine.QueryInterface(this.entity, IID_ProductionQueue); const removeAllQueuedTemplate = (token) => { - const queue = clone(this.queue); + if (!cmpProductionQueue) + { + warn("Cannot remove queued template: entity has no production queue component"); + return; + } + const template = this.entitiesMap.get(token); - for (const [id, item] of queue) - if (item.templateName == template) - this.StopBatch(id); + const queue = cmpProductionQueue.GetQueue(); + for (const item of queue) + if (item.unitTemplate === template) + cmpProductionQueue.RemoveItem(item.id); }; // ToDo: Notice this doesn't account for entity limits changing due to the template change. @@ -624,6 +631,10 @@ Trainer.prototype.QueueBatch = function(templateName, count, metadata) /** * @param {number} id - The ID of the batch being trained here we need to stop. + * + * @warning This method should only be called from ProductionQueue to maintain synchronization + * between the queues. For external callers, use ProductionQueue.RemoveItem() instead. + * Direct calls may cause desynchronization between Trainer and ProductionQueue. */ Trainer.prototype.StopBatch = function(id) { @@ -693,9 +704,8 @@ Trainer.prototype.OnValueModification = function(msg) // This also updates the queued production if necessary. this.CalculateEntitiesMap(); - // Inform the GUI that it'll need to recompute the selection panel. - // TODO: it would be better to only send the message if something actually changing - // for the current training queue. + // Mark the selection dirty (even though it didn't change) in order to trigger the GUI to recompute some cached values, which include the list of trainable entities. + // TODO: It would be better to only do this if something actually changed in this.entitiesMap const cmpPlayer = QueryOwnerInterface(this.entity); if (cmpPlayer) Engine.QueryInterface(SYSTEM_ENTITY, IID_GuiInterface).SetSelectionDirty(cmpPlayer.GetPlayerID()); diff --git a/binaries/data/mods/public/simulation/components/tests/test_Trainer.js b/binaries/data/mods/public/simulation/components/tests/test_Trainer.js index a75e18d5a5..c96ced7f83 100644 --- a/binaries/data/mods/public/simulation/components/tests/test_Trainer.js +++ b/binaries/data/mods/public/simulation/components/tests/test_Trainer.js @@ -10,7 +10,9 @@ Engine.LoadComponentScript("interfaces/Foundation.js"); Engine.LoadComponentScript("interfaces/StatisticsTracker.js"); Engine.LoadComponentScript("interfaces/Trainer.js"); Engine.LoadComponentScript("interfaces/TrainingRestrictions.js"); +Engine.LoadComponentScript("interfaces/ProductionQueue.js"); Engine.LoadComponentScript("interfaces/Trigger.js"); +Engine.LoadComponentScript("ProductionQueue.js"); Engine.LoadComponentScript("EntityLimits.js"); Engine.LoadComponentScript("Trainer.js"); Engine.LoadComponentScript("TrainingRestrictions.js"); @@ -34,6 +36,27 @@ let cmpTrainer = ConstructComponent(entityID, "Trainer", { }); cmpTrainer.GetUpgradedTemplate = (template) => template; +// ProductionQueue mock that just delegates to Trainer's queue +AddMock(entityID, IID_ProductionQueue, { + "GetQueue": () => + { + // Convert Trainer's internal queue to ProductionQueue format + const queue = []; + for (const [id, item] of cmpTrainer.queue) + queue.push({ + "id": id, + "unitTemplate": item.templateName, + "batchID": id // In this mock, batchID = ProductionQueue ID + }); + return queue; + }, + "RemoveItem": (id) => + { + // Simply call StopBatch on the Trainer + cmpTrainer.StopBatch(id); + } +}); + AddMock(SYSTEM_ENTITY, IID_PlayerManager, { "GetPlayerByID": id => playerEntityID }); @@ -329,6 +352,11 @@ TS_ASSERT_EQUALS(cmpTrainer.GetBatch(id2).unitTemplate, "units/iber/c"); // Test that we can affect an empty trainer. const emptyTrainer = ConstructComponent(entityID, "Trainer", null); +// Need to add ProductionQueue mock for empty trainer too +AddMock(entityID, IID_ProductionQueue, { + "GetQueue": () => [], + "RemoveItem": () => {} +}); emptyTrainer.OnValueModification({ "component": "Trainer", "entities": [entityID], "valueNames": ["Trainer/Entities/"] }); TS_ASSERT_UNEVAL_EQUALS( emptyTrainer.GetEntitiesList(),