mirrors/0ad
1
0
Fork 2
mirror of https://gitea.wildfiregames.com/0ad/0ad.git synced 2026-06-21 02:23:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

Reset modifiers properly when changing ownership

Browse Source 
The ModifiersManagers needs to reset caches and recompute modifiers when changing ownership. However, the code only did this for global modifiers for the new player entity, not the old player entity. This meant we would not reset all modifiers, and could OOS (#7996) or lead to incorrect values.

Fixes #7996

Reported by: Itms
This commit is contained in:
Lancelot de Ferrière
2025-06-14 14:18:18 +02:00
committed by wraitii
parent 58219b974c
commit ff754b8bb1
2 changed files with 120 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
binaries/data/mods/public/simulation/components/ModifiersManager.js
+28 -21
View File
@@ -216,37 +216,44 @@ ModifiersManager.prototype.OnGlobalOwnershipChanged = function(msg)
for (const propName of this.cachedValues.keys())
this.InvalidateCache(propName, msg.entity);
const owner = QueryOwnerEntityID(msg.entity);
if (!owner)
return;
const cmpIdentity = Engine.QueryInterface(msg.entity, IID_Identity);
if (!cmpIdentity)
return;
const classes = cmpIdentity.GetClassesList();
const cmpPlayerManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_PlayerManager);
const oldOwner = cmpPlayerManager.GetPlayerByID(msg.from);
const newOwner = cmpPlayerManager.GetPlayerByID(msg.to);
// Warn entities that our values have changed.
// Local modifiers will be added by the relevant components, so no need to check for them here.
const modifiedComponents = {};
const playerModifs = this.modifiersStorage.GetAllItems(owner);
for (const propertyName in playerModifs)
{
// We only need to find one one tech per component for a match.
const 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(item => item.value.forEach(modif => {
if (!DoesModificationApply(modif, classes))
return;
if (!modifiedComponents[component])
modifiedComponents[component] = [];
modifiedComponents[component].push(propertyName);
}));
}
const fetchPlayerModifiedValueNames = (owner) => {
if (!owner)
return;
const playerModifs = this.modifiersStorage.GetAllItems(owner);
for (const propertyName in playerModifs)
{
// We only need to find one one tech per component for a match.
const 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(item => item.value.forEach(modif => {
if (!DoesModificationApply(modif, classes))
return;
if (!modifiedComponents[component])
modifiedComponents[component] = new Set();
modifiedComponents[component].add(propertyName);
}));
}
};
// We'll assume these are always different.
fetchPlayerModifiedValueNames(oldOwner);
fetchPlayerModifiedValueNames(newOwner);
for (const component in modifiedComponents)
Engine.PostMessage(msg.entity, MT_ValueModification, { "entities": [msg.entity], "component": component, "valueNames": modifiedComponents[component] });
Engine.PostMessage(msg.entity, MT_ValueModification, { "entities": [msg.entity], "component": component, "valueNames": Array.from(modifiedComponents[component]) });
};
/**
binaries/data/mods/public/simulation/components/tests/test_ModifiersManager.js
+92
View File
@@ -2,6 +2,7 @@ Engine.LoadComponentScript("interfaces/ModifiersManager.js");
Engine.LoadComponentScript("ModifiersManager.js");
Engine.LoadHelperScript("Player.js");
Engine.LoadHelperScript("ValueModification.js");
Engine.LoadComponentScript("interfaces/Health.js");
let cmpModifiersManager = ConstructComponent(SYSTEM_ENTITY, "ModifiersManager", {});
cmpModifiersManager.Init();
@@ -155,3 +156,94 @@ AddMock(5, IID_Ownership, {
"GetOwner": () => PLAYER_ID_FOR_TEST + 1
});
TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Test_D", 10, 5), 16);
// Test: Entity changes owner from player 2 (HP modifier) to player 3 (Vision modifier)
(function Test_OwnerChange_ModifierSwitch() {
const PLAYER2_ID = 2;
const PLAYER3_ID = 3;
const PLAYER2_ENTITY = 20;
const PLAYER3_ENTITY = 21;
const TEST_ENTITY = 30;
const baseHp = 100;
const baseVision = 20;
// Set up mocks for both players
AddMock(SYSTEM_ENTITY, IID_PlayerManager, {
"GetPlayerByID": (a) => a === PLAYER2_ID ? PLAYER2_ENTITY : PLAYER3_ENTITY
});
AddMock(PLAYER2_ENTITY, IID_Player, {
"GetPlayerID": () => PLAYER2_ID
});
AddMock(PLAYER3_ENTITY, IID_Player, {
"GetPlayerID": () => PLAYER3_ID
});
AddMock(TEST_ENTITY, IID_Ownership, {
"GetOwner": () => PLAYER2_ID
});
AddMock(TEST_ENTITY, IID_Identity, {
"GetClassesList": () => "Unit"
});
// These components cache the values, so we need to mock the message passing.
let cachedHp = baseHp;
AddMock(TEST_ENTITY, IID_Health, {
"GetHitPoints": () => cachedHp,
});
let cachedVision = baseVision;
AddMock(TEST_ENTITY, IID_Vision, {
"GetRange": () => cachedVision
});
const oldPostMessage = Engine.PostMessage;
const oldBroadcastMessage = Engine.BroadcastMessage;
Engine.PostMessage = function(ent, iid, message)
{
if (message.component === "HP")
cachedHp = ApplyValueModificationsToEntity("HP", baseHp, TEST_ENTITY);
else if (message.component === "Vision")
cachedVision = ApplyValueModificationsToEntity("Vision", baseVision, TEST_ENTITY);
else
throw new Error("Unexpected component: " + message.component);
};
Engine.BroadcastMessage = function(iid, message)
{
if (message.component === "HP")
cachedHp = ApplyValueModificationsToEntity("HP", baseHp, TEST_ENTITY);
else if (message.component === "Vision")
cachedVision = ApplyValueModificationsToEntity("Vision", baseVision, TEST_ENTITY);
else
throw new Error("Unexpected component: " + message.component);
};
// Initialize ModifiersManager
const cmp = ConstructComponent(SYSTEM_ENTITY, "ModifiersManager", {});
cmp.Init();
cmp.OnGlobalPlayerEntityChanged({ "player": PLAYER2_ID, "from": INVALID_PLAYER, "to": PLAYER2_ENTITY });
cmp.OnGlobalPlayerEntityChanged({ "player": PLAYER3_ID, "from": INVALID_PLAYER, "to": PLAYER3_ENTITY });
// Player 2 gets HP modifier
cmp.AddModifier("HP", "HP_mod", [{ "affects": ["Unit"], "add": 50 }], PLAYER2_ENTITY);
// Player 3 gets Vision modifier
cmp.AddModifier("Vision", "Vision_mod", [{ "affects": ["Unit"], "add": 10 }], PLAYER3_ENTITY);
// Should have HP modified, not Vision
TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("HP", baseHp, TEST_ENTITY), 150);
TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Vision", baseVision, TEST_ENTITY), 20);
TS_ASSERT_EQUALS(Engine.QueryInterface(TEST_ENTITY, IID_Health).GetHitPoints(), 150);
TS_ASSERT_EQUALS(Engine.QueryInterface(TEST_ENTITY, IID_Vision).GetRange(), 20);
// Change owner to player 3
AddMock(TEST_ENTITY, IID_Ownership, {
"GetOwner": () => PLAYER3_ID
});
cmp.OnGlobalOwnershipChanged({ "entity": TEST_ENTITY, "from": PLAYER2_ID, "to": PLAYER3_ID });
// Now should have Vision modified, not HP
TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("HP", baseHp, TEST_ENTITY), 100);
TS_ASSERT_EQUALS(ApplyValueModificationsToEntity("Vision", baseVision, TEST_ENTITY), 30);
TS_ASSERT_EQUALS(Engine.QueryInterface(TEST_ENTITY, IID_Health).GetHitPoints(), 100);
TS_ASSERT_EQUALS(Engine.QueryInterface(TEST_ENTITY, IID_Vision).GetRange(), 30);
// Cleanup
Engine.PostMessage = oldPostMessage;
Engine.BroadcastMessage = oldBroadcastMessage;
})();