Page MenuHomeWildfire Games
Differential D844 Diff 7310 source/gui/scripting/JSInterface_IGUIObject.cpp

Changeset View

Standalone View

View Options

source/gui/scripting/JSInterface_IGUIObject.cpp

/* Copyright (C) 2017 Wildfire Games. /* Copyright (C) 2019 Wildfire Games.
* This file is part of 0 A.D. * This file is part of 0 A.D.
* *
* 0 A.D. is free software: you can redistribute it and/or modify * 0 A.D. is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by * it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 2 of the License, or * the Free Software Foundation, either version 2 of the License, or
* (at your option) any later version. * (at your option) any later version.
* *
* 0 A.D. is distributed in the hope that it will be useful, * 0 A.D. is distributed in the hope that it will be useful,
Show All 35 Lines
}; };
JSFunctionSpec JSI_IGUIObject::JSI_methods[] = JSFunctionSpec JSI_IGUIObject::JSI_methods[] =
{ {
JS_FS("toString", JSI_IGUIObject::toString, 0, 0), JS_FS("toString", JSI_IGUIObject::toString, 0, 0),
JS_FS("focus", JSI_IGUIObject::focus, 0, 0), JS_FS("focus", JSI_IGUIObject::focus, 0, 0),
JS_FS("blur", JSI_IGUIObject::blur, 0, 0), JS_FS("blur", JSI_IGUIObject::blur, 0, 0),
JS_FS("getComputedSize", JSI_IGUIObject::getComputedSize, 0, 0), JS_FS("getComputedSize", JSI_IGUIObject::getComputedSize, 0, 0),
JS_FS("getTextSize", JSI_IGUIObject::getTextSize, 0, 0),
JS_FS_END JS_FS_END
}; };
bool JSI_IGUIObject::getProperty(JSContext* cx, JS::HandleObject obj, JS::HandleId id, JS::MutableHandleValue vp) bool JSI_IGUIObject::getProperty(JSContext* cx, JS::HandleObject obj, JS::HandleId id, JS::MutableHandleValue vp)
{ {
JSAutoRequest rq(cx); JSAutoRequest rq(cx);
ScriptInterface* pScriptInterface = ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface; ScriptInterface* pScriptInterface = ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface;
Show All 15 Linesbool JSI_IGUIObject::getProperty(JSContext* cx, JS::HandleObject obj, JS::HandleId id, JS::MutableHandleValue vp)
// partly for efficiency, and also to allow correct reporting of attempts to // partly for efficiency, and also to allow correct reporting of attempts to
// access nonexistent properties.) // access nonexistent properties.)
if (propName == "constructor" || if (propName == "constructor" ||
propName == "prototype" || propName == "prototype" ||
propName == "toString" || propName == "toString" ||
propName == "toJSON" || propName == "toJSON" ||
propName == "focus" || propName == "focus" ||
propName == "blur" || propName == "blur" ||
propName == "getTextSize" ||
wraitiiUnsubmitted
Not Done
Inline Actions

FYI, I'm somewhat certain this will be useless in SM45.

wraitii: FYI, I'm somewhat certain this will be useless in SM45.
s0600204AuthorUnsubmitted
Not Done
Inline Actions

In what way?

s0600204: In what way?
propName == "getComputedSize" propName == "getComputedSize"
) )
return true; return true;
// Use onWhatever to access event handlers // Use onWhatever to access event handlers
if (propName.substr(0, 2) == "on") if (propName.substr(0, 2) == "on")
{ {
CStr eventName(CStr(propName.substr(2)).LowerCase()); CStr eventName(CStr(propName.substr(2)).LowerCase());
▲ Show 20 Lines • Show All 593 Lines • ▼ Show 20 Lines if (!e)
return false; return false;
e->GetGUI()->SetFocusedObject(NULL); e->GetGUI()->SetFocusedObject(NULL);
rec.rval().setUndefined(); rec.rval().setUndefined();
return true; return true;
} }
bool JSI_IGUIObject::getTextSize(JSContext* cx, uint argc, JS::Value* vp)
wraitiiUnsubmitted
Not Done
Inline Actions

This looks like it could be const-ed.

wraitii: This looks like it could be const-ed.
{
JSAutoRequest rq(cx);
JS::CallReceiver rec = JS::CallReceiverFromVp(vp);
JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
elexisUnsubmitted
Done
Inline Actions

https://bugzilla.mozilla.org/show_bug.cgi?id=1255800 So it's not yet deprecated and not part of the SM patches. (Got a patch somewhere, it's a 2-3 line replacement to nuke the macro.)

elexis: https://bugzilla.mozilla.org/show_bug.cgi?id=1255800 So it's not yet deprecated and not part of…
elexisUnsubmitted
Done
Inline Actions

JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
JS::RootedObject thisObj(cx, &args.thisv().toObject());

elexis: JS::CallArgs args = JS::CallArgsFromVp(argc, vp); JS::RootedObject thisObj(cx, &args.thisv().
elexisUnsubmitted
Not Done
Inline Actions

I suppose I took that from JSI_IGUIObject::construct

https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_reference/JS::CallArgs

https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_reference/JS_InitClass

Unlike other JSNatives, this function must not call JS_THIS or JS_THIS_OBJECT. Instead it must create a new object and return it.

The rest / macro uses the CallReceiver (CallArgs inherits CallReceiver). (So it doesn't seem wrong, just inconsistent.)

elexis: I suppose I took that from `JSI_IGUIObject::construct` https://developer.mozilla.org/en…
JS::RootedObject thisObj(cx, &args.thisv().toObject());
IGUIObject* obj = (IGUIObject*)JS_GetInstancePrivate(cx, thisObj, &JSI_IGUIObject::JSI_class, NULL);
if (!obj || !obj->SettingExists("caption"))
return false;
CStrW font;
if (GUI<CStrW>::GetSetting(obj, "font", font) != PSRETURN_OK || font.empty())
font = L"default";
CGUIString caption;
EGUISettingType Type;
obj->GetSettingType("caption", Type);
if (Type == GUIST_CGUIString)
// CText, CButton, CCheckBox, CRadioButton
GUI<CGUIString>::GetSetting(obj, "caption", caption);
else if (Type == GUIST_CStrW)
{
// CInput
CStrW captionStr;
GUI<CStrW>::GetSetting(obj, "caption", captionStr);
caption.SetValue(captionStr);
}
else
elexisUnsubmitted
Not Done
Inline Actions

Was wondering if it wouldn't be cleaner to call into a function of the specific GUIObject type class that returns the CGUIString caption or getTextSize. But since it doesn't actually hardcode objecttypes here and since there are only 2 or 3 string types, seems okay.
But in general there might be new GUIObject types added that want to return something other than a property of the caption when getTextSize is called. What about dropdowns for instance?
(It sounds like we might want to introduce one JSClass for every GUIObject class that may inherit some general functions)

elexis: Was wondering if it wouldn't be cleaner to call into a function of the specific GUIObject type…
s0600204AuthorUnsubmitted
Not Done
Inline Actions

There's a part of me that wonders if its not possible to use m_GeneratedTexts of IGUITextOwner (which is a parent-class to all gui objects that display text except CInput) somehow... but working out how to do that seems beyond me.

Perhaps a method of IGUITextOwner could iterate through the generated texts of the object in question, add the text sizes together, then return the result. But that's either not possible, or I'm just not sufficiently proficient in c++ and/or spidermonkey peculiarities to work out how to get the IGUITextOwner of a selected object. (Most likely describing it incorrectly, too.)

s0600204: There's a part of me that wonders if its not possible to use `m_GeneratedTexts` of…
return false;
obj->UpdateCachedSize();
wraitiiUnsubmitted
Not Done
Inline Actions

I'm assuming this is why you can't. Maybe it'd be worth consting those and making the cached variable mutable (for another patch)

wraitii: I'm assuming this is why you can't. Maybe it'd be worth consting those and making the cached…
float width = obj->m_CachedActualSize.GetWidth();
if (obj->SettingExists("scrollbar"))
{
bool scrollbar;
GUI<bool>::GetSetting(obj, "scrollbar", scrollbar);
if (scrollbar)
{
CStr scrollbar_style;
GUI<CStr>::GetSetting(obj, "scrollbar_style", scrollbar_style);
vladislavbelovUnsubmitted
Done
Inline Actions

The issue still presents: GetScrollBarStyle could return nullptr, then something bad may happen. Because you get a property of the nullptr object.

vladislavbelov: The issue still presents: GetScrollBarStyle could return `nullptr`, then something bad may…
const SGUIScrollBarStyle* scrollbar_style_object = obj->GetGUI()->GetScrollBarStyle(scrollbar_style);
if (scrollbar_style_object)
width -= scrollbar_style_object->m_Width;
}
}
float buffer_zone = 0.f;
vladislavbelovUnsubmitted
Done
Inline Actions

The friend is only for this line.

Also why you don't check it != map.end()? Currently the code has UB.

vladislavbelov: The `friend` is only for this line. Also why you don't check `it != map.end()`? Currently the…
GUI<float>::GetSetting(obj, "buffer_zone", buffer_zone);
SGUIText text = obj->GetGUI()->GenerateText(caption, font, width, buffer_zone, obj);
JS::RootedValue objVal(cx, JS::ObjectValue(*JS_NewPlainObject(cx)));
try
{
ScriptInterface* pScriptInterface = ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface;
pScriptInterface->SetProperty(objVal, "width", text.m_Size.cx, false, true);
pScriptInterface->SetProperty(objVal, "height", text.m_Size.cy, false, true);
}
catch (PSERROR_Scripting_ConversionFailed&)
{
debug_warn(L"Error creating size object!");
return false;
}
rec.rval().set(objVal);
return true;
elexisUnsubmitted
Not Done
Inline Actions

The copy could be avoided and the function length reduced by extending GUIScriptConversions.cpp for CSize. (Dunno if one can introduce only the ToJSVal without introducing FromJSVal for one type)

elexis: The copy could be avoided and the function length reduced by extending `GUIScriptConversions.
}
bool JSI_IGUIObject::getComputedSize(JSContext* cx, uint UNUSED(argc), JS::Value* vp) bool JSI_IGUIObject::getComputedSize(JSContext* cx, uint UNUSED(argc), JS::Value* vp)
{ {
JSAutoRequest rq(cx); JSAutoRequest rq(cx);
JS::CallReceiver rec = JS::CallReceiverFromVp(vp); JS::CallReceiver rec = JS::CallReceiverFromVp(vp);
JS::RootedObject thisObj(cx, JS_THIS_OBJECT(cx, vp)); JS::RootedObject thisObj(cx, JS_THIS_OBJECT(cx, vp));
IGUIObject* e = (IGUIObject*)JS_GetInstancePrivate(cx, thisObj, &JSI_IGUIObject::JSI_class, NULL); IGUIObject* e = (IGUIObject*)JS_GetInstancePrivate(cx, thisObj, &JSI_IGUIObject::JSI_class, NULL);
if (!e) if (!e)
return false; return false;
e->UpdateCachedSize(); e->UpdateCachedSize();
CRect size = e->m_CachedActualSize; CRect size = e->m_CachedActualSize;
JS::RootedValue objVal(cx, JS::ObjectValue(*JS_NewPlainObject(cx))); JS::RootedValue objVal(cx, JS::ObjectValue(*JS_NewPlainObject(cx)));
try try
{ {
ScriptInterface* pScriptInterface = ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface; ScriptInterface* pScriptInterface = ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface;
pScriptInterface->SetProperty(objVal, "left", size.left, false, true); pScriptInterface->SetProperty(objVal, "left", size.left, false, true);
pScriptInterface->SetProperty(objVal, "right", size.right, false, true); pScriptInterface->SetProperty(objVal, "right", size.right, false, true);
pScriptInterface->SetProperty(objVal, "top", size.top, false, true); pScriptInterface->SetProperty(objVal, "top", size.top, false, true);
pScriptInterface->SetProperty(objVal, "bottom", size.bottom, false, true); pScriptInterface->SetProperty(objVal, "bottom", size.bottom, false, true);
} }
elexisUnsubmitted
Done
Inline Actions

Why are width and height passed here?
They should be the same as right-left and bottom-top, or the different semantics should be reflected by the code by using a different object / reference, no?

elexis: Why are width and height passed here? They should be the same as right-left and bottom-top, or…
catch (PSERROR_Scripting_ConversionFailed&) catch (PSERROR_Scripting_ConversionFailed&)
{ {
debug_warn(L"Error creating size object!"); debug_warn(L"Error creating size object!");
return false; return false;
} }
rec.rval().set(objVal); rec.rval().set(objVal);
return true; return true;
} }
Wildfire Games · Phabricator