Page MenuHomeWildfire Games

Resize (XML object) bar JS GUI addon
Needs ReviewPublic

Authored by nani on Apr 14 2019, 12:19 AM.

Details

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

Makes possible to change some XML object size with the mouse a.k.a resize.
Has animations included but can be removed with ease.
It's easy to add to the current codebase and adds minimum complexity.

Depends on D1702

Test Plan

No real test, just see it works.
Alternative to implement it in c++. How? still pending to know.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 7187
Build 11715: arc lint + arc unit

Event Timeline

nani created this revision.Apr 14 2019, 12:19 AM

@vladislavbelov should this code not be in other source/gui/ code where all the other GUI code is, for two reasons (1) cohesion and (2) performance (onTick)?
About (1), I suspect C++ has access to things that JS doesn't have access, but would either required or useful to be accessible / interactable / configurable in this context.
But I can't make a conclusion without having seen the thing in action.

binaries/data/mods/mod/gui/common/ResizeBar.js
9

(still no .0)

16

{}

44

Still no to these 'compatibility' functions. Let the caller be explicit, always accept an object, or always accept a string

58

each case on a separate line

66

sideC, side0, sideX don't inform the reader what C 0 X mean

147

one statement per line, bar => {\n...
statements end with a semicolon: hidden = true;\n}

157

one statement per line if (x)\n

163

no else after return

216

A resizeBar and a ResizeBar object sound confusing.

246

if?

nani updated this revision to Diff 7749.Apr 14 2019, 4:29 PM
Owners added a subscriber: Restricted Owners Package.Apr 14 2019, 4:29 PM
wraitii added a reviewer: Restricted Owners Package.Apr 22 2019, 9:30 AM
wraitii added a subscriber: wraitii.

This does sound like a 'core' c++ feature to me, but having a JS implementation first and then possibly porting it to C++ sounds like an interesting approach to me.

However I agree that the two resizeBars are a bit confusing and I'd put a full "example usage" on top of the component.

nani added a comment.May 10 2019, 5:19 PM

However I agree that the two resizeBars are a bit confusing and I'd put a full "example usage" on top of the component.

I thought it was pretty standard having the class definition uppercased and the instance of such class lowercased.

As for the example, will add it shortly.

nani marked 10 inline comments as done.May 10 2019, 5:20 PM
nani added inline comments.
binaries/data/mods/mod/gui/common/ResizeBar.js
216

naming checks

nani updated this revision to Diff 7962.May 10 2019, 8:58 PM
nani marked an inline comment as done.

Added example.

This does sound like a 'core' c++ feature to me, but having a JS implementation first and then possibly porting it to C++ sounds like an interesting approach to me.

Is C++ the correct place to implement the feature idea or not?

elexis edited the summary of this revision. (Show Details)May 10 2019, 9:28 PM
In D1825#77536, @elexis wrote:

This does sound like a 'core' c++ feature to me, but having a JS implementation first and then possibly porting it to C++ sounds like an interesting approach to me.

Is C++ the correct place to implement the feature idea or not?

Because if C++ is the right place to have it, then committing it in JS means that writing it in C++ is deincentivized as compared to not committing it in JS.

On the other hand we have JS because it makes coding much easier and quicker.
So if there is no performance issue of onTick and determined not to be a wrong layer of of abstraction, then JS would be ideal and porting to C++ later would be wrong.

If JS was the better choice, then one may consider allowing the developers to specify JS GUI object types in JS instead of C++, somehow (JSInterface_IGUIObject.cpp). Such a formalization might or might not have some advantages.

(Perhaps some part of it would be better in C++ and some other part in JS, hypothetically...)

binaries/data/mods/mod/gui/common/ResizeBar.js
100

sideC -> sideComplementary
sideO -> sideOpposite
sideX -> sideOppositeComplementary
(or more expressive titles)

135

ScriptInterface.cpp provides a clone() function

283
nani updated this revision to Diff 7963.May 10 2019, 10:36 PM
nani marked 3 inline comments as done.

Because if C++ is the right place to have it, then committing it in JS means that writing it in C++ is deincentivized as compared to not committing it in JS.

I don't necessarily agree (see perf below). Furthermore, overall workforce is limited. If we have this in JS and it's not ideal but it works well enough, people who would have written c++ will do something else, and overall we'll have more features. That could be better.

On the other hand we have JS because it makes coding much easier and quicker.
So if there is no performance issue of onTick and determined not to be a wrong layer of of abstraction, then JS would be ideal and porting to C++ later would be wrong.

Performance issues depend on usage. What is quick enough now may become too slow in the future.


Regardless of the above, I wonder if this is the first such feature for which we are popping an object at runtime from the global nether. That seems like a different direction from our current GUI items.

nani added a comment.May 11 2019, 2:23 PM
In D1825#77538, @elexis wrote:
In D1825#77536, @elexis wrote:

This does sound like a 'core' c++ feature to me, but having a JS implementation first and then possibly porting it to C++ sounds like an interesting approach to me.

Is C++ the correct place to implement the feature idea or not?

Because if C++ is the right place to have it, then committing it in JS means that writing it in C++ is deincentivized as compared to not committing it in JS.

On the other hand we have JS because it makes coding much easier and quicker.
So if there is no performance issue of onTick and determined not to be a wrong layer of of abstraction, then JS would be ideal and porting to C++ later would be wrong.

If JS was the better choice, then one may consider allowing the developers to specify JS GUI object types in JS instead of C++, somehow (JSInterface_IGUIObject.cpp). Such a formalization might or might not have some advantages.

(Perhaps some part of it would be better in C++ and some other part in JS, hypothetically...)

If you want a reason as to why I've written it in Javascirpt:

  • It was originally written for my mod, so asking people to compile 0ad just for this wasn't an option.
  • For all intended purposes works just as fine as a C++ implementation.
  • Is faster to change if needed to (for the community of modders at least).
  • The interface calls to the engine are kept to the minimum, I haven't seen any performance loss worth the mention (at least in few numbers).
  • Easily extendable.
  • Can be later (or never as elexis says :P) move to C++ if someone stills feels itchy.
nani added a comment.May 11 2019, 2:28 PM

Regardless of the above, I wonder if this is the first such feature for which we are popping an object at runtime from the global nether. That seems like a different direction from our current GUI items.

Added to global.xml to minimize friction and maximize usefulness (having to add a resize bar xml definition in each page folder seems suboptimal though with a c++ implementation this could be entirely omitted)

If we have this in JS and it's not ideal but it works well enough, people who would have written c++ will do something else

If it's not ideal in JS, then someone should write it in C++, so the people who would have written it in C++ should be incentivized to do that, rather than deincentivized to?

and overall we'll have more features. That could be better.

Stacking problems usually ends up in convolution and our task is to solve problems, not to make more of them.
So the question is which parts of the code are a defect to have in C++ and which not.
If it's good in JS, then it'd be wrong to have it in C++ (rather than a tolerable evil).

In D1825#77564, @nani wrote:

If you want a reason as to why I've written it in Javascirpt:

  • It was originally written for my mod, so asking people to compile 0ad just for this wasn't an option.

I know, but it does not follow from that that it's the right thing to do for Pyrogenesis.

  • For all intended purposes works just as fine as a C++ implementation.

All intended purposes of alpha 23, but also all intended purposes of the Pyrogenesis engine (i.e. all mods, all future use cases too)?
As wraitii says, we might add a lot more complexity later.

  • Is faster to change if needed to (for the community of modders at least).
  • Easily extendable.

That's a full +1 for JS, that's why Pyrogenesis uses JS to begin with.

  • The interface calls to the engine are kept to the minimum, I haven't seen any performance loss worth the mention (at least in few numbers).

Depends on a hypothetical C++ implementation. I imagine it might be possible to create a resize-bar or drag GUI Object type that only needs to be defined in XML and then has zero interface calls (in particular no onTick events).

  • Can be later (or never as elexis says :P) move to C++ if someone stills feels itchy.

The question is what the ramifications are of making it in JS vs C++, what use cases are conceivable (not only the one use case we have now, but a class of use cases), whether it will be a step into the right direction or a step into the wrong direction which happens to cover all use cases we have now.

We might want to take a look at how other GUI engines support resize elements instead of reinventing the wheel.
(Btw you spoke about the idea of supporting a HTML/CSS engine, I've seen Philip had the same idea too, but I forgot where it was mentioned, and it will consume several weeks to months of work to transform everything.)

Anyhow, if JS was good, we may consider defining GUI object types in JS instead of C++, i.e. one could add <object type="myJSPrototype".... At least if we can conceive the creation of more than one JS GUI Object type.

Regardless of the above, I wonder if this is the first such feature for which we are popping an object at runtime from the global nether. That seems like a different direction from our current GUI items.

I think if we want resizable and movable dialogs, then most of them should be? For example the diplomacy dialog and chat window. Having two resizers on the same GUI page is not so remote.

(Also, a screenshot might illustrate the use case to the users who didn't test the patch stack.)