The JSGUI Interface defines a constructor for GUIObject from JS. This constructor is never used in js code, and only supports cloning the JS part of a C++ GUI object (since the private data is the same c++ object).
This deletes that and instead errors out, if we ever properly implement this constructor, it can be changed.
Details
Details
- Reviewers
wraitii - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) - Commits
- rP22530: Delete unused broken JSI_IGUIObject::construct.
Compile.
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- temp
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 7656 Build 12484: Vulcan Build Jenkins Build 12483: arc lint + arc unit
Event Timeline
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/1535/display/redirect
| source/gui/scripting/JSInterface_IGUIObject.cpp | ||
|---|---|---|
| 616 | not implemented ? | |
Comment Actions
It was added because it could be, I guess. It's there since 2004, and as far as I can tell hasn't changed much since (SM updates only, basically). The purpose is to have JS code call "new GUIObject", but since it needs to take another GUI object as an argument it seems to be a copy constructor? Only it doesn't copy the c++ object. I'm not sure if it was ever used but it does not seem to be now.
Indeed, forgot that.
Comment Actions
The function should be deleted altogehter, then new GUIObject() will complain without writing code for it.
D1699 has some more information, but I will split that revision into smaller diffs, including this one.
It was added in rP666, it can only be a mistake, because one entirely cannot pass a IGUIObject C++ class instance pointer via JS, nor construct that in JS, nor would make it sense to create such an object.
If someone wanted to have a use case to dynamically insert GUIObjects, it would at least have to be a constructor depending on GUI type, i.e. new GUIDropDown etc..
(That use case is unlikely, right now we specify everything in XML and build the code refering to that, but if someone wants to go for that new code, they should it do it in a way that doesn't break in various ways and allows the UI to segfault for no benefit.)
| source/gui/scripting/JSInterface_IGUIObject.cpp | ||
|---|---|---|
| 74 | These commens are more confusing than helping.
"toJSON" doesnt exist, although it's often defined for JSObjects (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#toJSON()_behavior) and I guess Yves added it for a reason (in rP13238).
So in theory toJSON and prototype would return something defined by SM. One can look into splitting the getter into multiple functions, but not here. | |
| 616 | delete function completely, this error is redundant | |