From 1cdc8f1356fcde268c9cdb11e304a7bae00f1697 Mon Sep 17 00:00:00 2001 From: elexis Date: Fri, 8 Sep 2017 04:22:53 +0000 Subject: [PATCH] Mark simulation component template and entity properties as read-only in simulation tests (as they are in actual games). Thus throw errors if a simulation test tries to alternate the template instead of silently passing tests with wrong values (as happened tests fixed by ff90bb8490). Remove the two clones from simulation components that were only relevant for that test. Add test to test the test. Differential Revision: https://code.wildfiregames.com/D871 Fixes #4759 setup.js and setup_test.js Reviewed By: leper This was SVN commit r20134. --- .../public/simulation/components/Attack.js | 5 ++- .../simulation/components/DeathDamage.js | 4 +-- .../simulation/components/tests/setup.js | 32 +++++++++++++------ .../simulation/components/tests/setup_test.js | 24 ++++++++++++++ .../components/tests/test_Attack.js | 4 --- .../components/tests/test_DeathDamage.js | 4 ++- .../components/tests/test_GarrisonHolder.js | 2 -- .../components/tests/test_scripts.h | 1 + 8 files changed, 54 insertions(+), 22 deletions(-) create mode 100644 binaries/data/mods/public/simulation/components/tests/setup_test.js diff --git a/binaries/data/mods/public/simulation/components/Attack.js b/binaries/data/mods/public/simulation/components/Attack.js index 50523da650..6e6b2cfc82 100644 --- a/binaries/data/mods/public/simulation/components/Attack.js +++ b/binaries/data/mods/public/simulation/components/Attack.js @@ -440,9 +440,8 @@ Attack.prototype.GetBonusTemplate = function(type) let template = this.template[type]; if (!template) template = this.template[type.split(".")[0]].Splash; - if (template.Bonuses) - return clone(template.Bonuses); - return null; + + return template.Bonuses || null; }; /** diff --git a/binaries/data/mods/public/simulation/components/DeathDamage.js b/binaries/data/mods/public/simulation/components/DeathDamage.js index b145ecc1c2..d1494e8860 100644 --- a/binaries/data/mods/public/simulation/components/DeathDamage.js +++ b/binaries/data/mods/public/simulation/components/DeathDamage.js @@ -57,9 +57,7 @@ DeathDamage.prototype.GetDeathDamageStrengths = function() DeathDamage.prototype.GetBonusTemplate = function() { - if (this.template.Bonuses) - return clone(this.template.Bonuses); - return null; + return this.template.Bonuses || null; }; DeathDamage.prototype.CauseDeathDamage = function() diff --git a/binaries/data/mods/public/simulation/components/tests/setup.js b/binaries/data/mods/public/simulation/components/tests/setup.js index 6497991c3e..c7849b8c44 100644 --- a/binaries/data/mods/public/simulation/components/tests/setup.js +++ b/binaries/data/mods/public/simulation/components/tests/setup.js @@ -8,23 +8,23 @@ var g_Components = {}; Engine.RegisterComponentType = function(iid, name, ctor) { TS_ASSERT(!g_ComponentTypes[name]); - g_ComponentTypes[name] = { iid: iid, ctor: ctor }; + g_ComponentTypes[name] = { "iid": iid, "ctor": ctor }; }; Engine.RegisterSystemComponentType = function(iid, name, ctor) { TS_ASSERT(!g_ComponentTypes[name]); - g_ComponentTypes[name] = { iid: iid, ctor: ctor }; + g_ComponentTypes[name] = { "iid": iid, "ctor": ctor }; }; Engine.RegisterInterface = function(name) { - global["IID_"+name] = g_NewIID++; + global["IID_" + name] = g_NewIID++; }; Engine.RegisterMessageType = function(name) { - global["MT_"+name] = g_NewMTID++; + global["MT_" + name] = g_NewMTID++; }; Engine.QueryInterface = function(ent, iid) @@ -41,9 +41,9 @@ Engine.RegisterGlobal = function(name, value) Engine.DestroyEntity = function(ent) { - for (var cid in g_Components[ent]) + for (let cid in g_Components[ent]) { - var cmp = g_Components[ent][cid]; + let cmp = g_Components[ent][cid]; if (cmp && cmp.Deinit) cmp.Deinit(); } @@ -85,9 +85,23 @@ global.DeleteMock = function(ent, iid) global.ConstructComponent = function(ent, name, template) { - var cmp = new g_ComponentTypes[name].ctor(); - cmp.entity = ent; - cmp.template = template; + let cmp = new g_ComponentTypes[name].ctor(); + + Object.defineProperties(cmp, { + "entity": { + "value": ent, + "configurable": false, + "enumerable": false, + "writable": false + }, + "template": { + "value": template && deepfreeze(clone(template)), + "configurable": false, + "enumerable": false, + "writable": false + } + }); + cmp.Init(); if (!g_Components[ent]) diff --git a/binaries/data/mods/public/simulation/components/tests/setup_test.js b/binaries/data/mods/public/simulation/components/tests/setup_test.js new file mode 100644 index 0000000000..cd90ce7016 --- /dev/null +++ b/binaries/data/mods/public/simulation/components/tests/setup_test.js @@ -0,0 +1,24 @@ +Engine.RegisterInterface("TestSetup"); + +function TestSetup() {}; +TestSetup.prototype.Init = function() {}; + +Engine.RegisterSystemComponentType(IID_TestSetup, "TestSetup", TestSetup); +let cmpTestSetup = ConstructComponent(SYSTEM_ENTITY, "TestSetup", { "property": "value" }); + +function expectException(func) +{ + try { + func(); + Engine.TS_FAIL("Missed exception at " + new Error().stack); + } catch (e) {} +} + +expectException(() => { cmpTestSetup.template = "replacement forbidden"; }); +expectException(() => { cmpTestSetup.template.property = "modification forbidden"; }); +expectException(() => { cmpTestSetup.template.other_property = "insertion forbidden"; }); +expectException(() => { delete cmpTestSetup.entity; }); +expectException(() => { delete cmpTestSetup.template; }); +expectException(() => { delete cmpTestSetup.template.property; }); + +TS_ASSERT_UNEVAL_EQUALS(cmpTestSetup.template, { "property": "value" }); diff --git a/binaries/data/mods/public/simulation/components/tests/test_Attack.js b/binaries/data/mods/public/simulation/components/tests/test_Attack.js index 5e9c82c601..d61979b93f 100644 --- a/binaries/data/mods/public/simulation/components/tests/test_Attack.js +++ b/binaries/data/mods/public/simulation/components/tests/test_Attack.js @@ -167,10 +167,6 @@ for (let className of ["Infantry", "Cavalry"]) attackComponentTest(className, true, (attacker, cmpAttack, defender) => { TS_ASSERT_EQUALS(cmpAttack.GetBonusTemplate("Melee").BonusCav.Multiplier, 2); - // Check that we don't leak data - let bonus = cmpAttack.GetBonusTemplate("Melee"); - bonus.BonusCav.Multiplier = 2.7; - TS_ASSERT_EQUALS(cmpAttack.GetBonusTemplate("Melee").BonusCav.Multiplier, 2); TS_ASSERT(cmpAttack.GetBonusTemplate("Capture") === null); diff --git a/binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js b/binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js index a0edfeff15..05ab7f14cf 100644 --- a/binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js +++ b/binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js @@ -65,8 +65,10 @@ AddMock(deadEnt, IID_Ownership, { TS_ASSERT_UNEVAL_EQUALS(cmpDeathDamage.GetDeathDamageStrengths(), modifiedDamage); cmpDeathDamage.CauseDeathDamage(); +// Test splash damage bonus let splashBonus = { "BonusCav": { "Classes": "Cavalry", "Multiplier": 3 } }; -cmpDeathDamage.template.Bonuses = splashBonus; +template.Bonuses = splashBonus; +cmpDeathDamage = ConstructComponent(deadEnt, "DeathDamage", template); result.splashBonus = splashBonus; TS_ASSERT_UNEVAL_EQUALS(cmpDeathDamage.GetDeathDamageStrengths(), modifiedDamage); cmpDeathDamage.CauseDeathDamage(); diff --git a/binaries/data/mods/public/simulation/components/tests/test_GarrisonHolder.js b/binaries/data/mods/public/simulation/components/tests/test_GarrisonHolder.js index f13e30b421..a2082ee0ad 100644 --- a/binaries/data/mods/public/simulation/components/tests/test_GarrisonHolder.js +++ b/binaries/data/mods/public/simulation/components/tests/test_GarrisonHolder.js @@ -133,8 +133,6 @@ TS_ASSERT_EQUALS(cmpGarrisonHolder.GetCapacity(), 10); TS_ASSERT_EQUALS(cmpGarrisonHolder.GetGarrisonedEntitiesCount(), 0); TS_ASSERT_EQUALS(cmpGarrisonHolder.CanPickup(unitToGarrisonId), false); -cmpGarrisonHolder.template.Pickup = true; -TS_ASSERT_EQUALS(cmpGarrisonHolder.CanPickup(unitToGarrisonId), true); TS_ASSERT_EQUALS(cmpGarrisonHolder.CanPickup(enemyUnitId), false); TS_ASSERT_EQUALS(cmpGarrisonHolder.IsFull(), false); TS_ASSERT_EQUALS(cmpGarrisonHolder.IsAllowedToGarrison(enemyUnitId), false); diff --git a/source/simulation2/components/tests/test_scripts.h b/source/simulation2/components/tests/test_scripts.h index 319a91b832..cb31cc6afb 100644 --- a/source/simulation2/components/tests/test_scripts.h +++ b/source/simulation2/components/tests/test_scripts.h @@ -66,6 +66,7 @@ public: VfsPaths paths; TS_ASSERT_OK(vfs::GetPathnames(g_VFS, L"simulation/components/tests/", L"test_*.js", paths)); + paths.push_back(VfsPath(L"simulation/components/tests/setup_test.js")); for (const VfsPath& path : paths) { CSimContext context;