Page MenuHomeWildfire Games

Don't pretend we implement a JS constructor for GUIObject.
Needs ReviewPublic

Authored by wraitii on Tue, May 28, 6:45 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

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.

Test Plan

Compile.

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 BuildJenkins
Build 12483: arc lint + arc unit

Event Timeline

wraitii created this revision.Tue, May 28, 6:45 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1535/display/redirect

wraitii added a subscriber: Itms.Tue, May 28, 7:58 PM

@Itms Possibly of interest to you.

Stan added a subscriber: Stan.Tue, May 28, 7:59 PM
Stan added inline comments.
source/gui/scripting/JSInterface_IGUIObject.cpp
616

not implemented ?

Do we have places where the removed code might be useful? Why it was added?

elexis added a subscriber: elexis.Sun, Jun 2, 1:43 AM

Some overlap with D1699

Do we have places where the removed code might be useful? Why it was added?

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.

In D1933#80581, @elexis wrote:

Some overlap with D1699

Indeed, forgot that.