From 507f44f7f926ceb5934b15e843496dcb4657c00a Mon Sep 17 00:00:00 2001 From: wraitii Date: Sat, 15 May 2021 13:54:58 +0000 Subject: [PATCH] Remove all external usage of CmptPrivate. Header cleanup. This removes usage of CmptPrivate outside of ScriptInterface. ScriptRequest can now be used to safely recover the scriptInterface from a JSContext instead of going through ScriptInterface, which allows more code cleanup. Follows 34b1920e7b Differential Revision: https://code.wildfiregames.com/D3963 This was SVN commit r25442. --- source/graphics/MapGenerator.cpp | 4 +- source/gui/GUIManager.cpp | 2 +- source/gui/GUIManager.h | 2 +- .../gui/Scripting/JSInterface_GUIManager.cpp | 11 +-- .../gui/Scripting/JSInterface_GUIProxy_impl.h | 9 ++- source/gui/Scripting/JSInterface_GUISize.cpp | 9 ++- source/gui/SettingTypes/CGUISize.cpp | 4 +- source/lobby/scripting/JSInterface_Lobby.cpp | 6 +- source/ps/GameSetup/HWDetect.cpp | 3 +- source/ps/scripting/JSInterface_Mod.cpp | 12 ++-- source/scriptinterface/FunctionWrapper.h | 48 ++++--------- source/scriptinterface/ScriptInterface.cpp | 70 ++++++++++++------- source/scriptinterface/ScriptInterface.h | 47 +++++++++++-- source/scriptinterface/ScriptRequest.h | 20 +++++- .../tests/test_FunctionWrapper.h | 18 ++--- .../simulation2/components/CCmpAIManager.cpp | 4 +- .../components/tests/test_scripts.h | 13 ++-- .../scripting/EngineScriptConversions.cpp | 2 +- .../serialization/StdDeserializer.cpp | 5 +- .../simulation2/system/ComponentManager.cpp | 2 +- 20 files changed, 173 insertions(+), 118 deletions(-) diff --git a/source/graphics/MapGenerator.cpp b/source/graphics/MapGenerator.cpp index 8a22b7e518..0c9f994f5c 100644 --- a/source/graphics/MapGenerator.cpp +++ b/source/graphics/MapGenerator.cpp @@ -163,9 +163,9 @@ bool CMapGeneratorWorker::Run() } #define REGISTER_MAPGEN_FUNC(func) \ - ScriptFunction::Register<&CMapGeneratorWorker::func, ScriptFunction::ObjectFromCBData>(rq, #func); + ScriptFunction::Register<&CMapGeneratorWorker::func, ScriptInterface::ObjectFromCBData>(rq, #func); #define REGISTER_MAPGEN_FUNC_NAME(func, name) \ - ScriptFunction::Register<&CMapGeneratorWorker::func, ScriptFunction::ObjectFromCBData>(rq, name); + ScriptFunction::Register<&CMapGeneratorWorker::func, ScriptInterface::ObjectFromCBData>(rq, name); void CMapGeneratorWorker::InitScriptInterface(const u32 seed) { diff --git a/source/gui/GUIManager.cpp b/source/gui/GUIManager.cpp index 0715641dd2..5b97680d8f 100644 --- a/source/gui/GUIManager.cpp +++ b/source/gui/GUIManager.cpp @@ -85,7 +85,7 @@ size_t CGUIManager::GetPageCount() const return m_PageStack.size(); } -void CGUIManager::SwitchPage(const CStrW& pageName, ScriptInterface* srcScriptInterface, JS::HandleValue initData) +void CGUIManager::SwitchPage(const CStrW& pageName, const ScriptInterface* srcScriptInterface, JS::HandleValue initData) { // The page stack is cleared (including the script context where initData came from), // therefore we have to clone initData. diff --git a/source/gui/GUIManager.h b/source/gui/GUIManager.h index c308ac2acd..da7f5df858 100644 --- a/source/gui/GUIManager.h +++ b/source/gui/GUIManager.h @@ -60,7 +60,7 @@ public: /** * Load a new GUI page and make it active. All current pages will be destroyed. */ - void SwitchPage(const CStrW& name, ScriptInterface* srcScriptInterface, JS::HandleValue initData); + void SwitchPage(const CStrW& name, const ScriptInterface* srcScriptInterface, JS::HandleValue initData); /** * Load a new GUI page and make it active. All current pages will be retained, diff --git a/source/gui/Scripting/JSInterface_GUIManager.cpp b/source/gui/Scripting/JSInterface_GUIManager.cpp index ce8444297c..c448da9d01 100644 --- a/source/gui/Scripting/JSInterface_GUIManager.cpp +++ b/source/gui/Scripting/JSInterface_GUIManager.cpp @@ -24,6 +24,7 @@ #include "gui/ObjectBases/IGUIObject.h" #include "ps/GameSetup/Config.h" #include "scriptinterface/FunctionWrapper.h" +#include "scriptinterface/ScriptInterface.h" #include "scriptinterface/StructuredClone.h" namespace JSI_GUIManager @@ -35,9 +36,9 @@ void PushGuiPage(const ScriptRequest& rq, const std::wstring& name, JS::HandleVa g_GUI->PushPage(name, Script::WriteStructuredClone(rq, initData), callbackFunction); } -void SwitchGuiPage(ScriptInterface::CmptPrivate* pCmptPrivate, const std::wstring& name, JS::HandleValue initData) +void SwitchGuiPage(const ScriptInterface& scriptInterface, const std::wstring& name, JS::HandleValue initData) { - g_GUI->SwitchPage(name, pCmptPrivate->pScriptInterface, initData); + g_GUI->SwitchPage(name, &scriptInterface, initData); } void PopGuiPage(const ScriptRequest& rq, JS::HandleValue args) @@ -84,8 +85,8 @@ void RegisterScriptFunctions(const ScriptRequest& rq) ScriptFunction::Register<&TemplateExists>(rq, "TemplateExists"); ScriptFunction::Register<&GetTemplate>(rq, "GetTemplate"); - ScriptFunction::Register<&CGUI::FindObjectByName, &ScriptFunction::ObjectFromCBData>(rq, "GetGUIObjectByName"); - ScriptFunction::Register<&CGUI::SetGlobalHotkey, &ScriptFunction::ObjectFromCBData>(rq, "SetGlobalHotkey"); - ScriptFunction::Register<&CGUI::UnsetGlobalHotkey, &ScriptFunction::ObjectFromCBData>(rq, "UnsetGlobalHotkey"); + ScriptFunction::Register<&CGUI::FindObjectByName, &ScriptInterface::ObjectFromCBData>(rq, "GetGUIObjectByName"); + ScriptFunction::Register<&CGUI::SetGlobalHotkey, &ScriptInterface::ObjectFromCBData>(rq, "SetGlobalHotkey"); + ScriptFunction::Register<&CGUI::UnsetGlobalHotkey, &ScriptInterface::ObjectFromCBData>(rq, "UnsetGlobalHotkey"); } } diff --git a/source/gui/Scripting/JSInterface_GUIProxy_impl.h b/source/gui/Scripting/JSInterface_GUIProxy_impl.h index 7f7adeed3c..e997868dc2 100644 --- a/source/gui/Scripting/JSInterface_GUIProxy_impl.h +++ b/source/gui/Scripting/JSInterface_GUIProxy_impl.h @@ -25,7 +25,7 @@ #include "ps/CLogger.h" #include "scriptinterface/FunctionWrapper.h" #include "scriptinterface/ScriptExtraHeaders.h" -#include "scriptinterface/ScriptInterface.h" +#include "scriptinterface/ScriptRequest.h" #include @@ -164,8 +164,7 @@ std::unique_ptr JSI_GUIProxy::CreateJSObject(const ScriptReq template bool JSI_GUIProxy::get(JSContext* cx, JS::HandleObject proxy, JS::HandleValue UNUSED(receiver), JS::HandleId id, JS::MutableHandleValue vp) const { - ScriptInterface* pScriptInterface = ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface; - ScriptRequest rq(*pScriptInterface); + ScriptRequest rq(cx); T* e = IGUIProxyObject::FromPrivateSlot(proxy.get()); if (!e) @@ -242,7 +241,7 @@ bool JSI_GUIProxy::set(JSContext* cx, JS::HandleObject proxy, JS::HandleId id return result.fail(JSMSG_OBJECT_REQUIRED); } - ScriptRequest rq(*ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface); + ScriptRequest rq(cx); JS::RootedValue idval(rq.cx); if (!JS_IdToValue(rq.cx, id, &idval)) @@ -297,7 +296,7 @@ bool JSI_GUIProxy::delete_(JSContext* cx, JS::HandleObject proxy, JS::HandleI return result.fail(JSMSG_OBJECT_REQUIRED); } - ScriptRequest rq(*ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface); + ScriptRequest rq(cx); JS::RootedValue idval(rq.cx); if (!JS_IdToValue(rq.cx, id, &idval)) diff --git a/source/gui/Scripting/JSInterface_GUISize.cpp b/source/gui/Scripting/JSInterface_GUISize.cpp index ce99061945..766ef3a5b2 100644 --- a/source/gui/Scripting/JSInterface_GUISize.cpp +++ b/source/gui/Scripting/JSInterface_GUISize.cpp @@ -48,10 +48,10 @@ void JSI_GUISize::RegisterScriptClass(ScriptInterface& scriptInterface) bool JSI_GUISize::construct(JSContext* cx, uint argc, JS::Value* vp) { JS::CallArgs args = JS::CallArgsFromVp(argc, vp); - ScriptInterface* pScriptInterface = ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface; - ScriptRequest rq(*pScriptInterface); + ScriptRequest rq(cx); + const ScriptInterface& scriptInterface = rq.GetScriptInterface(); - JS::RootedObject obj(rq.cx, pScriptInterface->CreateCustomObject("GUISize")); + JS::RootedObject obj(rq.cx, scriptInterface.CreateCustomObject("GUISize")); if (args.length() == 8) { @@ -107,8 +107,7 @@ bool JSI_GUISize::toString(JSContext* cx, uint argc, JS::Value* vp) JS::CallArgs args = JS::CallArgsFromVp(argc, vp); CStr buffer; - ScriptInterface* pScriptInterface = ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface; - ScriptRequest rq(*pScriptInterface); + ScriptRequest rq(cx); double val, valr; #define SIDE(side) \ diff --git a/source/gui/SettingTypes/CGUISize.cpp b/source/gui/SettingTypes/CGUISize.cpp index 8d98d862b9..63b0a9c54c 100644 --- a/source/gui/SettingTypes/CGUISize.cpp +++ b/source/gui/SettingTypes/CGUISize.cpp @@ -144,8 +144,8 @@ bool CGUISize::FromString(const CStr& Value) void CGUISize::ToJSVal(const ScriptRequest& rq, JS::MutableHandleValue ret) const { - ScriptInterface* pScriptInterface = ScriptInterface::GetScriptInterfaceAndCBData(rq.cx)->pScriptInterface; - ret.setObjectOrNull(pScriptInterface->CreateCustomObject("GUISize")); + const ScriptInterface& scriptInterface = rq.GetScriptInterface(); + ret.setObjectOrNull(scriptInterface.CreateCustomObject("GUISize")); if (!ret.isObject()) { diff --git a/source/lobby/scripting/JSInterface_Lobby.cpp b/source/lobby/scripting/JSInterface_Lobby.cpp index d490089dc3..bc29625d9c 100644 --- a/source/lobby/scripting/JSInterface_Lobby.cpp +++ b/source/lobby/scripting/JSInterface_Lobby.cpp @@ -110,11 +110,11 @@ IXmppClient* XmppGetter(const ScriptRequest&, JS::CallArgs&) return g_XmppClient; } -void SendRegisterGame(ScriptInterface::CmptPrivate* pCmptPrivate, JS::HandleValue data) +void SendRegisterGame(const ScriptInterface& scriptInterface, JS::HandleValue data) { if (!g_XmppClient) { - ScriptRequest rq(pCmptPrivate->pScriptInterface); + ScriptRequest rq(scriptInterface); ScriptException::Raise(rq, "Cannot call SendRegisterGame without an initialized XmppClient!"); return; } @@ -126,7 +126,7 @@ void SendRegisterGame(ScriptInterface::CmptPrivate* pCmptPrivate, JS::HandleValu return; } - g_XmppClient->SendIqRegisterGame(*(pCmptPrivate->pScriptInterface), data); + g_XmppClient->SendIqRegisterGame(scriptInterface, data); } // Unlike other functions, this one just returns Undefined if XmppClient isn't initialised. diff --git a/source/ps/GameSetup/HWDetect.cpp b/source/ps/GameSetup/HWDetect.cpp index 4d9ceb1fa3..e83790502b 100644 --- a/source/ps/GameSetup/HWDetect.cpp +++ b/source/ps/GameSetup/HWDetect.cpp @@ -44,8 +44,9 @@ #include "ps/UserReport.h" #include "ps/VideoMode.h" #include "scriptinterface/FunctionWrapper.h" -#include "scriptinterface/Object.h" #include "scriptinterface/JSON.h" +#include "scriptinterface/Object.h" +#include "scriptinterface/ScriptInterface.h" #if OS_LINUX #include diff --git a/source/ps/scripting/JSInterface_Mod.cpp b/source/ps/scripting/JSInterface_Mod.cpp index b19b157453..ac4e7dc518 100644 --- a/source/ps/scripting/JSInterface_Mod.cpp +++ b/source/ps/scripting/JSInterface_Mod.cpp @@ -24,25 +24,24 @@ extern void RestartEngine(); -namespace +namespace JSI_Mod { -bool SetModsAndRestartEngine(ScriptInterface::CmptPrivate* pCmptPrivate, const std::vector& mods) +bool SetModsAndRestartEngine(const ScriptInterface& scriptInterface, const std::vector& mods) { Mod::ClearIncompatibleMods(); - if (!Mod::CheckAndEnableMods(*(pCmptPrivate->pScriptInterface), mods)) + if (!Mod::CheckAndEnableMods(scriptInterface, mods)) return false; RestartEngine(); return true; } -} -bool HasFailedMods(ScriptInterface::CmptPrivate* UNUSED(pCmptPrivate)) +bool HasFailedMods() { return Mod::GetFailedMods().size() > 0; } -void JSI_Mod::RegisterScriptFunctions(const ScriptRequest& rq) +void RegisterScriptFunctions(const ScriptRequest& rq) { ScriptFunction::Register<&Mod::GetEngineInfo>(rq, "GetEngineInfo"); ScriptFunction::Register<&Mod::GetAvailableMods>(rq, "GetAvailableMods"); @@ -51,3 +50,4 @@ void JSI_Mod::RegisterScriptFunctions(const ScriptRequest& rq) ScriptFunction::Register<&Mod::GetFailedMods>(rq, "GetFailedMods"); ScriptFunction::Register<&SetModsAndRestartEngine>(rq, "SetModsAndRestartEngine"); } +} diff --git a/source/scriptinterface/FunctionWrapper.h b/source/scriptinterface/FunctionWrapper.h index 764af14b3a..978c124709 100644 --- a/source/scriptinterface/FunctionWrapper.h +++ b/source/scriptinterface/FunctionWrapper.h @@ -21,9 +21,13 @@ #include "Object.h" #include "ScriptConversions.h" #include "ScriptExceptions.h" -#include "ScriptInterface.h" #include "ScriptRequest.h" +#include +#include + +class ScriptInterface; + /** * This class introduces templates to conveniently wrap C++ functions in JSNative functions. * This _is_ rather template heavy, so compilation times beware. @@ -128,14 +132,14 @@ private: /** * ConvertFromJS is a wrapper around DoConvertFromJS, and serves to: * - unwrap the tuple types as a parameter pack - * - handle specific cases for the first argument (cmptPrivate, ScriptRequest). + * - handle specific cases for the first argument (ScriptRequest, ...). * * Trick: to unpack the types of the tuple as a parameter pack, we deduce them from the function signature. * To do that, we want the tuple in the arguments, but we don't want to actually have to default-instantiate, * so we'll pass a nullptr that's static_cast to what we want. */ template - static std::tuple ConvertFromJS(ScriptInterface::CmptPrivate*, const ScriptRequest& rq, JS::CallArgs& args, bool& went_ok, std::tuple*) + static std::tuple ConvertFromJS(const ScriptRequest& rq, JS::CallArgs& args, bool& went_ok, std::tuple*) { if constexpr (sizeof...(Types) == 0) { @@ -147,23 +151,9 @@ private: return DoConvertFromJS<0, Types...>(rq, args, went_ok); } - // Overloads for CmptPrivate* first argument. - template - static std::tuple ConvertFromJS(ScriptInterface::CmptPrivate* cmptPrivate, const ScriptRequest& rq, JS::CallArgs& args, bool& went_ok, std::tuple*) - { - if constexpr (sizeof...(Types) == 0) - { - // GCC (at least < 9) & VS17 prints warnings if arguments are not used in some constexpr branch. - UNUSED2(rq); UNUSED2(args); UNUSED2(went_ok); - return std::forward_as_tuple(cmptPrivate); - } - else - return std::tuple_cat(std::forward_as_tuple(cmptPrivate), DoConvertFromJS<0, Types...>(rq, args, went_ok)); - } - // Overloads for ScriptRequest& first argument. template - static std::tuple ConvertFromJS(ScriptInterface::CmptPrivate*, const ScriptRequest& rq, JS::CallArgs& args, bool& went_ok, std::tuple*) + static std::tuple ConvertFromJS(const ScriptRequest& rq, JS::CallArgs& args, bool& went_ok, std::tuple*) { if constexpr (sizeof...(Types) == 0) { @@ -177,16 +167,16 @@ private: // Overloads for ScriptInterface& first argument. template - static std::tuple ConvertFromJS(ScriptInterface::CmptPrivate* cmptPrivate, const ScriptRequest& rq, JS::CallArgs& args, bool& went_ok, std::tuple*) + static std::tuple ConvertFromJS(const ScriptRequest& rq, JS::CallArgs& args, bool& went_ok, std::tuple*) { if constexpr (sizeof...(Types) == 0) { // GCC (at least < 9) & VS17 prints warnings if arguments are not used in some constexpr branch. UNUSED2(rq); UNUSED2(args); UNUSED2(went_ok); - return std::forward_as_tuple(*cmptPrivate->pScriptInterface); + return std::forward_as_tuple(rq.GetScriptInterface()); } else - return std::tuple_cat(std::forward_as_tuple(*cmptPrivate->pScriptInterface), DoConvertFromJS<0, Types...>(rq, args, went_ok)); + return std::tuple_cat(std::forward_as_tuple(rq.GetScriptInterface()), DoConvertFromJS<0, Types...>(rq, args, went_ok)); } /////////////////////////////////////////////////////////////////////////// @@ -289,7 +279,7 @@ public: * so that it can be called from JS and manipulated in Spidermonkey. * Most C++ functions can be directly wrapped, so long as their arguments are * convertible from JS::Value and their return value is convertible to JS::Value (or void) - * The C++ function may optionally take const ScriptRequest& or CmptPrivate* as its first argument. + * The C++ function may optionally take const ScriptRequest& or ScriptInterface& as its first argument. * The function may be an object method, in which case you need to pass an appropriate getter * * Optimisation note: the ScriptRequest object is created even without arguments, @@ -303,8 +293,7 @@ public: using ObjType = typename args_info::object_type; JS::CallArgs args = JS::CallArgsFromVp(argc, vp); - ScriptInterface* scriptInterface = ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface; - ScriptRequest rq(*scriptInterface); + ScriptRequest rq(cx); // If the callable is an object method, we must specify how to fetch the object. static_assert(std::is_same_v::object_type, void> || thisGetter != nullptr, @@ -327,7 +316,7 @@ public: #endif bool went_ok = true; - typename args_info::arg_types outs = ConvertFromJS(ScriptInterface::GetScriptInterfaceAndCBData(cx), rq, args, went_ok, static_cast::arg_types*>(nullptr)); + typename args_info::arg_types outs = ConvertFromJS(rq, args, went_ok, static_cast::arg_types*>(nullptr)); if (!went_ok) return false; @@ -412,15 +401,6 @@ public: { JS_DefineFunction(cx, scope, name, &ToJSNative, args_info::nb_args, flags); } - - /** - * Convert the CmptPrivate callback data to T* - */ - template - static T* ObjectFromCBData(const ScriptRequest& rq, JS::CallArgs&) - { - return static_cast(ScriptInterface::GetScriptInterfaceAndCBData(rq.cx)->pCBData); - } }; #endif // INCLUDED_FUNCTIONWRAPPER diff --git a/source/scriptinterface/ScriptInterface.cpp b/source/scriptinterface/ScriptInterface.cpp index eafe1ffde4..bfddde9f1c 100644 --- a/source/scriptinterface/ScriptInterface.cpp +++ b/source/scriptinterface/ScriptInterface.cpp @@ -52,7 +52,6 @@ * directly accessing the underlying JS api. */ - struct ScriptInterface_impl { ScriptInterface_impl(const char* nativeScopeName, const shared_ptr& context); @@ -76,14 +75,21 @@ struct ScriptInterface_impl * Constructor for ScriptRequest - here because it needs access into ScriptInterface_impl. */ ScriptRequest::ScriptRequest(const ScriptInterface& scriptInterface) : - cx(scriptInterface.m->m_cx), glob(scriptInterface.m->m_glob), nativeScope(scriptInterface.m->m_nativeScope) + cx(scriptInterface.m->m_cx), + glob(scriptInterface.m->m_glob), + nativeScope(scriptInterface.m->m_nativeScope), + m_ScriptInterface(scriptInterface) { - m_formerRealm = JS::EnterRealm(cx, scriptInterface.m->m_glob); + m_FormerRealm = JS::EnterRealm(cx, scriptInterface.m->m_glob); } ScriptRequest::~ScriptRequest() { - JS::LeaveRealm(cx, m_formerRealm); + JS::LeaveRealm(cx, m_FormerRealm); +} + +ScriptRequest::ScriptRequest(JSContext* cx) : ScriptRequest(ScriptInterface::CmptPrivate::GetScriptInterface(cx)) +{ } JS::Value ScriptRequest::globalValue() const @@ -91,6 +97,10 @@ JS::Value ScriptRequest::globalValue() const return JS::ObjectValue(*glob); } +const ScriptInterface& ScriptRequest::GetScriptInterface() const +{ + return m_ScriptInterface; +} namespace { @@ -112,7 +122,7 @@ JSClass global_class = { bool print(JSContext* cx, uint argc, JS::Value* vp) { JS::CallArgs args = JS::CallArgsFromVp(argc, vp); - ScriptRequest rq(*ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface); \ + ScriptRequest rq(cx); for (uint i = 0; i < args.length(); ++i) { @@ -135,7 +145,7 @@ bool logmsg(JSContext* cx, uint argc, JS::Value* vp) return true; } - ScriptRequest rq(*ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface); \ + ScriptRequest rq(cx); std::wstring str; if (!Script::FromJSVal(rq, args[0], str)) return false; @@ -153,7 +163,7 @@ bool warn(JSContext* cx, uint argc, JS::Value* vp) return true; } - ScriptRequest rq(*ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface); \ + ScriptRequest rq(cx); std::wstring str; if (!Script::FromJSVal(rq, args[0], str)) return false; @@ -171,7 +181,7 @@ bool error(JSContext* cx, uint argc, JS::Value* vp) return true; } - ScriptRequest rq(*ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface); \ + ScriptRequest rq(cx); std::wstring str; if (!Script::FromJSVal(rq, args[0], str)) return false; @@ -271,24 +281,24 @@ static double generate_uniform_real(boost::rand48& rng, double min, double max) } } -bool Math_random(JSContext* cx, uint argc, JS::Value* vp) -{ - JS::CallArgs args = JS::CallArgsFromVp(argc, vp); - double r; - if (!ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface->MathRandom(r)) - return false; +} // anonymous namespace - args.rval().setNumber(r); +bool ScriptInterface::MathRandom(double& nbr) const +{ + if (m->m_rng == nullptr) + return false; + nbr = generate_uniform_real(*(m->m_rng), 0.0, 1.0); return true; } -} // anonymous namespace - -bool ScriptInterface::MathRandom(double& nbr) +bool ScriptInterface::Math_random(JSContext* cx, uint argc, JS::Value* vp) { - if (m->m_rng == NULL) + JS::CallArgs args = JS::CallArgsFromVp(argc, vp); + double r; + if (!ScriptInterface::CmptPrivate::GetScriptInterface(cx).MathRandom(r)) return false; - nbr = generate_uniform_real(*(m->m_rng), 0.0, 1.0); + + args.rval().setNumber(r); return true; } @@ -356,18 +366,30 @@ ScriptInterface::~ScriptInterface() } } +const ScriptInterface& ScriptInterface::CmptPrivate::GetScriptInterface(JSContext *cx) +{ + CmptPrivate* pCmptPrivate = (CmptPrivate*)JS::GetRealmPrivate(JS::GetCurrentRealmOrNull(cx)); + ENSURE(pCmptPrivate); + return *pCmptPrivate->pScriptInterface; +} + +void* ScriptInterface::CmptPrivate::GetCBData(JSContext *cx) +{ + CmptPrivate* pCmptPrivate = (CmptPrivate*)JS::GetRealmPrivate(JS::GetCurrentRealmOrNull(cx)); + return pCmptPrivate ? pCmptPrivate->pCBData : nullptr; +} + void ScriptInterface::SetCallbackData(void* pCBData) { m_CmptPrivate.pCBData = pCBData; } -ScriptInterface::CmptPrivate* ScriptInterface::GetScriptInterfaceAndCBData(JSContext* cx) +template <> +void* ScriptInterface::ObjectFromCBData(const ScriptRequest& rq) { - CmptPrivate* pCmptPrivate = (CmptPrivate*)JS::GetRealmPrivate(JS::GetCurrentRealmOrNull(cx)); - return pCmptPrivate; + return ScriptInterface::CmptPrivate::GetCBData(rq.cx); } - bool ScriptInterface::LoadGlobalScripts() { // Ignore this failure in tests diff --git a/source/scriptinterface/ScriptInterface.h b/source/scriptinterface/ScriptInterface.h index 58b571616b..e5c884a65e 100644 --- a/source/scriptinterface/ScriptInterface.h +++ b/source/scriptinterface/ScriptInterface.h @@ -73,6 +73,7 @@ class ScriptInterface NONCOPYABLE(ScriptInterface); friend class ScriptRequest; + public: /** @@ -88,12 +89,36 @@ public: struct CmptPrivate { + friend class ScriptInterface; + public: + static const ScriptInterface& GetScriptInterface(JSContext* cx); + static void* GetCBData(JSContext* cx); + protected: ScriptInterface* pScriptInterface; // the ScriptInterface object the compartment belongs to void* pCBData; // meant to be used as the "this" object for callback functions - } m_CmptPrivate; + }; void SetCallbackData(void* pCBData); - static CmptPrivate* GetScriptInterfaceAndCBData(JSContext* cx); + + /** + * Convert the CmptPrivate callback data to T* + */ + template + static T* ObjectFromCBData(const ScriptRequest& rq) + { + static_assert(!std::is_same_v); + ScriptInterface::CmptPrivate::GetCBData(rq.cx); + return static_cast(ObjectFromCBData(rq)); + } + + /** + * Variant for the function wrapper. + */ + template + static T* ObjectFromCBData(const ScriptRequest& rq, JS::CallArgs&) + { + return ObjectFromCBData(rq); + } /** * GetGeneralJSContext returns the context without starting a GC request and without @@ -177,12 +202,14 @@ public: template bool Eval(const char* code, T& out) const; /** - * MathRandom (this function) calls the random number generator assigned to this ScriptInterface instance and - * returns the generated number. - * Math_random (with underscore, not this function) is a global function, but different random number generators can be - * stored per ScriptInterface. It calls MathRandom of the current ScriptInterface instance. + * Calls the random number generator assigned to this ScriptInterface instance and returns the generated number. */ - bool MathRandom(double& nbr); + bool MathRandom(double& nbr) const; + + /** + * JSNative wrapper of the above. + */ + static bool Math_random(JSContext* cx, uint argc, JS::Value* vp); /** * Retrieve the private data field of a JSObject that is an instance of the given JSClass. @@ -230,6 +257,8 @@ private: JSNative m_Constructor; }; + CmptPrivate m_CmptPrivate; + // Take care to keep this declaration before heap rooted members. Destructors of heap rooted // members have to be called before the custom destructor of ScriptInterface_impl. std::unique_ptr m; @@ -237,6 +266,10 @@ private: std::map m_CustomObjectTypes; }; +// Explicitly instantiate void* as that is used for the generic template, +// and we want to define it in the .cpp file. +template <> void* ScriptInterface::ObjectFromCBData(const ScriptRequest& rq); + template bool ScriptInterface::SetGlobal(const char* name, const T& value, bool replace, bool constant, bool enumerate) { diff --git a/source/scriptinterface/ScriptRequest.h b/source/scriptinterface/ScriptRequest.h index c23f212e9f..e5c834fc92 100644 --- a/source/scriptinterface/ScriptRequest.h +++ b/source/scriptinterface/ScriptRequest.h @@ -71,12 +71,30 @@ public: ScriptRequest(std::shared_ptr scriptInterface) : ScriptRequest(*scriptInterface) {} ~ScriptRequest(); + /** + * Create a script request from a JSContext. + * This can be used to get the script interface in a JSNative function. + * In general, you shouldn't have to rely on this otherwise. + */ + ScriptRequest(JSContext* cx); + + /** + * Return the scriptInterface active when creating this ScriptRequest. + * Note that this is multi-request safe: even if another ScriptRequest is created, + * it will point to the original scriptInterface, and thus can be used to re-enter the realm. + */ + const ScriptInterface& GetScriptInterface() const; + JS::Value globalValue() const; + + // Note that JSContext actually changes behind the scenes when creating another ScriptRequest for another realm, + // so be _very_ careful when juggling between different realms. JSContext* cx; JS::HandleObject glob; JS::HandleObject nativeScope; private: - JS::Realm* m_formerRealm; + const ScriptInterface& m_ScriptInterface; + JS::Realm* m_FormerRealm; }; diff --git a/source/scriptinterface/tests/test_FunctionWrapper.h b/source/scriptinterface/tests/test_FunctionWrapper.h index 433ae4ec62..3bc43bc08c 100644 --- a/source/scriptinterface/tests/test_FunctionWrapper.h +++ b/source/scriptinterface/tests/test_FunctionWrapper.h @@ -43,8 +43,8 @@ public: static void _handle(JS::HandleValue) {}; static void _handle_2(int, JS::HandleValue, bool) {}; - static void _cmpt_private(ScriptInterface::CmptPrivate*) {}; - static int _cmpt_private_2(ScriptInterface::CmptPrivate*, int a, bool) { return a; }; + static void _script_interface(const ScriptInterface&) {}; + static int _script_interface_2(const ScriptInterface&, int a, bool) { return a; }; static void _script_request(const ScriptRequest&) {}; static int _script_request_2(const ScriptRequest&, int a, bool) { return a; }; @@ -53,8 +53,8 @@ public: { static_assert(std::is_same_v), JSNative>); static_assert(std::is_same_v), JSNative>); - static_assert(std::is_same_v), JSNative>); - static_assert(std::is_same_v), JSNative>); + static_assert(std::is_same_v), JSNative>); + static_assert(std::is_same_v), JSNative>); static_assert(std::is_same_v), JSNative>); static_assert(std::is_same_v), JSNative>); } @@ -71,13 +71,13 @@ public: void test_method_wrappers() { static_assert(std::is_same_v>), JSNative>); + &ScriptInterface::ObjectFromCBData>), JSNative>); static_assert(std::is_same_v>), JSNative>); + &ScriptInterface::ObjectFromCBData>), JSNative>); static_assert(std::is_same_v>), JSNative>); + &ScriptInterface::ObjectFromCBData>), JSNative>); static_assert(std::is_same_v>), JSNative>); + &ScriptInterface::ObjectFromCBData>), JSNative>); } void test_calling() @@ -100,7 +100,7 @@ public: TS_ASSERT_EQUALS(ret, 4); } - ScriptFunction::Register<&TestFunctionWrapper::_cmpt_private_2>(script, "_cmpt_private_2"); + ScriptFunction::Register<&TestFunctionWrapper::_script_interface_2>(script, "_cmpt_private_2"); { std::string input = "Test._cmpt_private_2(4);"; int ret = 0; diff --git a/source/simulation2/components/CCmpAIManager.cpp b/source/simulation2/components/CCmpAIManager.cpp index 47c5c2a4d1..58a7f761c9 100644 --- a/source/simulation2/components/CCmpAIManager.cpp +++ b/source/simulation2/components/CCmpAIManager.cpp @@ -236,7 +236,7 @@ public: ScriptRequest rq(m_ScriptInterface); #define REGISTER_FUNC_NAME(func, name) \ - ScriptFunction::Register<&CAIWorker::func, ScriptFunction::ObjectFromCBData>(rq, name); + ScriptFunction::Register<&CAIWorker::func, ScriptInterface::ObjectFromCBData>(rq, name); REGISTER_FUNC_NAME(PostCommand, "PostCommand"); REGISTER_FUNC_NAME(LoadScripts, "IncludeModule"); @@ -343,7 +343,7 @@ public: /** * Debug function for AI scripts to dump 2D array data (e.g. terrain tile weights). */ - void DumpImage(ScriptInterface::CmptPrivate* UNUSED(pCmptPrivate), const std::wstring& name, const std::vector& data, u32 w, u32 h, u32 max) + void DumpImage(const std::wstring& name, const std::vector& data, u32 w, u32 h, u32 max) { // TODO: this is totally not threadsafe. VfsPath filename = L"screenshots/aidump/" + name; diff --git a/source/simulation2/components/tests/test_scripts.h b/source/simulation2/components/tests/test_scripts.h index b1296db508..5dd8f6c649 100644 --- a/source/simulation2/components/tests/test_scripts.h +++ b/source/simulation2/components/tests/test_scripts.h @@ -49,21 +49,22 @@ public: TSM_ASSERT(L"Running script "+pathname.string(), scriptInterface.LoadScript(pathname, content)); } - static void Script_LoadComponentScript(ScriptInterface::CmptPrivate* pCmptPrivate, const VfsPath& pathname) + static void Script_LoadComponentScript(const ScriptInterface& scriptInterface, const VfsPath& pathname) { - CComponentManager* componentManager = static_cast (pCmptPrivate->pCBData); + ScriptRequest rq(scriptInterface); + CComponentManager* componentManager = scriptInterface.ObjectFromCBData(rq); TS_ASSERT(componentManager->LoadScript(VfsPath(L"simulation/components") / pathname)); } - static void Script_LoadHelperScript(ScriptInterface::CmptPrivate* pCmptPrivate, const VfsPath& pathname) + static void Script_LoadHelperScript(const ScriptInterface& scriptInterface, const VfsPath& pathname) { - CComponentManager* componentManager = static_cast (pCmptPrivate->pCBData); + ScriptRequest rq(scriptInterface); + CComponentManager* componentManager = scriptInterface.ObjectFromCBData(rq); TS_ASSERT(componentManager->LoadScript(VfsPath(L"simulation/helpers") / pathname)); } - static JS::Value Script_SerializationRoundTrip(ScriptInterface::CmptPrivate* pCmptPrivate, JS::HandleValue value) + static JS::Value Script_SerializationRoundTrip(const ScriptInterface& scriptInterface, JS::HandleValue value) { - ScriptInterface& scriptInterface = *(pCmptPrivate->pScriptInterface); ScriptRequest rq(scriptInterface); JS::RootedValue val(rq.cx); diff --git a/source/simulation2/scripting/EngineScriptConversions.cpp b/source/simulation2/scripting/EngineScriptConversions.cpp index 0cab3ee6ba..dc3ee19e3f 100644 --- a/source/simulation2/scripting/EngineScriptConversions.cpp +++ b/source/simulation2/scripting/EngineScriptConversions.cpp @@ -55,7 +55,7 @@ template<> void Script::ToJSVal(const ScriptRequest& rq, JS::Mutab // Otherwise we need to construct a wrapper object // (TODO: cache wrapper objects?) JS::RootedObject obj(rq.cx); - if (!val->NewJSObject(*ScriptInterface::GetScriptInterfaceAndCBData(rq.cx)->pScriptInterface, &obj)) + if (!val->NewJSObject(rq.GetScriptInterface(), &obj)) { // Report as an error, since scripts really shouldn't try to use unscriptable interfaces LOGERROR("IComponent does not have a scriptable interface"); diff --git a/source/simulation2/serialization/StdDeserializer.cpp b/source/simulation2/serialization/StdDeserializer.cpp index 00217dda3c..c41dec6a3f 100644 --- a/source/simulation2/serialization/StdDeserializer.cpp +++ b/source/simulation2/serialization/StdDeserializer.cpp @@ -26,6 +26,7 @@ #include "scriptinterface/Object.h" #include "scriptinterface/ScriptConversions.h" #include "scriptinterface/ScriptExtraHeaders.h" // For typed arrays and ArrayBuffer +#include "scriptinterface/ScriptInterface.h" #include "simulation2/serialization/ISerializer.h" #include "simulation2/serialization/SerializedScriptTypes.h" #include "simulation2/serialization/StdSerializer.h" // for DEBUG_SERIALIZER_ANNOTATE @@ -33,14 +34,14 @@ CStdDeserializer::CStdDeserializer(const ScriptInterface& scriptInterface, std::istream& stream) : m_ScriptInterface(scriptInterface), m_Stream(stream) { - JS_AddExtraGCRootsTracer(m_ScriptInterface.GetGeneralJSContext(), CStdDeserializer::Trace, this); + JS_AddExtraGCRootsTracer(ScriptRequest(scriptInterface).cx, CStdDeserializer::Trace, this); // Insert a dummy object in front, as valid tags start at 1. m_ScriptBackrefs.emplace_back(nullptr); } CStdDeserializer::~CStdDeserializer() { - JS_RemoveExtraGCRootsTracer(m_ScriptInterface.GetGeneralJSContext(), CStdDeserializer::Trace, this); + JS_RemoveExtraGCRootsTracer(ScriptRequest(m_ScriptInterface).cx, CStdDeserializer::Trace, this); } void CStdDeserializer::Trace(JSTracer *trc, void *data) diff --git a/source/simulation2/system/ComponentManager.cpp b/source/simulation2/system/ComponentManager.cpp index b5e5c4f594..32a9200879 100644 --- a/source/simulation2/system/ComponentManager.cpp +++ b/source/simulation2/system/ComponentManager.cpp @@ -70,7 +70,7 @@ CComponentManager::CComponentManager(CSimContext& context, shared_ptr Getter = &ScriptFunction::ObjectFromCBData; + constexpr ScriptFunction::ObjectGetter Getter = &ScriptInterface::ObjectFromCBData; ScriptFunction::Register<&CComponentManager::Script_RegisterComponentType, Getter>(rq, "RegisterComponentType"); ScriptFunction::Register<&CComponentManager::Script_RegisterSystemComponentType, Getter>(rq, "RegisterSystemComponentType"); ScriptFunction::Register<&CComponentManager::Script_ReRegisterComponentType, Getter>(rq, "ReRegisterComponentType");