HomeWildfire Games

Add a `GetTextSize()` method to GUI objects
AuditedrP22134

Description

Add a GetTextSize() method to GUI objects

Usage:

let size = Engine.GetGUIObjectByName({gui_object_name}).GetTextSize()

Returns a JS object containing the height and width of the primary text field within
the object, taking into account new lines, text wrapping, and font changes.

Unless the object doesn't contain text, in which case the method will return undefined.

Commented on by: vladislavbelov
Additional code by: elexis
Reviewed By: wraitii
Differential Revision: https://code.wildfiregames.com/D844

Event Timeline

elexis added a subscriber: elexis.Mar 19 2019, 2:15 PM

Unless the object doesn't contain text, in which case the method will return undefined`

I thought you mean empty string (that should return 0 size instead of undefined, because the size is known for empty string),
but you mean absence of a caption property from what I gather.

let size = Engine.GetGUIObjectByName({gui_object_name}).GetTextSize()

Looking at the implementation, I guess it's GetCaptionSize(), since it doesn't work for objects that store text but not in the caption property.
It would be good if the reader wouldn't have to know about the implementation of every GUI object type to determine which GUI object types have that function available.
It's the first function that is available to every GUI obejct type but only some of them support it - all the other functions work with all GUI object types, no?
Same mentionings in D1781 (proposal of a new function given to any GUI Object but only some of the GUI object types can use it).
The more functions we add to this file, the more fuzzy the file becomes. The specific logic of specific GUI obejct types leaks into this file. This means the file can't be read anymore without preknowledge about the other GUI object types, making it harder to understand and maintain.
The JSInterface should be restructured, so that it only contains general purpose functions, and the special purpose functions would be moved to the special purpose GUIObject types.
Correcting that before adding further new functions would prevent having to relocate that code later (edit minimization principle).

Searching the code to determine which GUI obejcts (implementing IGUIObject) can actually use this new function, it's CButton, CCheckbox, CInput, CProgressBar, CText, CTooltip.
GUI Object types that exclude it are CChart, CDropdown, CList, COList, CSlider, Minimap (did I find all GUI Object types?).

So the function is used by half of the GUI object types and not used by the other half
(as opposed to addItem from D1781 which only works for one of the at least 12 GUI object types).

Creating a separate JSInterface file for every GUI obejct type solely for this function seems like adding noise at first.

But on the other hand, having the JSI for every GUI object type also establishes a space where developers can add new specific function.

It's not obvious that having more specific JSInterfaces is a possibility and I suspect most people who look at this file at first never get the idea.
Having it implemented would open up the imagination to these developers to add new method that are supported only by some GUI Objects.
I'm sure there are use cases for JSInterface methods that only work for sliders, dropdowns, tables, progress bars, minimaps and so forth.

While for D1781, it's obvious that it would be useful to have in a new JSinterface_CList.cpp or similar, it's not so obvious for this more common function.

To avoid duplication, the common functions may still be defined only once.
That would mean JSInterface_GUIObjectCaption for this one? Still a bit weird - better categorize the JS GUI Interfaces by the GUI object types, so that the reader can deduce from the object he wants to operate with which files are relevant to modify.
Needs some exploration and creativity to determine the ideal.

The code of the function itself looks clean and I recall that we need this JSI function to autosize the entity template information dialog, so thanks for the furtherance of that cause!

/ps/trunk/source/gui/IGUIScrollBarOwner.cpp
58

Wondering whether a LOGERROR would be consistent and/or reasonable. Equally for the case where the new function is called but without complaining if the GUI object type does not use the "caption" property.

elexis raised a concern with this commit.Jul 30 2019, 5:40 PM

Some more inline comments I posted in https://code.wildfiregames.com/D844#89182 after noticing that its the revision proposal, not the commit. To me it seems cleaner to rewrite all of this.

This commit now has outstanding concerns.Jul 30 2019, 5:40 PM

Proposed fix: D2136

elexis accepted this commit.Aug 2 2019, 6:58 PM

Thanks for the feature (template viewer + engine changes)!

All concerns with this commit have now been addressed.Aug 2 2019, 6:58 PM