Opened 5 years ago

Last modified 5 years ago

#5340 new defect

Replace hardcoded JS in C++ with native JS functions, ScriptInterface Eval calls

Reported by: elexis Owned by:
Priority: Nice to Have Milestone: Backlog
Component: Core engine Keywords:
Cc: Patch: Phab:D1667

Description

Initializing JS objects in C++ is done in about 111 lines using the Eval function of ScriptInterface.

But Eval is bad practice for the following reasons:

  • Semantic and syntax errors can be found at compile time when using designated C++ functions (instead of execution time).
  • A cpp file should contain only cpp code.
  • The hardcoded JS code is supposed to be machine readable, but can't be read by any parser, for example the compiler, the syntax highlighters.
  • The JS syntax or semantic may change, but noone suspects JS code hardcoded all over the place in cpp files.

There are a number of examples that use JS native methods to setup JS Objects and JS Arrays, for example in VisualReplay.cpp.

source/simulation2/Simulation2.cpp:	if (!scriptInterface.Eval("({})", &ais) || !scriptInterface.SetProperty(ais, "AIData", aiData))
source/simulation2/tests/test_Serializer.h:		TSM_ASSERT(msg, script.Eval(input, &obj));
source/simulation2/tests/test_Serializer.h:		TS_ASSERT(script.Eval("([1, 2, function () { }])", &obj));
source/simulation2/tests/test_Serializer.h:		TS_ASSERT(script.Eval(input, &obj));
source/simulation2/serialization/StdDeserializer.cpp:		m_ScriptInterface.Eval("(new Set())", &setVal);
source/simulation2/components/ICmpAIManager.cpp:		self->m_ScriptInterface.Eval("({})", &ai);
source/simulation2/components/tests/test_CommandQueue.h:		TS_ASSERT(test.GetScriptInterface().Eval("var cmds = []; function ProcessCommand(player, cmd) { cmds.push([player, cmd]); }"));
source/simulation2/components/tests/test_CommandQueue.h:		TS_ASSERT(test.GetScriptInterface().Eval("([1,2,3])", &cmd));
source/simulation2/components/tests/test_CommandQueue.h:		TS_ASSERT(test.GetScriptInterface().Eval("({x:4})", &cmd));
source/simulation2/components/tests/test_CommandQueue.h:		TS_ASSERT(test.GetScriptInterface().Eval("({y:5})", &cmd));
source/simulation2/components/tests/test_CommandQueue.h:		TS_ASSERT(test.GetScriptInterface().Eval("uneval(cmds)", output));
source/simulation2/components/CCmpAIManager.cpp:			m_ScriptInterface->Eval(L"({})", &settings);
source/simulation2/components/CCmpAIManager.cpp:		m_ScriptInterface->Eval(L"({})", &settings);
source/simulation2/components/CCmpAIManager.cpp:		m_ScriptInterface->Eval(L"({})", &playersID);
source/simulation2/components/CCmpAIManager.cpp:		m_ScriptInterface->Eval("({})", &m_EntityTemplates);
source/simulation2/components/CCmpAIManager.cpp:		scriptInterface.Eval("({})", &classesVal);
source/gui/GUIManager.cpp:	m_ScriptInterface->Eval("({})", &data);
source/gui/IGUIObject.cpp:	m_pGUI->GetScriptInterface()->Eval("({})", &mouse);
source/gui/MiniMap.cpp:	g_GUI->GetActiveGUI()->GetScriptInterface()->Eval("({})", &coords);
source/scriptinterface/ScriptInterface.h:	bool Eval(const char* code) const;
source/scriptinterface/ScriptInterface.h:	template<typename CHAR> bool Eval(const CHAR* code, JS::MutableHandleValue out) const;
source/scriptinterface/ScriptInterface.h:	template<typename T, typename CHAR> bool Eval(const CHAR* code, T& out) const;
source/scriptinterface/ScriptInterface.h:bool ScriptInterface::Eval(const CHAR* code, JS::MutableHandleValue ret) const
source/scriptinterface/ScriptInterface.h:bool ScriptInterface::Eval(const CHAR* code, T& ret) const
source/scriptinterface/tests/test_ScriptInterface.h:		TS_ASSERT(script1.Eval("({'x': 123, 'y': [1, 1.5, '2', 'test', undefined, null, true, false]})", &obj1));
source/scriptinterface/tests/test_ScriptInterface.h:		TS_ASSERT(script1.Eval("var s = '?'; var v = ({get x() { return 123 }, 'y': {'w':{get z() { delete v.y; delete v.n; v = null; s += s; return 4 }}}, 'n': 100}); v", &obj1));
source/scriptinterface/tests/test_ScriptInterface.h:		TS_ASSERT(script1.Eval("var x = []; x[0] = x; ({'a': x, 'b': x})", &obj1));
source/scriptinterface/tests/test_ScriptInterface.h:		TS_ASSERT(script.Eval("({ "
source/scriptinterface/tests/test_ScriptInterface.h:		TS_ASSERT(script.Eval("Math.random()", d1));
source/scriptinterface/tests/test_ScriptInterface.h:		TS_ASSERT(script.Eval("Math.random()", d2));
source/scriptinterface/tests/test_ScriptInterface.h:		TS_ASSERT(script.Eval("Math.random()", d1));
source/scriptinterface/tests/test_ScriptInterface.h:		TS_ASSERT(script.Eval("Math.random()", d2));
source/scriptinterface/tests/test_ScriptInterface.h:		TS_ASSERT(script.Eval("Math.random()", d2));
source/scriptinterface/tests/test_ScriptInterface.h:		TS_ASSERT(script.Eval(input.c_str(), &val));
source/scriptinterface/ScriptInterface.cpp:bool ScriptInterface::Eval(const char* code) const
source/network/StunClient.cpp:	scriptInterface.Eval("({})", &stunEndpoint);
source/network/NetClient.cpp:		GetScriptInterface().Eval("({ 'type':'netwarn', 'warntype': 'server-timeout' })", &msg);
source/network/NetClient.cpp:		GetScriptInterface().Eval("({ 'type':'netwarn', 'warntype': 'server-latency' })", &msg);
source/network/NetClient.cpp:	GetScriptInterface().Eval("({'type':'players', 'newAssignments':{}})", &msg);
source/network/NetClient.cpp:		GetScriptInterface().Eval("({})", &assignment);
source/network/NetClient.cpp:	GetScriptInterface().Eval("({'type':'netstatus','status':'disconnected'})", &msg);
source/network/NetClient.cpp:		GetScriptInterface().Eval("({'type':'netstatus','status':'join_syncing'})", &msg);
source/network/NetClient.cpp:		GetScriptInterface().Eval("({'type':'netstatus','status':'waiting_for_players'})", &msg);
source/network/NetClient.cpp:	client->GetScriptInterface().Eval("({'type':'netstatus','status':'connected'})", &msg);
source/network/NetClient.cpp:			client->GetScriptInterface().Eval("({'type':'netstatus','status':'disconnected'})", &msg);
source/network/NetClient.cpp:	client->GetScriptInterface().Eval("({'type':'netstatus','status':'authenticated'})", &msg);
source/network/NetClient.cpp:	client->GetScriptInterface().Eval("({'type':'chat'})", &msg);
source/network/NetClient.cpp:	client->GetScriptInterface().Eval("({'type':'ready'})", &msg);
source/network/NetClient.cpp:	client->GetScriptInterface().Eval("({'type':'gamesetup'})", &msg);
source/network/NetClient.cpp:	client->GetScriptInterface().Eval("({'type':'start'})", &msg);
source/network/NetClient.cpp:	client->GetScriptInterface().Eval("({'type':'rejoined'})", &msg);
source/network/NetClient.cpp:	client->GetScriptInterface().Eval("({})", &msg);
source/network/NetClient.cpp:	client->GetScriptInterface().Eval("({ 'type':'netwarn', 'warntype': 'client-timeout' })", &msg);
source/network/NetClient.cpp:		client->GetScriptInterface().Eval("({ 'type':'netwarn', 'warntype': 'client-latency' })", &msg);
source/network/NetClient.cpp:	client->GetScriptInterface().Eval("({ 'type':'clients-loading' })", &msg);
source/network/NetClient.cpp:	client->GetScriptInterface().Eval("({ 'type':'paused' })", &msg);
source/network/NetClient.cpp:	client->GetScriptInterface().Eval("({'type':'netstatus','status':'active'})", &msg);
source/network/tests/test_Net.h:		scriptInterface.Eval("({mapType:'scenario',map:'maps/scenarios/Saharan Oases',mapPath:'maps/scenarios/',thing:'example'})", &attrs);
source/network/tests/test_Net.h:			client1.GetScriptInterface().Eval("({type:'debug-print', message:'[>>> client1 test sim command]\\n'})", &cmd);
source/network/tests/test_Net.h:			client2.GetScriptInterface().Eval("({type:'debug-print', message:'[>>> client2 test sim command]\\n'})", &cmd);
source/network/tests/test_Net.h:		scriptInterface.Eval("({mapType:'scenario',map:'maps/scenarios/Saharan Oases',mapPath:'maps/scenarios/',thing:'example'})", &attrs);
source/network/tests/test_Net.h:			client1.GetScriptInterface().Eval("({type:'debug-print', message:'[>>> client1 test sim command 1]\\n'})", &cmd);
source/network/tests/test_Net.h:			client1.GetScriptInterface().Eval("({type:'debug-print', message:'[>>> client1 test sim command 2]\\n'})", &cmd);
source/network/tests/test_Net.h:			client1.GetScriptInterface().Eval("({type:'debug-print', message:'[>>> client1 test sim command 3]\\n'})", &cmd);
source/network/tests/test_Net.h:			client1.GetScriptInterface().Eval("({type:'debug-print', message:'[>>> client1 test sim command 4]\\n'})", &cmd);
source/network/tests/test_NetMessage.h:		script.Eval("[4]", &val);
source/network/NetClientTurnManager.cpp:	scriptInterface.Eval("({ 'type':'out-of-sync' })", &msg);
source/tools/profiler2/jquery-1.6.4.js:		jQuery.globalEval( ( elem.text || elem.textContent || elem.innerHTML || "" ).replace( rcleanScript, "/*$0*/" ) );
source/tools/profiler2/jquery-1.6.4.js:			jQuery.globalEval( text );
source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp:		scriptInterface.Eval("({})", &attrs);
source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp:		scriptInterface.Eval("({})", &settings);
source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp:		scriptInterface.Eval("([])", &playerData);
source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp:			scriptInterface.Eval("({})", &player);
source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp:		scriptInterface.Eval("({})", &atts);
source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp:	scriptInterface.Eval("({})", &attrs);
source/tools/atlas/GameInterface/ActorViewer.cpp:			m.Simulation2.GetScriptInterface().Eval(code.c_str(), repeattime);
source/tools/replayprofile/jquery.js:		jQuery.globalEval( elem.text || elem.textContent || elem.innerHTML || "" );
source/tools/replayprofile/jquery.js:				jQuery.globalEval( data );
source/lobby/XmppClient.cpp:		scriptInterface.Eval("({})", &player);
source/lobby/XmppClient.cpp:		scriptInterface.Eval("({})", &game);
source/lobby/XmppClient.cpp:	scriptInterface.Eval("([])", ret);
source/lobby/XmppClient.cpp:		scriptInterface.Eval("({})", &board);
source/lobby/XmppClient.cpp:	scriptInterface.Eval("([])", ret);
source/lobby/XmppClient.cpp:		scriptInterface.Eval("({})", &profile);
source/lobby/XmppClient.cpp:	scriptInterface.Eval("({})", &ret);
source/graphics/MapReader.cpp:	scriptInterface.Eval("({})", ret);
source/graphics/MapGenerator.cpp:	self->m_ScriptInterface->Eval("({})", &returnValue);
source/ps/SavedGame.cpp:	simulation.GetScriptInterface().Eval("({})", &metadata);
source/ps/SavedGame.cpp:	simulation.GetScriptInterface().Eval("({})", &cameraMetadata);
source/ps/SavedGame.cpp:		scriptInterface.Eval("({})", &game);
source/ps/CConsole.cpp:	pScriptInterface->Eval(szLine, &rval);
source/ps/VisualReplay.cpp:				scriptInterface.Eval("({})", &replayData);
source/ps/VisualReplay.cpp:	scriptInterface.Eval("({})", &replayData);
source/ps/VisualReplay.cpp:	pCxPrivate->pScriptInterface->Eval("({})", &attribs);
source/ps/GameSetup/HWDetect.cpp:	scriptInterface.Eval("[]", ret);
source/ps/GameSetup/HWDetect.cpp:		scriptInterface.Eval("({})", &cache);
source/ps/GameSetup/HWDetect.cpp:	scriptInterface.Eval("[]", ret);
source/ps/GameSetup/HWDetect.cpp:		scriptInterface.Eval("({})", &tlb);
source/ps/GameSetup/HWDetect.cpp:	scriptInterface.Eval("({})", &settings);
source/ps/GameSetup/GameSetup.cpp:	scriptInterface.Eval("({})", &playerAssignments);
source/ps/GameSetup/GameSetup.cpp:		scriptInterface.Eval("({})", &localPlayer);
source/ps/GameSetup/GameSetup.cpp:	scriptInterface.Eval("({})", &sessionInitData);
source/ps/GameSetup/GameSetup.cpp:				scriptInterface->Eval("({})", &data);
source/ps/GameSetup/GameSetup.cpp:	scriptInterface.Eval("({})", &attrs);
source/ps/GameSetup/GameSetup.cpp:	scriptInterface.Eval("({})", &settings);
source/ps/GameSetup/GameSetup.cpp:	scriptInterface.Eval("([])", &playerData);
source/ps/GameSetup/GameSetup.cpp:			scriptInterface.Eval("({})", &player);
source/ps/GameSetup/GameSetup.cpp:				scriptInterface.Eval("({})", &player);
source/ps/GameSetup/GameSetup.cpp:				scriptInterface.Eval("({})", &player);
source/ps/GameSetup/GameSetup.cpp:				scriptInterface.Eval("({})", &player);
source/ps/GameSetup/GameSetup.cpp:					scriptInterface.Eval("({})", &player);
source/ps/Mod.cpp:	scriptInterface.Eval("({})", &metainfo);
source/ps/ProfileViewer.cpp:			m_ScriptInterface.Eval(L"({})", &t);
source/ps/ProfileViewer.cpp:			m_ScriptInterface.Eval("({})", &data);
source/ps/ProfileViewer.cpp:				m_ScriptInterface.Eval("([])", &row);

If it takes more than one simple function call to initialize the JS object/array, one may consider extending the ScriptInterface with a function returning an initialized object/array (similar to SetProperty).

Another particularly ugly example is array property setting using this in XmppClient.cpp:

scriptInterface.CallFunctionVoid(ret, "push", game);

Change History (1)

comment:1 by elexis, 5 years ago

Patch: Phab:D1667
Note: See TracTickets for help on using tickets.