Sketching out an improved GUI2 API

Discussion among members of the development team.

Moderator: Forum Moderators

Post Reply
User avatar
Celtic_Minstrel
Developer
Posts: 2166
Joined: August 3rd, 2012, 11:26 pm
Location: Canada
Contact:

Sketching out an improved GUI2 API

Post by Celtic_Minstrel »

The existing Lua GUI2 API for custom dialogs is very procedural and not very well-designed. It was only ever intended to be temporary anyway, from what I understand, and people have kept patching it and patching it, adding new features rather than fixing the design flaws. (Every other GUI2 function has been moved to the new gui module, but the custom dialog functions still remain in wesnoth due to this.)

Basically, I want to create a new and improved API for 1.16.

The current API consists of the monolithic wesnoth.show_dialog along with a host of support functions that don't even work unless a dialog is currently being shown. The following summarizes these functions.
  • get_dialog_value(path) — This is vague; what does "value" actually mean? The correct answer is "it depends on what type of widget this is", but… why!?
  • set_dialog_value(value, path) — Same complaint as above.
  • set_dialog_tooltip(tooltip, path)
  • set_dialog_active(enabled?, path) — Leaking implementation naming conventions that don't really fit. (Actually, I'm not sure if it's even correct for this to be a boolean; do some widgets have more than two states here?)
  • set_dialog_visible(visible?, path)
  • set_dialog_focus(path) — Thanks to all the functions beginning with "set_dialog_", it's not really obvious that this one affects the dialog itself, more so than the widget. (I mean, it affects both, sure — it sets the widget that has keyboard focus.)
  • set_dialog_markup(use_markup?, path)
  • set_dialog_canvas(index, canvas_content, path) — I feel like this one is a major landmine. I think what it does is allow customizing the appearance of a widget?
  • set_dialog_callback(function, path) — Has the same problem as the value functions — the meaning of the callback depends on the type of widget.
  • add_dialog_tree_node(type, index, path) — The tree view is the only widget to have a dedicated function for it.
  • remove_dialog_item(index, count, path) — Removes an item from any compound widget… but wait, how do you add one to a compound widget other than a treeview? I think the answer is set_dialog_value but I'm not quite sure…
Beyond problems with specific functions, you might also notice that every function takes a path as its final parameter. This path refers to the widget being modified, which most likely seems weird to anyone used to assignment syntax. You'd expect the path to come first, not last, right? The other issue is that none of these functions actually do anything if you just call them — they only work in the pre_show or post_show functions passed to show_dialog and in any callback functions passed to set_dialog_callback.

The last problem is that the GUI2 WML needs to be specified in Lua. Well… that's actually a lie. It was true in 1.12, and probably 1.14, but in 1.15.x there is a wml.load function would would let you load it from an actual file, if you prefer. It still doesn't support the automatic resolution-selection of general GUI2, though.

So the general way of writing a custom GUI2 dialog with the API is kinda like this:

Code: Select all

local result = "Your Name"
wesnoth.show_dialog(
	wml.load "my_dialog_defn.cfg", -- Most people just put the WML here for obvious reasons, but I'm doing this to shorten the example code.
	function --[[pre_show]]()
		-- Set the dialog's initial values, callbacks, etc
		wesnoth.set_dialog_value(result, "path", "to", "name_text_field")
		wesnoth.set_dialog_callback(function()
			-- Note: This is illustrative, not necessarily valid - I'm not sure if the current system actually treats the callback for a text field as an "on change" callback.
			if #wesnoth.get_dialog_value("path", "to", "name_text_field") > 15 then
				gui.alert("That name is too long!")
			end,
			"path", "to", "name_text_field"
		)
	end,
	function --[[post_show]]()
		-- Retrieve any important values from the dialog and save them elsewhere
		result = wesnoth.get_dialog_value("path", "to", "name_text_field")
	end
)
For a first sketch, I think it would be nicer if it could be written like this:

Code: Select all

local result = "Your Name"
local dialog = gui.create_dialog(wml.load "my_dialog_defn.cfg")

function dialog:pre_show()
	local text_field = self("path", "to", "name_text_field")
	text_field.content = result
	function text_field:on_change()
		if #self.content > 15 then
			gui.alert("That name is too long!")
		end
	end
end

function dialog:post_show()
	result = self("path", "to", "name_text_field").content
end

dialog:show()
But perhaps that could be improved even more. Does anyone have suggestions?

I think I'll also need to come up with examples for some slightly more complicated things — at the very least, a simple tree view example and a simple list box example. If someone has some existing (not-too-complicated) Lua GUI2 code that they'd like to post, I can post how It think it could look in a better API like this example. (Just the pre/post show functions, I don't need to see the entire dialog definition.)
Author of The Black Cross of Aleron campaign and Default++ era.
Former maintainer of Steelhive.
User avatar
Elvish_Hunter
Posts: 1575
Joined: September 4th, 2009, 2:39 pm
Location: Lintanir Forest...

Re: Sketching out an improved GUI2 API

Post by Elvish_Hunter »

Celtic_Minstrel wrote: December 6th, 2019, 2:18 am But perhaps that could be improved even more. Does anyone have suggestions?
Not really a suggestion, but just a reminder: currently there's no way to use [menu_button] widgets from Lua, because set_dialog_value() can't populate them. We talked about this here: viewtopic.php?f=58&t=28320&start=90#p633507
Celtic_Minstrel wrote: December 6th, 2019, 2:18 am I think I'll also need to come up with examples for some slightly more complicated things — at the very least, a simple tree view example and a simple list box example.
I never used treeviews in WML/Lua (and one of the few situations where I can imagine them being used is anything involving filesystem manipulation, which we don't do in Wesnoth), but if you want a very generic example of listbox that's supposed to be used by UMC authors there's the [choice_box] dialog in the WLP.
Current maintainer of these add-ons, all on 1.16:
The Sojournings of Grog, Children of Dragons, A Rough Life, Wesnoth Lua Pack, The White Troll (co-author)
gfgtdf
Developer
Posts: 1432
Joined: February 10th, 2013, 2:25 pm

Re: Sketching out an improved GUI2 API

Post by gfgtdf »

I agree that the current gui2 API is quite bad mostly because of the things you already said about get/set_dialog_value/callback.

The add-on worldconquestII used treeviews quite a lot mostly in a help-manu like fashion where on the left you have a treeview and on the left a Multipage with additional information about the entry.


set_dialog_canvas is quite powerful, in particular it.basicually turns a panel into a useful 'paint tool' that is you could for example draw a custom map for a scenario, or maybe draw some status bars etc. . Quite sure that era of high sorcery does that.

set_dialog_active: I'm not sure what you mean here iirc this one just sets whtger a widget is 'greyed out' or not and the name (active) seems quite fitting to me.
Scenario with Robots SP scenario (1.11/1.12), allows you to build your units with components, PYR No preperation turn 1.12 mp-mod that allows you to select your units immideately after the game begins.
User avatar
Iris
Site Administrator
Posts: 6797
Joined: November 14th, 2006, 5:54 pm
Location: Chile
Contact:

Re: Sketching out an improved GUI2 API

Post by Iris »

gfgtdf wrote: December 6th, 2019, 2:07 pm set_dialog_active: I'm not sure what you mean here iirc this one just sets whtger a widget is 'greyed out' or not and the name (active) seems quite fitting to me.
The original developer of GUI2 made some questionable naming decisions and this is one I have to agree with celmin on. Anyone who hasn't had the misfortune of having to read through the full C++ of GUI2 would be forgiven for interpreting "active" as referring as to whether a widget has the focus or not.

(Incidentally, the focusedness of a widget is an even more complicated design detail since a widget can have either mouse focus or keyboard focus or both independently of anything else as long as it is enabled/active.)
Author of the unofficial UtBS sequels Invasion from the Unknown and After the Storm.
gfgtdf
Developer
Posts: 1432
Joined: February 10th, 2013, 2:25 pm

Re: Sketching out an improved GUI2 API

Post by gfgtdf »

Hmm okay so what'd be you name suggestion?
Scenario with Robots SP scenario (1.11/1.12), allows you to build your units with components, PYR No preperation turn 1.12 mp-mod that allows you to select your units immideately after the game begins.
User avatar
Celtic_Minstrel
Developer
Posts: 2166
Joined: August 3rd, 2012, 11:26 pm
Location: Canada
Contact:

Re: Sketching out an improved GUI2 API

Post by Celtic_Minstrel »

It should be called "enabled". That's what pretty much every other GUI framework calls it.
Author of The Black Cross of Aleron campaign and Default++ era.
Former maintainer of Steelhive.
gfgtdf
Developer
Posts: 1432
Joined: February 10th, 2013, 2:25 pm

Re: Sketching out an improved GUI2 API

Post by gfgtdf »

Ok I see no problem.witg that either.

As not so complicated treeview example can be found here. https://github.com/gfgtdf/World_Conques ... dialog.lua

Also I am wondering whether we should allow directly acceccing child.widgets via __index, so that self("path", "to", "name_text_field") would become self.path.to.name_text_field, I think.wpuld feel more.natural.but the disadvantage would.be that the IDs of widgets would not be allowed to use names that are already in use for widget functions/properties
Scenario with Robots SP scenario (1.11/1.12), allows you to build your units with components, PYR No preperation turn 1.12 mp-mod that allows you to select your units immideately after the game begins.
User avatar
Celtic_Minstrel
Developer
Posts: 2166
Joined: August 3rd, 2012, 11:26 pm
Location: Canada
Contact:

Re: Sketching out an improved GUI2 API

Post by Celtic_Minstrel »

Yeah, I can see that feeling a little more natural, but I'm not sure the conflict problem is solvable. (Given that there's no way for __index to distinguish between dot access and colon access.) I don't think the __call version is all that unnatural either, though.

Another thing I'm wondering is to what extent it would be possible to access properties, set callbacks, etc at the global scope (rather than in pre_show). From the C++ perspective, I think this would be equivalent to post_build which isn't used much in existing dialogs, so I'm not sure what kind of limitations it might have compared to doing the work in pre_show?
Author of The Black Cross of Aleron campaign and Default++ era.
Former maintainer of Steelhive.
gfgtdf
Developer
Posts: 1432
Joined: February 10th, 2013, 2:25 pm

Re: Sketching out an improved GUI2 API

Post by gfgtdf »

Celtic_Minstrel wrote: December 7th, 2019, 3:04 am but I'm not sure the conflict problem is solvable.
Well the most obvious way to solve it would.be to forbid the use of these names as widget ids
Scenario with Robots SP scenario (1.11/1.12), allows you to build your units with components, PYR No preperation turn 1.12 mp-mod that allows you to select your units immideately after the game begins.
gfgtdf
Developer
Posts: 1432
Joined: February 10th, 2013, 2:25 pm

Re: Sketching out an improved GUI2 API

Post by gfgtdf »

Celtic_Minstrel wrote: December 7th, 2019, 3:04 am
Another thing I'm wondering is to what extent it would be possible to access properties, set callbacks, etc at the global scope (rather than in pre_show).
With the 'dialog object' is somehow makes sense to assume (from the umc authors point if view) that it's functions can be called anytime. In particular also that the show () function can be called multiple times, one would think that registering callbacks in preshow would then register these callbacks twice(assuming they are not on a dynamically created widget) when show is called twice, and that for this reason setting callbacks ( except for dynamically created widgets) _should_ be set outside preshow.
Scenario with Robots SP scenario (1.11/1.12), allows you to build your units with components, PYR No preperation turn 1.12 mp-mod that allows you to select your units immideately after the game begins.
User avatar
Celtic_Minstrel
Developer
Posts: 2166
Joined: August 3rd, 2012, 11:26 pm
Location: Canada
Contact:

Re: Sketching out an improved GUI2 API

Post by Celtic_Minstrel »

Indeed - if it actually works, I'd probably prefer that at least for callbacks.
Author of The Black Cross of Aleron campaign and Default++ era.
Former maintainer of Steelhive.
gfgtdf
Developer
Posts: 1432
Joined: February 10th, 2013, 2:25 pm

Re: Sketching out an improved GUI2 API

Post by gfgtdf »

Also post_show might not be needed anymore aswell, I mean you can probably just query these values from the dialog after calling show ()
Scenario with Robots SP scenario (1.11/1.12), allows you to build your units with components, PYR No preperation turn 1.12 mp-mod that allows you to select your units immideately after the game begins.
Post Reply