Page MenuHomeWildfire Games
Differential D1844 Diff 7878 source/scriptinterface/ScriptInterface.h

Changeset View

Standalone View

View Options

source/scriptinterface/ScriptInterface.h

Show First 20 Lines • Show All 132 Lines • ▼ Show 20 Linespublic:
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, DontEnum}. We do not allow to make it DontDelete, so that it can be hotloaded
* to set an already-defined value will fail. * by deleting it and re-creating it, which is done by setting @p replace to true.
*/ */
ItmsAuthorUnsubmitted
Done
Inline Actions

I think this is way clearer.

Itms: I think this is way clearer.
StanUnsubmitted
Not Done
Inline Actions

Maybe you could add @param and @return :)

Stan: Maybe you could add @param and @return :)
wraitiiUnsubmitted
Not Done
Inline Actions

Still not 100% clear to me.
ReadOnly maps to "constant", DontEnum maps to enumerate I suppose, and then what does "DontDelete" map to?

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 Actions

The 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 replace = false, bool constant = true, bool enumerate = true);
ItmsAuthorUnsubmitted
Done
Inline Actions

I 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 Actions

2019 - screens are wide

wraitii: 2019 - screens are wide
elexisUnsubmitted
Not Done
Inline Actions

2019 - 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 Actions

I 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

I think it's more clear, but I don't intended it to be transparent, rather properly encapsulated.

Being transparent and properly encapsulated aren't mutually exclusive.
(We can have two variants of the function, one that exposes the JS attributes as arguments, one educated about hotloading that calls the other one for example. Sounds both more transparent and encapsulated simultaneously?)

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 204 Lines • ▼ Show 20 Linespublic:
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 replace, bool constant, bool enumerate);
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 constant, 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 constant, 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 constant, 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);
struct CustomType struct CustomType
{ {
▲ Show 20 Lines • Show All 115 Lines • ▼ Show 20 Lines
{ {
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, replace, constant, enumerate);
} }
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 constant, 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);
return SetProperty_(obj, name, val, readonly, enumerate); return SetProperty_(obj, name, val, constant, enumerate);
} }
template<typename T> template<typename T>
bool ScriptInterface::SetProperty(JS::HandleValue obj, const wchar_t* name, const T& value, bool readonly, bool enumerate) const bool ScriptInterface::SetProperty(JS::HandleValue obj, const wchar_t* name, const T& value, bool constant, 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);
return SetProperty_(obj, name, val, readonly, enumerate); return SetProperty_(obj, name, val, constant, enumerate);
} }
template<typename T> template<typename T>
bool ScriptInterface::SetPropertyInt(JS::HandleValue obj, int name, const T& value, bool readonly, bool enumerate) const bool ScriptInterface::SetPropertyInt(JS::HandleValue obj, int name, const T& value, bool constant, 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);
return SetPropertyInt_(obj, name, val, readonly, enumerate); return SetPropertyInt_(obj, name, val, constant, enumerate);
} }
template<typename T> template<typename T>
bool ScriptInterface::GetProperty(JS::HandleValue obj, const char* name, T& out) const bool ScriptInterface::GetProperty(JS::HandleValue obj, const char* name, T& out) const
{ {
JSContext* cx = GetContext(); JSContext* cx = GetContext();
JSAutoRequest rq(cx); JSAutoRequest rq(cx);
JS::RootedValue val(cx); JS::RootedValue val(cx);
Show All 34 Lines
Wildfire Games · Phabricator