Page MenuHomeWildfire Games

Improve the warning message for FromJSValue<CColor>
ClosedPublic

Authored by Imarok on Jun 1 2020, 12:01 PM.

Details

Summary

Make it clear where the warning is coming from when we try to convert a non-object to CColor.

Test Plan

Agree with the improved string.

Event Timeline

Imarok created this revision.Jun 1 2020, 12:01 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/886/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2284/display/redirect

wraitii accepted this revision.Jun 1 2020, 1:00 PM
This revision is now accepted and ready to land.Jun 1 2020, 1:00 PM
Stan added a subscriber: Stan.Jun 1 2020, 1:46 PM

The mac build failiure in unrelated. Itms disabled Mac SVN build until he has some time to address the bug. Linux one is the usual segfault.

Imarok added a comment.Jun 1 2020, 6:02 PM
In D2778#117793, @Stan wrote:

The mac build failiure in unrelated. Itms disabled Mac SVN build until he has some time to address the bug. Linux one is the usual segfault.

Ahm, what usual segfault? :O

Stan added a comment.Jun 1 2020, 6:09 PM

gcc6 segfault :)

This revision was landed with ongoing or failed builds.Jun 1 2020, 6:09 PM
This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.Jun 1 2020, 6:09 PM
elexis added a subscriber: elexis.Jun 1 2020, 7:34 PM

Make it clear where the warning is coming from when we try to convert a non-object to CColor.

It's the purpose of the JS stack to indicate where it's coming from. That only fails to indicate it when there are multiple FromJSVal<CColor> calls in the same line I suppose?
So I wonder how the error could be triggered to begin with and whether there is a stack at all. (FAIL function includes JS_ReportError but I've seen implementations that made the error reporter go silent).
I tried Engine.GetGUIObjectByName("gameDescription").textcolor=X; with X in { r: 33, g: 44, b: 55, a:0 }, 42and "11 22 33" .
I only got:

ERROR: JavaScript error: gui/gamesetup/gamesetup.js line 44
TypeError: invalid arguments
ERROR: GUI page 'page_gamesetup.xml': Failed to call init() function

That string seems to be generic from SM https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Typed_array_invalid_arguments
So for this specific way to trigger a invalid-color-argument warning, we get a JS stack which is good. But I don't see the changed string that could enable developers to see which object assignment in that line failed.

The only explicit call to FromJSVal<CColor> I find with a grep for this string is an occurrence in CGUISetting.cpp which should be the aforementioned way to trigger the warning.
Looking at that code, it seems the ctextobject.textcolor=33 assignment should trigger the error but doesn't?

Searching for FromJSVal( indicates Scripting/JSInterface_IGUIObject.cpp, but that shold be the same code path as mentioned above.
Scripting/GuiScriptConversions.cpp path is only triggered for XML files, is that the way the changed string can be triggered?
(tests/test_GuiManager.h and SettingTypes/CGUISize.cpp seem unrelated, so I don't find another way to trigger the message with the limited timeeffort I spent.)
As you changed this specific string, I suppose you found a way to trigger that string and were unsatisfied with the string, so it should be possible to mention how it can be triggered without disproportionate effort?