Add a dir() function to improve API discoverability #5818

CelticMinstrel · 2021-05-29T15:32:02Z

This is intended to allow someone to explore the API right in the Lua console. The current commit implements the actual function, but there's a lot more work to support it throughout the API, which is why this is a draft. I don't expect this to be finished for 1.16.

stevecotton · 2021-06-03T01:35:37Z

Does this interact with the already-implemented tab completion?

CelticMinstrel · 2021-06-03T03:31:09Z

It currently doesn't… I was thinking that unifying them might be nice though.

stevecotton · 2021-07-24T10:43:31Z

src/scripting/lua_kernel_base.cpp

+ * A list of keys is gathered from the following sources:
+ * - For a table, all keys defined in the table
+ * - Any keys accessible through the metatable chain (if __index on the metatable is a table)
+ * - The output of the __dir metafunction


How will this filter out deprecated stuff?

Replying to #5965 (comment) :

Renaming something because the old name is misleading is an accomplishment in and of itself

Removing the old name is an accomplishment, but merely adding a new name gives people more things to look at. It will improve the readability of code that uses the new name, but its affect on the task of writing new code is not always beneficial.

That's a good point. For userdata types that rely on a __dir metafunction, they just wouldn't add those keys, but that won't help for tables. Perhaps I could add a __dir_exclude metafunction/table, which lists keys to exclude from the dir listing even if they exist in the table. This metafunction would need to be checked all the way up the metatable chain. (Actually, maybe even __dir should be?)

I implemented something a bit different in the "Exclude deprecated things from dir()" commit (not direct-linking it because the hash could evolve as I force-push). Does that look sufficient to you? There could in theory be a few deprecated things that are caught by that system but they should be rare.

The implementation is likely inefficient, but as this is intended for use in the Lua console to discover API (particularly experimental API), I don't think that matters much.

CelticMinstrel · 2022-06-05T04:36:51Z

Note: I merged part of new_rendering in so I could actually test stuff. That'll be merged to master before this is done and I'll force-push it into oblivion at that time, so just ignore those extra commits.

The purpose is to simply list everything that's available in a given module or object. Currently, it only works for tables. The plan is to extend it to userdata objects if and when it is approved.

This places all read-only functions straight into the AI table, which means it sticks when stdlib.lua nulls out the old move map API. The AI table then implements the __dir metamethod to add the mutating functions in if read_only is false. This also adds a __dir metamethod for the aspects table which gets the list of aspects directly from the engine.

Note that using it on wesnoth.preferences only returns preferences that are currently set and may omit some preferences that are possible but currently unset. However, it DOES include all registered advanced preferences even if they are unset.

…e object

This fetches all known attributes for that widget type and also walks the widget tree to grab the IDs of any child widgets, so it SHOULD get everything. Deprecated keys value_compat and callback are excluded.

- Named tuples - Version - vconfig

src/scripting/lua_color.cpp

src/scripting/lua_kernel_base.cpp

stevecotton · 2022-06-13T06:41:57Z

A name as generic as dir shouldn't be added to the global namespace.

stevecotton · 2022-06-13T08:24:40Z

src/scripting/lua_kernel_base.cpp

+	return 1;
+}
+
+static int impl_get_dir_suffix(lua_State*L)


Are these suffixes a convention, either from Lua or another language?

If they're not a convention: for functions, the suffix () would seem more natural to me than ƒ, although both make sense. I'm not seeing the logic that associates daggers with tables.

General comment on the whole PR: more code-comments would help. Knowing what each function is meant to do makes reviewing both easier and better (because the reviewers don't make as many incorrect assumptions).

I chose dagger for table because it resembles a T, and florin for functions because it resembles an F. An italicized lower-case F (which the florin basically is) is commonly used for functions in mathematics, which influenced my decision.

Just for the record, I don't actually have any special attachment to these suffixes. If someone has better suggestions, I'm open to changing them.

stevecotton · 2022-06-13T08:54:23Z

src/scripting/lua_widget_attributes.cpp

@@ -574,4 +574,47 @@ int impl_widget_set(lua_State* L)
 	ERR_LUA << "invalid modifiable property of '" <<  typeid(w).name()<< "' widget:" << str << "\n";
 	return luaL_argerror(L, 2, "invalid modifiable property of widget");
 }
+
+int impl_widget_dir(lua_State* L)


Is there a way to get to this in the Lua console?

After puzzling about it, I saw that https://wiki.wesnoth.org/LuaAPI/gui/widget says " Since these functions take a widget userdata as the first parameter, they can only be used in the preshow or postshow of a GUI2 dialog (directly or indirectly)". That makes me think it's not accessible from the Lua console, so it's unclear how to test it.

gui.show_dialog works from the Lua console, so I tested it via gui.show_dialog(dialog_wml, function(window) dir(whatever) end).

For my test case I loaded the tutorial character selection dialog into dialog_wml, and I also added an ID to every widget in the tutorial to prove that that aspect of it works correctly.

stevecotton · 2022-06-13T12:05:23Z

data/lua/core/wml.lua

@@ -345,6 +366,7 @@ if wesnoth.kernel_type() ~= "Application Lua Kernel" then

 	local root_variable_mt = {
 		__metatable = "WML variables proxy",
+		__dir = dir_vars(true),


wml.variables is one of the things that tab-completion doesn't work for. I'm wondering if adding tab-completion for it would be a parallel mechanism, and whether it would be better to implement dir on top of that mechanism instead of adding a mechanism that's limited to only the dir function.

I think it would be the other way around – the tab completion could be made to use the dir mechanism instead of what it currently does. That doesn't mean calling the dir() function but rather using the underlying implementation that dir() uses. It also means tab-completion will not reveal deprecated functions, but that might be fine?

Have you already looked at the existing mechanism, and if so please would you give an overview of what existing functions are gaining parallel implementations in this PR?

(I'll have a look for myself, but the review can be quicker if I you give me a quick-start on that.)

The part of intf_object_dir that populates the keys array is basically doing the same thing that lua_kernel_base::get_attributes is doing. If pulled out into a separate function, both intf_object_dir and lua_kernel_base::get_attributes could call that function.

CelticMinstrel · 2022-06-13T17:30:14Z

A name as generic as dir shouldn't be added to the global namespace.

I don't really agree; I feel that the global namespace is precisely where this belongs. It doesn't really fit in the wesnoth namespace or any of our other namespaces. That said, if the namespacing is the only blocker, I'm open to moving it.

stevecotton · 2022-06-14T05:22:12Z

It doesn't really fit in the wesnoth namespace or any of our other namespaces.

Given that it's easy for anyone to alias dir = wesnoth.debugging_utils_or_something.dir, please move it out of global anyway. Having thought it over, I also suggest giving it a name that can be searched for - try typing dir into the forum and wiki's search boxes, and there will be lots of results for other things.

CelticMinstrel · 2022-06-14T12:23:43Z

Um, if you're going to suggest a rename, please give specific suggestions rather than "maybe this isn't really a good name". Otherwise I'm going to stick with the current name.

Note that the name is the same as the WFL function that does the same thing, so I think that's one argument for keeping the name.

CelticMinstrel · 2022-06-16T12:44:50Z

I'm still waiting to see if anyone has any suggestions for a different qualified name for this function. If no-one makes any specific suggestions, I'll just merge at some point with the current name (or maybe moved into wesnoth namespace since people seem to feel strongly about that).

src/scripting/lua_kernel_base.cpp

stevecotton · 2022-06-16T13:13:50Z

I'm still waiting to see if anyone has any suggestions for a different qualified name for this function.

list_attributes would work well on the Wiki, but still gets many unrelated matches on the forums.
print_attributes seems good in both.

Given that you can alias dir = wesnoth.print_attributes, I see no reason to put it in the global namespace.

CelticMinstrel · 2022-06-17T12:07:55Z

Given that you can alias dir = wesnoth.print_attributes, I see no reason to put it in the global namespace.

There is one reason:

Note that the name is the same as the WFL function that does the same thing, so I think that's one argument for keeping the name.

Would you object to adding it as wesnoth.print_attributes and also adding a line dir = wesnoth.print_attributes in core.lua? Or if not in core.lua, then automatically aliasing it whenever you open the Lua console?

Also, to clarify the tab-completion situation… are you saying you intend to work on wiring this into it once I merge this, or did you want me to do that?

However, the original name dir() is still exposed in the Lua console only.

…te function

CelticMinstrel · 2022-06-18T05:42:46Z

I believe this is ready for merge now, unless you wanted me to wire it into the tab-completion before merging. There was one other thing I wanted to do with it, but it's not urgent and could wait until later (basically, adapting the output to the actual width of the Lua console window).

stevecotton · 2022-06-18T15:03:55Z

Note that the name is the same as the WFL function that does the same thing, so I think that's one argument for keeping the name.

Would you object to adding it as wesnoth.print_attributes and also adding a line dir = wesnoth.print_attributes in core.lua? Or if not in core.lua, then automatically aliasing it whenever you open the Lua console?

Look at how much unrelated stuff turns up when searching the Wiki for dir though.

I would object, because it's easy to add but hard to remove - removing it would take a deprecation cycle.

Also, to clarify the tab-completion situation… are you saying you intend to work on wiring this into it once I merge this, or did you want me to do that?

I meant that I intended to review, but wanted your knowledge as a head-start.

CelticMinstrel · 2022-06-18T16:42:57Z

removing it would take a deprecation cycle.

Not if it's only in the Lua console. That's not API that can be used in Lua loaded into the scenario. You claim that aliasing it manually is "easy", and that's technically true, but it's still annoying to have to do it every single time you open a game. And the name "dir" is already used in WFL for a function that does the exact same thing. Sure it's not very searchable, but we can probably add something to the startup log in the Lua console if that's the only problem.

I meant that I intended to review, but wanted your knowledge as a head-start.

So you are planning to work on that…?

CelticMinstrel added the Lua API Issues with the Lua engine and API. label May 29, 2021

CelticMinstrel force-pushed the lua_dir branch from f60167f to 156b294 Compare July 2, 2021 18:50

github-actions bot added the AI Issues with the AI engine, including micro AIs. label Jul 2, 2021

stevecotton reviewed Jul 24, 2021

View reviewed changes

CelticMinstrel force-pushed the lua_dir branch 2 times, most recently from 86d90bd to 154d563 Compare June 4, 2022 14:53

github-actions bot added Editor Map/scenario editor issues. UI User interface issues, including both back-end and front-end issues. Units Issues that involve unit definitions or their implementation in the engine. labels Jun 5, 2022

CelticMinstrel removed UI User interface issues, including both back-end and front-end issues. Editor Map/scenario editor issues. Units Issues that involve unit definitions or their implementation in the engine. labels Jun 5, 2022

CelticMinstrel added 10 commits June 11, 2022 00:18

Add a dir() function to improve API discoverability

a1db339

The purpose is to simply list everything that's available in a given module or object. Currently, it only works for tables. The plan is to extend it to userdata objects if and when it is approved.

Lua API: Make the WML module dir-friendly

6dbe1aa

Exclude deprecated things from dir()

84da56c

Annotate the dir() list with element type (function, table, or other)

5d1704f

Make dir() more robust against errors that can occur while walking th…

4cd8b90

…e object

Implement dir() metamethod for GUI2 widgets

e55d308

This fetches all known attributes for that widget type and also walks the widget tree to grab the IDs of any child widgets, so it SHOULD get everything. Deprecated keys value_compat and callback are excluded.

dir() support for colour ranges

99fe664

Misc dir() support

a19d3b3

- Named tuples - Version - vconfig

CelticMinstrel force-pushed the lua_dir branch from 2d3801c to a19d3b3 Compare June 11, 2022 13:43

CelticMinstrel marked this pull request as ready for review June 11, 2022 13:43

fix whitespace

cc6f198

github-actions bot added the UI User interface issues, including both back-end and front-end issues. label Jun 11, 2022

stevecotton reviewed Jun 12, 2022

View reviewed changes

src/scripting/lua_color.cpp Outdated Show resolved Hide resolved

stevecotton reviewed Jun 12, 2022

View reviewed changes

src/scripting/lua_kernel_base.cpp Outdated Show resolved Hide resolved

stevecotton reviewed Jun 12, 2022

View reviewed changes

src/scripting/lua_kernel_base.cpp Show resolved Hide resolved

stevecotton reviewed Jun 13, 2022

View reviewed changes

src/scripting/lua_kernel_base.cpp Outdated Show resolved Hide resolved

stevecotton reviewed Jun 13, 2022

View reviewed changes

src/scripting/lua_kernel_base.cpp Outdated Show resolved Hide resolved

stevecotton reviewed Jun 13, 2022

View reviewed changes

Address some review comments

1887132

stevecotton reviewed Jun 16, 2022

View reviewed changes

src/scripting/lua_kernel_base.cpp Outdated Show resolved Hide resolved

CelticMinstrel added 3 commits June 17, 2022 22:06

Address another review comment

b4bb680

Rename dir() to wesnoth.print_attributes()

8062a5f

However, the original name dir() is still exposed in the Lua console only.

Factor out the code that fetches the list of attributes into a separa…

b72ebe1

…te function

CelticMinstrel merged commit ab9fe9e into master Jun 21, 2022

CelticMinstrel deleted the lua_dir branch June 21, 2022 00:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a dir() function to improve API discoverability #5818

Add a dir() function to improve API discoverability #5818

CelticMinstrel commented May 29, 2021

stevecotton commented Jun 3, 2021

CelticMinstrel commented Jun 3, 2021

stevecotton Jul 24, 2021

CelticMinstrel Jul 24, 2021 •

edited

CelticMinstrel Jun 5, 2022

CelticMinstrel commented Jun 5, 2022

stevecotton commented Jun 13, 2022

stevecotton Jun 13, 2022

CelticMinstrel Jun 13, 2022

CelticMinstrel Jun 16, 2022

stevecotton Jun 13, 2022 •

edited

CelticMinstrel Jun 13, 2022

stevecotton Jun 13, 2022

CelticMinstrel Jun 13, 2022

stevecotton Jun 16, 2022

CelticMinstrel Jun 16, 2022

CelticMinstrel commented Jun 13, 2022

stevecotton commented Jun 14, 2022

CelticMinstrel commented Jun 14, 2022 •

edited

CelticMinstrel commented Jun 16, 2022

stevecotton commented Jun 16, 2022

CelticMinstrel commented Jun 17, 2022 •

edited

CelticMinstrel commented Jun 18, 2022 •

edited

stevecotton commented Jun 18, 2022

CelticMinstrel commented Jun 18, 2022

Add a dir() function to improve API discoverability #5818

Add a dir() function to improve API discoverability #5818

Conversation

CelticMinstrel commented May 29, 2021

stevecotton commented Jun 3, 2021

CelticMinstrel commented Jun 3, 2021

Choose a reason for hiding this comment

CelticMinstrel Jul 24, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CelticMinstrel commented Jun 5, 2022

stevecotton commented Jun 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevecotton Jun 13, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CelticMinstrel commented Jun 13, 2022

stevecotton commented Jun 14, 2022

CelticMinstrel commented Jun 14, 2022 • edited

CelticMinstrel commented Jun 16, 2022

stevecotton commented Jun 16, 2022

CelticMinstrel commented Jun 17, 2022 • edited

CelticMinstrel commented Jun 18, 2022 • edited

stevecotton commented Jun 18, 2022

CelticMinstrel commented Jun 18, 2022

CelticMinstrel Jul 24, 2021 •

edited

stevecotton Jun 13, 2022 •

edited

CelticMinstrel commented Jun 14, 2022 •

edited

CelticMinstrel commented Jun 17, 2022 •

edited

CelticMinstrel commented Jun 18, 2022 •

edited