Changeset View
Standalone View
source/scriptinterface/ScriptInterface.h
| Show First 20 Lines • Show All 132 Lines • ▼ Show 20 Lines | public: | ||||
| JSObject* CreateCustomObject(const std::string & typeName) const; | JSObject* CreateCustomObject(const std::string & typeName) const; | ||||
| void DefineCustomObjectType(JSClass *clasp, JSNative constructor, uint minArgs, JSPropertySpec *ps, JSFunctionSpec *fs, JSPropertySpec *static_ps, JSFunctionSpec *static_fs); | void DefineCustomObjectType(JSClass *clasp, JSNative constructor, uint minArgs, JSPropertySpec *ps, JSFunctionSpec *fs, JSPropertySpec *static_ps, JSFunctionSpec *static_fs); | ||||
| JS::Value GetGlobalObject() const; | JS::Value GetGlobalObject() const; | ||||
| /** | /** | ||||
| * Set the named property on the global object. | * Set the named property on the global object. | ||||
| * If @p replace is true, an existing property will be overwritten; otherwise attempts | * Optionally makes it {!ReadOnly, !DontDelete, DontEnum}. | ||||
| * to set an already-defined value will fail. | * If @p hotloadable is true and @p constant is true, we will not make it DontDelete. | ||||
| * This will allow us to override its value during hotload (i.e. when @p currentlyHotloading is true). | |||||
Itms: I think this is way clearer. | |||||
StanUnsubmitted Not Done Inline ActionsMaybe you could add @param and @return :) Stan: Maybe you could add @param and @return :) | |||||
wraitiiUnsubmitted Not Done Inline ActionsStill not 100% clear to me. Also where do these name come from? wraitii: Still not 100% clear to me.
ReadOnly maps to "constant", DontEnum maps to enumerate I suppose… | |||||
ItmsAuthorUnsubmitted Done Inline ActionsThe names come from the ecma standard. I mainly made the doc string consistent with the other functions. constant maps with ReadOnly and DontDelete, but we don't keep the latter if we want to make the constant hotloadable. Itms: The names come from the ecma standard. I mainly made the doc string consistent with the other… | |||||
| */ | */ | ||||
| template<typename T> | template<typename T> | ||||
| bool SetGlobal(const char* name, const T& value, bool replace = false, bool constant = true, bool enumerate = true); | bool SetGlobal(const char* name, const T& value, bool constant = true, bool enumerate = true, bool hotloadable = false, bool currentlyHotloading = false); | ||||
ItmsAuthorUnsubmitted Done Inline ActionsI chose to move the replace parameter (now called currentlyHotloading) at the end for semantics. This does increase the length of lines in general, but I think it's an improvement. Itms: I chose to move the `replace` parameter (now called `currentlyHotloading`) at the end for… | |||||
wraitiiUnsubmitted Not Done Inline Actions2019 - screens are wide wraitii: 2019 - screens are wide | |||||
elexisUnsubmitted Not Done Inline Actions2019 - mac users ony have one mouse button :/ Is currentlyHotloading really more clear (because the thing that currentlyHotloading triggers may also be useful outside of hotloading context)? The other names "constant", "enumerate" directly relate to defined JS property... properties. To me it adds more indirection than transparency, I know what JSPROP_READONLY | JSPROP_PERMANENT means, but "hotloadable" is something WFG branded by the author, not so clearly defined what is meant. One could also consider a SetGlobalHotloadable proxy function, or just a code comment that warns developers from making globals permanent (since hotloading needs to replace them). elexis: 2019 - mac users ony have one mouse button :/
Is `currentlyHotloading` really more clear… | |||||
ItmsAuthorUnsubmitted Done Inline ActionsI think it's more clear, but I don't intended it to be transparent, rather properly encapsulated. I wanted to provide an external interface to developers that only has useful parameters (constant, enumerable, hotloadable) instead of making them deal with the JS property attributes (in case they don't know that DontDelete prevents hotloading). currentlyHotloading is something that the simulation passes to the script interface, whereas replace could have been abused by developers wanting to work around the fact that a global is constant (which they shouldn't). I put it as last optional parameter since devs should usually leave it to false when defining globals. Now that you say it, we are indeed using hotloadable = true in all occurrences of the function, and I am not sure we will have a reason to make a global not hotloadable. So a code comment explaining why SetGlobal(..., constant = true, ...) doesn't add the DontDelete attribute would be enough. Itms: I think it's more clear, but I don't intended it to be transparent, rather properly… | |||||
elexisUnsubmitted Not Done Inline Actions
Being transparent and properly encapsulated aren't mutually exclusive. elexis: > I think it's more clear, but I don't intended it to be transparent, rather properly… | |||||
| /** | /** | ||||
| * Set the named property on the given object. | * Set the named property on the given object. | ||||
| * Optionally makes it {ReadOnly, DontDelete, DontEnum}. | * Optionally makes it {ReadOnly, DontDelete, DontEnum}. | ||||
| */ | */ | ||||
| template<typename T> | template<typename T> | ||||
| bool SetProperty(JS::HandleValue obj, const char* name, const T& value, bool constant = false, bool enumerate = true) const; | bool SetProperty(JS::HandleValue obj, const char* name, const T& value, bool constant = false, bool enumerate = true) const; | ||||
| ▲ Show 20 Lines • Show All 203 Lines • ▼ Show 20 Lines | public: | ||||
| template <typename T> | template <typename T> | ||||
| static T AssignOrFromJSVal(JSContext* cx, const JS::HandleValue& val, bool& ret); | static T AssignOrFromJSVal(JSContext* cx, const JS::HandleValue& val, bool& ret); | ||||
| private: | private: | ||||
| bool CallFunction_(JS::HandleValue val, const char* name, JS::HandleValueArray argv, JS::MutableHandleValue ret) const; | bool CallFunction_(JS::HandleValue val, const char* name, JS::HandleValueArray argv, JS::MutableHandleValue ret) const; | ||||
| bool Eval_(const char* code, JS::MutableHandleValue ret) const; | bool Eval_(const char* code, JS::MutableHandleValue ret) const; | ||||
| bool Eval_(const wchar_t* code, JS::MutableHandleValue ret) const; | bool Eval_(const wchar_t* code, JS::MutableHandleValue ret) const; | ||||
| bool SetGlobal_(const char* name, JS::HandleValue value, bool replace, bool constant, bool enumerate); | bool SetGlobal_(const char* name, JS::HandleValue value, bool constant, bool enumerate, bool hotloadable, bool currentlyHotloading); | ||||
| bool SetProperty_(JS::HandleValue obj, const char* name, JS::HandleValue value, bool readonly, bool enumerate) const; | bool SetProperty_(JS::HandleValue obj, const char* name, JS::HandleValue value, bool readonly, bool enumerate) const; | ||||
| bool SetProperty_(JS::HandleValue obj, const wchar_t* name, JS::HandleValue value, bool readonly, bool enumerate) const; | bool SetProperty_(JS::HandleValue obj, const wchar_t* name, JS::HandleValue value, bool readonly, bool enumerate) const; | ||||
| bool SetPropertyInt_(JS::HandleValue obj, int name, JS::HandleValue value, bool readonly, bool enumerate) const; | bool SetPropertyInt_(JS::HandleValue obj, int name, JS::HandleValue value, bool readonly, bool enumerate) const; | ||||
| bool GetProperty_(JS::HandleValue obj, const char* name, JS::MutableHandleValue out) const; | bool GetProperty_(JS::HandleValue obj, const char* name, JS::MutableHandleValue out) const; | ||||
| bool GetPropertyInt_(JS::HandleValue obj, int name, JS::MutableHandleValue value) const; | bool GetPropertyInt_(JS::HandleValue obj, int name, JS::MutableHandleValue value) const; | ||||
| static bool IsExceptionPending(JSContext* cx); | static bool IsExceptionPending(JSContext* cx); | ||||
| static const JSClass* GetClass(JS::HandleObject obj); | static const JSClass* GetClass(JS::HandleObject obj); | ||||
| static void* GetPrivate(JS::HandleObject obj); | static void* GetPrivate(JS::HandleObject obj); | ||||
| ▲ Show 20 Lines • Show All 109 Lines • ▼ Show 20 Lines | |||||
| template<> | template<> | ||||
| inline JS::HandleValue ScriptInterface::AssignOrFromJSVal<JS::HandleValue>(JSContext* UNUSED(cx), const JS::HandleValue& val, bool& ret) | inline JS::HandleValue ScriptInterface::AssignOrFromJSVal<JS::HandleValue>(JSContext* UNUSED(cx), const JS::HandleValue& val, bool& ret) | ||||
| { | { | ||||
| ret = true; | ret = true; | ||||
| return val; | return val; | ||||
| } | } | ||||
| template<typename T> | template<typename T> | ||||
| bool ScriptInterface::SetGlobal(const char* name, const T& value, bool replace, bool constant, bool enumerate) | bool ScriptInterface::SetGlobal(const char* name, const T& value, bool constant, bool enumerate, bool hotloadable, bool currentlyHotloading) | ||||
| { | { | ||||
| JSAutoRequest rq(GetContext()); | JSAutoRequest rq(GetContext()); | ||||
| JS::RootedValue val(GetContext()); | JS::RootedValue val(GetContext()); | ||||
| AssignOrToJSVal(GetContext(), &val, value); | AssignOrToJSVal(GetContext(), &val, value); | ||||
| return SetGlobal_(name, val, replace, constant, enumerate); | return SetGlobal_(name, val, constant, enumerate, hotloadable, currentlyHotloading); | ||||
| } | } | ||||
| template<typename T> | template<typename T> | ||||
| bool ScriptInterface::SetProperty(JS::HandleValue obj, const char* name, const T& value, bool readonly, bool enumerate) const | bool ScriptInterface::SetProperty(JS::HandleValue obj, const char* name, const T& value, bool readonly, bool enumerate) const | ||||
| { | { | ||||
| JSAutoRequest rq(GetContext()); | JSAutoRequest rq(GetContext()); | ||||
| JS::RootedValue val(GetContext()); | JS::RootedValue val(GetContext()); | ||||
| AssignOrToJSVal(GetContext(), &val, value); | AssignOrToJSVal(GetContext(), &val, value); | ||||
| ▲ Show 20 Lines • Show All 61 Lines • Show Last 20 Lines | |||||
I think this is way clearer.