From f5f493681d11d19813ed3b5966ecfeb4714971aa Mon Sep 17 00:00:00 2001 From: wraitii Date: Mon, 7 Jun 2021 18:48:16 +0000 Subject: [PATCH] Fix atlas crash with RM capture the relic. As no default values got set, some game settings became NaN, which triggered exceptions. This sets sane default. This also includes better debugging logic in case of exception, so it's easier to know what happens. Fixes #6200 Reported by: langbart Differential Revision: https://code.wildfiregames.com/D4113 This was SVN commit r25736. --- .../mods/public/simulation/helpers/Setup.js | 26 +++++++++---------- source/scriptinterface/Object.h | 19 ++++++++++++++ .../simulation2/scripting/ScriptComponent.cpp | 17 +++++++++++- .../serialization/SerializedScriptTypes.h | 3 +-- .../simulation2/tests/test_ComponentManager.h | 1 + 5 files changed, 50 insertions(+), 16 deletions(-) diff --git a/binaries/data/mods/public/simulation/helpers/Setup.js b/binaries/data/mods/public/simulation/helpers/Setup.js index e648de8c23..39dc7a838b 100644 --- a/binaries/data/mods/public/simulation/helpers/Setup.js +++ b/binaries/data/mods/public/simulation/helpers/Setup.js @@ -9,28 +9,28 @@ function LoadMapSettings(settings) settings = {}; if (settings.DefaultStance) - for (let ent of Engine.GetEntitiesWithInterface(IID_UnitAI)) + for (const ent of Engine.GetEntitiesWithInterface(IID_UnitAI)) { - let cmpUnitAI = Engine.QueryInterface(ent, IID_UnitAI); + const cmpUnitAI = Engine.QueryInterface(ent, IID_UnitAI); cmpUnitAI.SwitchToStance(settings.DefaultStance); } if (settings.RevealMap) { - let cmpRangeManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_RangeManager); + const cmpRangeManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_RangeManager); if (cmpRangeManager) cmpRangeManager.SetLosRevealAll(-1, true); } if (settings.DisableTreasures) - for (let ent of Engine.GetEntitiesWithInterface(IID_Treasure)) + for (const ent of Engine.GetEntitiesWithInterface(IID_Treasure)) Engine.DestroyEntity(ent); - let cmpRangeManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_RangeManager); + const cmpRangeManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_RangeManager); if (cmpRangeManager) cmpRangeManager.SetLosCircular(!!settings.CircularMap); - let cmpObstructionManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_ObstructionManager); + const cmpObstructionManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_ObstructionManager); if (cmpObstructionManager) cmpObstructionManager.SetPassabilityCircular(!!settings.CircularMap); @@ -47,15 +47,15 @@ function LoadMapSettings(settings) Engine.QueryInterface(SYSTEM_ENTITY, IID_Trigger).SetDifficulty(defaultDiff); } - let cmpEndGameManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_EndGameManager); - let gameSettings = { "victoryConditions": clone(settings.VictoryConditions) }; + const cmpEndGameManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_EndGameManager); + const gameSettings = { "victoryConditions": clone(settings.VictoryConditions) }; if (gameSettings.victoryConditions.indexOf("capture_the_relic") != -1) { - gameSettings.relicCount = settings.RelicCount; - gameSettings.relicDuration = settings.RelicDuration * 60 * 1000; + gameSettings.relicCount = (settings.RelicCount ?? 1); + gameSettings.relicDuration = (settings.RelicDuration ?? 1) * 60 * 1000; } if (gameSettings.victoryConditions.indexOf("wonder") != -1) - gameSettings.wonderDuration = settings.WonderDuration * 60 * 1000; + gameSettings.wonderDuration = (settings.WonderDuration ?? 1) * 60 * 1000; if (gameSettings.victoryConditions.indexOf("regicide") != -1) gameSettings.regicideGarrison = settings.RegicideGarrison; cmpEndGameManager.SetGameSettings(gameSettings); @@ -64,9 +64,9 @@ function LoadMapSettings(settings) if (settings.LockTeams && settings.LastManStanding) warn("Last man standing is only available in games with unlocked teams!"); - let cmpCeasefireManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_CeasefireManager); + const cmpCeasefireManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_CeasefireManager); if (settings.Ceasefire) - cmpCeasefireManager.StartCeasefire(settings.Ceasefire * 60 * 1000); + cmpCeasefireManager.StartCeasefire((settings.Ceasefire ?? 1) * 60 * 1000); } Engine.RegisterGlobal("LoadMapSettings", LoadMapSettings); diff --git a/source/scriptinterface/Object.h b/source/scriptinterface/Object.h index 48115b8655..774db88993 100644 --- a/source/scriptinterface/Object.h +++ b/source/scriptinterface/Object.h @@ -131,6 +131,25 @@ inline bool SetPropertyInt(const ScriptRequest& rq, JS::HandleValue obj, int nam return SetProperty(rq, obj, name, value, constant, enumerable); } +template +inline bool GetObjectClassName(const ScriptRequest& rq, JS::HandleObject obj, T& name) +{ + JS::RootedValue constructor(rq.cx, JS::ObjectOrNullValue(JS_GetConstructor(rq.cx, obj))); + return constructor.isObject() && Script::HasProperty(rq, constructor, "name") && Script::GetProperty(rq, constructor, "name", name); +} + +/** + * Get the name of the object's class. Note that inheritance may lead to unexpected results. + */ +template +inline bool GetObjectClassName(const ScriptRequest& rq, JS::HandleValue val, T& name) +{ + JS::RootedObject obj(rq.cx, val.toObjectOrNull()); + if (!obj) + return false; + return GetObjectClassName(rq, obj, name); +} + inline bool FreezeObject(const ScriptRequest& rq, JS::HandleValue objVal, bool deep) { if (!objVal.isObject()) diff --git a/source/simulation2/scripting/ScriptComponent.cpp b/source/simulation2/scripting/ScriptComponent.cpp index 1ea4fa241d..d0e8f76032 100644 --- a/source/simulation2/scripting/ScriptComponent.cpp +++ b/source/simulation2/scripting/ScriptComponent.cpp @@ -20,6 +20,8 @@ #include "ScriptComponent.h" #include "scriptinterface/FunctionWrapper.h" +#include "scriptinterface/JSON.h" +#include "scriptinterface/ScriptInterface.h" #include "scriptinterface/Object.h" #include "simulation2/serialization/ISerializer.h" #include "simulation2/serialization/IDeserializer.h" @@ -60,7 +62,20 @@ void CComponentTypeScript::Serialize(ISerializer& serialize) { ScriptRequest rq(m_ScriptInterface); - serialize.ScriptVal("comp", &m_Instance); + try + { + serialize.ScriptVal("comp", &m_Instance); + } + catch(PSERROR_Serialize& err) + { + int ent = INVALID_ENTITY; + Script::GetProperty(rq, m_Instance, "entity", ent); + std::string name = "(error)"; + Script::GetObjectClassName(rq, m_Instance, name); + LOGERROR("Script component %s of entity %i failed to serialize: %s\nSerializing:\n%s", name, ent, err.what(), Script::ToString(rq, &m_Instance)); + // Rethrow now that we added more details + throw; + } } void CComponentTypeScript::Deserialize(const CParamNode& paramNode, IDeserializer& deserialize, entity_id_t ent) diff --git a/source/simulation2/serialization/SerializedScriptTypes.h b/source/simulation2/serialization/SerializedScriptTypes.h index d794b2450f..4521bc5433 100644 --- a/source/simulation2/serialization/SerializedScriptTypes.h +++ b/source/simulation2/serialization/SerializedScriptTypes.h @@ -65,8 +65,7 @@ inline SPrototypeSerialization GetPrototypeInfo(const ScriptRequest& rq, JS::Han { SPrototypeSerialization ret; - JS::RootedValue constructor(rq.cx, JS::ObjectOrNullValue(JS_GetConstructor(rq.cx, prototype))); - if (!Script::GetProperty(rq, constructor, "name", ret.name)) + if (!Script::GetObjectClassName(rq, prototype, ret.name)) throw PSERROR_Serialize_ScriptError("Could not get constructor name."); // Nothing to do for basic Object objects. diff --git a/source/simulation2/tests/test_ComponentManager.h b/source/simulation2/tests/test_ComponentManager.h index 7f876bd76c..9041cc1f3f 100644 --- a/source/simulation2/tests/test_ComponentManager.h +++ b/source/simulation2/tests/test_ComponentManager.h @@ -837,6 +837,7 @@ entities:\n\ man.AddComponent(hnd1, man.LookupCID("TestScript1_getter"), noParam); + TestLogger log; std::stringstream stateStream; TS_ASSERT_THROWS_PSERROR(man.SerializeState(stateStream), PSERROR_Serialize_ScriptError, "Cannot serialize property getters"); // (The script will die if the getter is executed)