gptbot/0ad
1
0
Fork 0
forked from mirrors/0ad
Code Pull Requests Activity

Fix issue with D274/e16c4c4800 - modifications are a list of effects, not a single effect

Browse Source 
D274 introduced a silent change that modifications would only have one
effect per value-path, which was an unnoticed regression.

Detected following the comments from SonarQube reported here:
e16c4c4800#inline-4214

Comments By: elexis
Differential Revision: https://code.wildfiregames.com/D2315
This was SVN commit r22967.
This commit is contained in:
wraitii
2019-09-22 12:05:04 +00:00
parent 31a2dd294e
commit db4637f73a
8 changed files with 35 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files
binaries/data/mods/public/simulation/components/Auras.js
+4 -14
View File
@@ -346,12 +346,7 @@ Auras.prototype.ApplyTemplateAura = function(name, players)
let modifName = this.GetModifierIdentifier(name);
for (let player of players)
{
let playerId = cmpPlayerManager.GetPlayerByID(player);
for (let modifierPath in derivedModifiers)
for (let modifier of derivedModifiers[modifierPath])
cmpModifiersManager.AddModifier(modifierPath, modifName, modifier, playerId);
}
cmpModifiersManager.AddModifiers(modifName, derivedModifiers, cmpPlayerManager.GetPlayerByID(player));
};
Auras.prototype.RemoveFormationAura = function(memberList)
@@ -386,8 +381,7 @@ Auras.prototype.RemoveTemplateAura = function(name)
{
let playerId = cmpPlayerManager.GetPlayerByID(player);
for (let modifierPath in derivedModifiers)
for (let modifier of derivedModifiers[modifierPath])
cmpModifiersManager.RemoveModifier(modifierPath, modifName, playerId);
cmpModifiersManager.RemoveModifier(modifierPath, modifName, playerId);
}
};
@@ -425,10 +419,7 @@ Auras.prototype.ApplyAura = function(name, ents)
let modifName = this.GetModifierIdentifier(name);
for (let ent of validEnts)
for (let modifierPath in derivedModifiers)
for (let modifier of derivedModifiers[modifierPath])
cmpModifiersManager.AddModifier(modifierPath, modifName, modifier, ent);
cmpModifiersManager.AddModifiers(modifName, derivedModifiers, ent);
};
Auras.prototype.RemoveAura = function(name, ents, skipModifications = false)
@@ -466,8 +457,7 @@ Auras.prototype.RemoveAura = function(name, ents, skipModifications = false)
let modifName = this.GetModifierIdentifier(name);
for (let ent of ents)
for (let modifierPath in derivedModifiers)
for (let modifier of derivedModifiers[modifierPath])
cmpModifiersManager.RemoveModifier(modifierPath, modifName, ent);
cmpModifiersManager.RemoveModifier(modifierPath, modifName, ent);
};
Auras.prototype.OnOwnershipChanged = function(msg)
binaries/data/mods/public/simulation/components/ModifiersManager.js
+6 -3
View File
@@ -104,7 +104,10 @@ ModifiersManager.prototype.FetchModifiedProperty = function(classesList, propert
let modifs = this.modifiersStorage.GetItems(propertyName, target);
if (!modifs.length)
return originalValue;
return GetTechModifiedProperty(modifs, classesList, originalValue);
// Flatten the list of modifications
let modifications = [];
modifs.forEach(item => { modifications = modifications.concat(item.value); });
return GetTechModifiedProperty(modifications, classesList, originalValue);
};
/**
@@ -238,13 +241,13 @@ ModifiersManager.prototype.OnGlobalOwnershipChanged = function(msg)
let component = propertyName.split("/")[0];
// Only inform if the modifier actually applies to the entity as an optimisation.
// TODO: would it be better to call FetchModifiedProperty here and compare values?
playerModifs[propertyName].forEach(modif => {
playerModifs[propertyName].forEach(item => item.value.forEach(modif => {
if (!DoesModificationApply(modif, classes))
return;
if (!modifiedComponents[component])
modifiedComponents[component] = [];
modifiedComponents[component].push(propertyName);
});
}));
}
for (let component in modifiedComponents)
binaries/data/mods/public/simulation/components/TechnologyManager.js
+1 -4
View File
@@ -220,10 +220,7 @@ TechnologyManager.prototype.ResearchTechnology = function(tech)
if (template.modifications)
{
let cmpModifiersManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_ModifiersManager);
let derivedModifiers = DeriveModificationsFromTech(template);
for (let modifierPath in derivedModifiers)
for (let modifier of derivedModifiers[modifierPath])
cmpModifiersManager.AddModifier(modifierPath, "tech/" + tech, modifier, this.entity);
cmpModifiersManager.AddModifiers("tech/" + tech, DeriveModificationsFromTech(template), this.entity);
}
if (template.replaces && template.replaces.length > 0)
binaries/data/mods/public/simulation/components/tests/test_ModifiersManager.js
+14 -14
View File
@@ -55,15 +55,15 @@ function SerializationCycle()
cmpModifiersManager.OnGlobalPlayerEntityChanged({ "player": PLAYER_ID_FOR_TEST, "from": -1, "to": PLAYER_ENTITY_ID });
cmpModifiersManager.AddModifier("Test_A", "Test_A_0", { "affects": ["Structure"], "add": 10 }, 10, "testLol");
cmpModifiersManager.AddModifier("Test_A", "Test_A_0", [{ "affects": ["Structure"], "add": 10 }], 10, "testLol");
cmpModifiersManager.AddModifier("Test_A", "Test_A_0", { "affects": ["Structure"], "add": 10 }, PLAYER_ENTITY_ID);
cmpModifiersManager.AddModifier("Test_A", "Test_A_1", { "affects": ["Infantry"], "add": 5 }, PLAYER_ENTITY_ID);
cmpModifiersManager.AddModifier("Test_A", "Test_A_2", { "affects": ["Unit"], "add": 3 }, PLAYER_ENTITY_ID);
cmpModifiersManager.AddModifier("Test_A", "Test_A_0", [{ "affects": ["Structure"], "add": 10 }], PLAYER_ENTITY_ID);
cmpModifiersManager.AddModifier("Test_A", "Test_A_1", [{ "affects": ["Infantry"], "add": 5 }], PLAYER_ENTITY_ID);
cmpModifiersManager.AddModifier("Test_A", "Test_A_2", [{ "affects": ["Unit"], "add": 3 }], PLAYER_ENTITY_ID);
TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_A", 5, PLAYER_ENTITY_ID), 5);
cmpModifiersManager.AddModifier("Test_A", "Test_A_Player", { "affects": ["Player"], "add": 3 }, PLAYER_ENTITY_ID);
cmpModifiersManager.AddModifier("Test_A", "Test_A_Player", [{ "affects": ["Player"], "add": 3 }], PLAYER_ENTITY_ID);
TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_A", 5, PLAYER_ENTITY_ID), 8);
TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_A", 5, 5), 15);
@@ -81,23 +81,23 @@ cmpModifiersManager.RemoveAllModifiers("Test_A_0", PLAYER_ENTITY_ID);
TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_A", 5, 5), 5);
cmpModifiersManager.AddModifiers("Test_A_0", {
"Test_A": { "affects": ["Structure"], "add": 10 },
"Test_B": { "affects": ["Structure"], "add": 8 },
"Test_A": [{ "affects": ["Structure"], "add": 10 }],
"Test_B": [{ "affects": ["Structure"], "add": 8 }],
}, PLAYER_ENTITY_ID);
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 }], 5);
cmpModifiersManager.AddModifier("Test_C", "Test_C_1", [{ "affects": ["Unit"], "add": 5 }], 5);
SerializationCycle();
TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_C", 5, 5), 15);
// test that local modifications are indeed applied after global managers
cmpModifiersManager.AddModifier("Test_C", "Test_C_2", { "affects": ["Structure"], "replace": 0 }, 5);
cmpModifiersManager.AddModifier("Test_C", "Test_C_2", [{ "affects": ["Structure"], "replace": 0 }], 5);
TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_C", 5, 5), 0);
TS_ASSERT(!cmpModifiersManager.HasAnyModifier("Test_C_3", PLAYER_ENTITY_ID));
@@ -105,7 +105,7 @@ TS_ASSERT(!cmpModifiersManager.HasAnyModifier("Test_C_3", PLAYER_ENTITY_ID));
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);
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));
@@ -133,9 +133,9 @@ AddMock(PLAYER_ENTITY_ID + 1, IID_Player, {
cmpModifiersManager = ConstructComponent(SYSTEM_ENTITY, "ModifiersManager", {});
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_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.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 });
binaries/data/mods/public/simulation/helpers/Cheat.js
+5 -5
View File
@@ -74,11 +74,11 @@ function Cheat(input)
cmpModifiersManager.RemoveAllModifiers("cheat/fastactions", playerEnt);
else
cmpModifiersManager.AddModifiers("cheat/fastactions", {
"Cost/BuildTime": { "affects": [["Structure"], ["Unit"]], "multiply": 0.01 },
"ResourceGatherer/BaseSpeed": { "affects": [["Structure"], ["Unit"]], "multiply": 1000 },
"Pack/Time": { "affects": [["Structure"], ["Unit"]], "multiply": 0.01 },
"Upgrade/Time": { "affects": [["Structure"], ["Unit"]], "multiply": 0.01 },
"ProductionQueue/TechCostMultiplier/time": { "affects": [["Structure"], ["Unit"]], "multiply": 0.01 }
"Cost/BuildTime": [{ "affects": [["Structure"], ["Unit"]], "multiply": 0.01 }],
"ResourceGatherer/BaseSpeed": [{ "affects": [["Structure"], ["Unit"]], "multiply": 1000 }],
"Pack/Time": [{ "affects": [["Structure"], ["Unit"]], "multiply": 0.01 }],
"Upgrade/Time": [{ "affects": [["Structure"], ["Unit"]], "multiply": 0.01 }],
"ProductionQueue/TechCostMultiplier/time": [{ "affects": [["Structure"], ["Unit"]], "multiply": 0.01 }]
}, playerEnt);
return;
case "changephase":
binaries/data/mods/public/simulation/helpers/InitGame.js
+3 -3
View File
@@ -59,9 +59,9 @@ function InitGame(settings)
cmpPlayer.SetAI(true);
AIDiff = Math.min(AIDiff, rate.length - 1);
cmpModifiersManager.AddModifiers("AI Bonus", {
"ResourceGatherer/BaseSpeed": { "affects": ["Unit", "Structure"], "multiply": rate[AIDiff] },
"Trader/GainMultiplier": { "affects": ["Unit", "Structure"], "multiply": rate[AIDiff] },
"Cost/BuildTime": { "affects": ["Unit", "Structure"], "multiply": time[AIDiff] },
"ResourceGatherer/BaseSpeed": [{ "affects": ["Unit", "Structure"], "multiply": rate[AIDiff] }],
"Trader/GainMultiplier": [{ "affects": ["Unit", "Structure"], "multiply": rate[AIDiff] }],
"Cost/BuildTime": [{ "affects": ["Unit", "Structure"], "multiply": time[AIDiff] }],
}, cmpPlayer.entity);
}
if (settings.PopulationCap)
binaries/data/mods/public/simulation/helpers/MultiKeyMap.js
+1 -1
View File
@@ -183,7 +183,7 @@ MultiKeyMap.prototype._AddItem = function(primaryKey, itemID, item, secondaryKey
it._count++;
return stackable;
}
items.push(Object.assign({ "_ID": itemID, "_count": 1 }, item));
items.push({ "_ID": itemID, "_count": 1, "value": item });
return true;
};
binaries/data/mods/public/simulation/helpers/tests/test_MultiKeyMap.js
+1 -1
View File
@@ -99,7 +99,7 @@ function test_items(map)
TS_ASSERT("prim_c" in items);
let sum = 0;
for (let key in items)
items[key].forEach(item => (sum += item.value * item._count));
items[key].forEach(item => { sum += item.value.value * item._count; });
TS_ASSERT(sum == 22);
}