diff --git a/binaries/data/mods/public/simulation/components/ModifiersManager.js b/binaries/data/mods/public/simulation/components/ModifiersManager.js index ec5b64917f..d7e5c9add9 100644 --- a/binaries/data/mods/public/simulation/components/ModifiersManager.js +++ b/binaries/data/mods/public/simulation/components/ModifiersManager.js @@ -110,10 +110,7 @@ ModifiersManager.prototype.FetchModifiedProperty = function(classesList, propert return GetTechModifiedProperty(modifications.flat(), classesList, originalValue); }; -/** - * @returns originalValue after modifiers - */ -ModifiersManager.prototype.Cache = function(classesList, propertyName, originalValue, entity) +ModifiersManager.prototype.Cache = function(classesList, propertyName, originalValue, newValue, entity) { let cache = this.cachedValues.get(propertyName); if (!cache) @@ -123,9 +120,7 @@ ModifiersManager.prototype.Cache = function(classesList, propertyName, originalV if (!cache2) cache2 = cache.set(entity, new Map()).get(entity); - let value = this.FetchModifiedProperty(classesList, propertyName, originalValue, entity); - cache2.set(originalValue, value); - return value; + cache2.set(originalValue, newValue); }; /** @@ -174,7 +169,8 @@ ModifiersManager.prototype.ApplyModifiers = function(propertyName, originalValue pc.add(entity); newValue = this.FetchModifiedProperty(classesList, propertyName, newValue, ownerEntity); } - newValue = this.Cache(classesList, propertyName, newValue, entity); + newValue = this.FetchModifiedProperty(classesList, propertyName, newValue, entity); + this.Cache(classesList, propertyName, originalValue, newValue, entity); return newValue; }; diff --git a/binaries/data/mods/public/simulation/components/tests/test_ModifiersManager.js b/binaries/data/mods/public/simulation/components/tests/test_ModifiersManager.js index 17a5e4e9bd..f84e8d02d0 100644 --- a/binaries/data/mods/public/simulation/components/tests/test_ModifiersManager.js +++ b/binaries/data/mods/public/simulation/components/tests/test_ModifiersManager.js @@ -9,6 +9,7 @@ cmpModifiersManager.Init(); // These should be different as that is the general case. const PLAYER_ID_FOR_TEST = 2; const PLAYER_ENTITY_ID = 3; +const STRUCTURE_ENTITY_ID = 5; AddMock(SYSTEM_ENTITY, IID_RangeManager, { "GetEntitiesByPlayer": () => [], @@ -22,7 +23,7 @@ AddMock(PLAYER_ENTITY_ID, IID_Player, { "GetPlayerID": () => PLAYER_ID_FOR_TEST }); -let entitiesToTest = [5, 6, 7, 8]; +let entitiesToTest = [STRUCTURE_ENTITY_ID, 6, 7, 8]; for (let ent of entitiesToTest) AddMock(ent, IID_Ownership, { "GetOwner": () => PLAYER_ID_FOR_TEST @@ -31,7 +32,7 @@ for (let ent of entitiesToTest) AddMock(PLAYER_ENTITY_ID, IID_Identity, { "GetClassesList": () => "Player", }); -AddMock(5, IID_Identity, { +AddMock(STRUCTURE_ENTITY_ID, IID_Identity, { "GetClassesList": () => "Structure", }); AddMock(6, IID_Identity, { @@ -87,38 +88,47 @@ cmpModifiersManager.AddModifiers("Test_A_0", { TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_A", 5, 5), 15); TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_B", 5, 8), 13); + // Add two local modifications, only the first should stick. -cmpModifiersManager.AddModifier("Test_C", "Test_C_0", [{ "affects": ["Structure"], "add": 10 }], 5); -cmpModifiersManager.AddModifier("Test_C", "Test_C_1", [{ "affects": ["Unit"], "add": 5 }], 5); +cmpModifiersManager.AddModifier("Test_C", "Test_C_0", [{ "affects": ["Structure"], "add": 10 }], STRUCTURE_ENTITY_ID); +cmpModifiersManager.AddModifier("Test_C", "Test_C_invalid", [{ "affects": ["Unit"], "add": 5 }], STRUCTURE_ENTITY_ID); SerializationCycle(); -TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_C", 5, 5), 15); +TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_C", 5, STRUCTURE_ENTITY_ID), 15); // test that local modifications are indeed applied after global managers -cmpModifiersManager.AddModifier("Test_C", "Test_C_2", [{ "affects": ["Structure"], "replace": 0 }], 5); -TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_C", 5, 5), 0); +cmpModifiersManager.AddModifier("Test_C", "Test_C_player", [{ "affects": ["Structure"], "replace": 2 }], PLAYER_ENTITY_ID); -TS_ASSERT(!cmpModifiersManager.HasAnyModifier("Test_C_3", PLAYER_ENTITY_ID)); +TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_C", 5, STRUCTURE_ENTITY_ID), 12); +TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_C", 2, STRUCTURE_ENTITY_ID), 12); SerializationCycle(); -// check that things still work properly if we change global modifications -cmpModifiersManager.AddModifier("Test_C", "Test_C_3", [{ "affects": ["Structure"], "add": 10 }], PLAYER_ENTITY_ID); -TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_C", 5, 5), 0); - -TS_ASSERT(cmpModifiersManager.HasAnyModifier("Test_C_3", PLAYER_ENTITY_ID)); -TS_ASSERT(cmpModifiersManager.HasModifier("Test_C", "Test_C_3", PLAYER_ENTITY_ID)); -TS_ASSERT(cmpModifiersManager.HasModifier("Test_C", "Test_C_2", 5)); - // test removal -cmpModifiersManager.RemoveModifier("Test_C", "Test_C_2", 5); -TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_C", 5, 5), 25); +cmpModifiersManager.RemoveModifier("Test_C", "Test_C_player", PLAYER_ENTITY_ID); +TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_C", 5, STRUCTURE_ENTITY_ID), 15); + +// check that things still work properly if we change global modifications +cmpModifiersManager.AddModifier("Test_C", "Test_C_player", [{ "affects": ["Structure"], "add": 12 }], PLAYER_ENTITY_ID); +TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_C", 5, STRUCTURE_ENTITY_ID), 27); + +TS_ASSERT(cmpModifiersManager.HasAnyModifier("Test_C_player", PLAYER_ENTITY_ID)); +TS_ASSERT(cmpModifiersManager.HasModifier("Test_C", "Test_C_player", PLAYER_ENTITY_ID)); SerializationCycle(); -TS_ASSERT(cmpModifiersManager.HasModifier("Test_C", "Test_C_3", PLAYER_ENTITY_ID)); -TS_ASSERT(!cmpModifiersManager.HasModifier("Test_C", "Test_C_2", 5)); +TS_ASSERT(cmpModifiersManager.HasModifier("Test_C", "Test_C_player", PLAYER_ENTITY_ID)); +TS_ASSERT(!cmpModifiersManager.HasModifier("Test_C", "Test_C_player", STRUCTURE_ENTITY_ID)); + + +// Regression test for a caching issue +cmpModifiersManager.AddModifier("Test_E", "Test_E_player", [{ "affects": ["Structure"], "add": 1 }], PLAYER_ENTITY_ID); +TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_E", 3, STRUCTURE_ENTITY_ID), 4); +cmpModifiersManager.AddModifier("Test_E", "Test_E_1", [{ "affects": ["Structure"], "add": 1 }], STRUCTURE_ENTITY_ID); +TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_E", 4, STRUCTURE_ENTITY_ID), 6); +TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_E", 5, STRUCTURE_ENTITY_ID), 7); + // Test that entities keep local modifications but not global ones when changing owner. AddMock(SYSTEM_ENTITY, IID_PlayerManager, { @@ -134,7 +144,7 @@ cmpModifiersManager.Init(); cmpModifiersManager.AddModifier("Test_D", "Test_D_0", [{ "affects": ["Structure"], "add": 10 }], PLAYER_ENTITY_ID); cmpModifiersManager.AddModifier("Test_D", "Test_D_1", [{ "affects": ["Structure"], "add": 1 }], PLAYER_ENTITY_ID + 1); -cmpModifiersManager.AddModifier("Test_D", "Test_D_2", [{ "affects": ["Structure"], "add": 5 }], 5); +cmpModifiersManager.AddModifier("Test_D", "Test_D_2", [{ "affects": ["Structure"], "add": 5 }], STRUCTURE_ENTITY_ID); cmpModifiersManager.OnGlobalPlayerEntityChanged({ "player": PLAYER_ID_FOR_TEST, "from": -1, "to": PLAYER_ENTITY_ID }); cmpModifiersManager.OnGlobalPlayerEntityChanged({ "player": PLAYER_ID_FOR_TEST + 1, "from": -1, "to": PLAYER_ENTITY_ID + 1 });