Refactor the game event system to be based on Lua at the core #5663

CelticMinstrel · 2021-04-04T05:16:46Z

All events are now just a Lua function and a package of arguments that are passed to the function. A typical event added via WML (ie, with the [event] tag) is the function wesnoth.wml_actions.command with the contained ActionWML tags passed as the argument, so they work just the same as before. However, you can now add events in two other ways:

Instead of using ActionWML, you can write the entire event inline as Lua code using [event]code=. Although this is similar to just using an event containing a single [lua] tag, there are some differences. The code is compiled when the event is added, rather than when it is executed, and the argument passed to the event code is a copy of the [event] tag with filter data and event name and ID stripped out.
You can now register an event directly from Lua using the new wesnoth.game_events.add function, which supercedes the on_event() system that World Conquest and a few other things depend on. Unlike on_event(), this interface supports everything that [event] does, such as first_time_only=true or giving the event an ID that will allow other events to remove it. It also supports some things that [event] doesn't, such as using an arbitrary Lua function to handle the event (although if you do this, the event becomes unserializable, just the same as if you used on_event()) or marking the event as a menu item event (meaning it will be removed if the menu item with the same ID is removed).

The other significant change in this PR is a refactor of the event filtering system, which would make it very easy to add a new type of filter if we wanted to in the future (although I'm not anticipating any new types). As part of that, two new ways of filtering in events have been added:

The filter_formula key in [event] takes a WFL formula that determines whether the event should trigger. It can access all the event context info (like loc1, event_id, unit1, etc) as well as the game state (such as map, units, turn, and time_of_day). In particular, it exposes event context info that nothing else does in event_data, although there may not be any way to actually set that data yet in [fire_event].
~~The filter key in [event] takes Lua code that determines whether the event should trigger. If there is a [filter_args] tag, its contents is passed to this function.~~
In addition, when adding an event from Lua, you can pass an arbitrary Lua function as the filter. Doing this makes the event unserializable, of course.

One other thing I've done here is to divorce menu items from events at the Lua level. This was already the case in C++, and it seemed to make for a cleaner API. The relevant functions were never documented, so this is probably a minor thing, but basically: wesnoth.interface.set_menu_item adds or updates a menu item but does not make it do anything - the [command] tag is ignored. To make it do something, you need to use wesnoth.game_events.add to create the event that the menu item will trigger. Of course, [set_menu_item] still does both for you.

Pentarctagon · 2021-04-04T05:29:07Z

Unit tests proving out the different aspects of this functionality would be very beneficial.

Pentarctagon · 2021-04-04T05:34:30Z

Adding doxygen doc comments to methods is also a good habit for us to all get into, I think.

src/formula/callable_objects.hpp

src/game_events/handlers.cpp

src/game_events/manager_impl.cpp

src/game_events/manager_impl.hpp

src/scripting/lua_kernel_base.cpp

data/lua/wml-tags.lua

CelticMinstrel · 2021-04-11T22:56:08Z

I think this addresses most of the concerns people mentioned. I still have a few things to finish though:

delayed_variable_substitution=yes seems to be broken
[remove_event] seems to be broken
make pending_event_handler moveable
extract the code chunk name fix

src/formula/callable_objects.cpp

stevecotton

There still isn't enough documentation in this PR to enable the PR to be reviewed.

Change requested: add the documentation.

CelticMinstrel · 2021-04-13T13:44:23Z

The documentation goes on the wiki though, right? It's not part of the PR.

stevecotton · 2021-04-13T15:55:21Z

There's more than just API docs to add. I think docs should be written like the game Taboo - try to explain what it does without repeating the words in the class name, function name, argument types or argument names; that often acts as a design review.

The gamestate_callable class could do with an explanation of what it is, and that it doesn't need any state passed to the constructor.

The event_callable and pending_event_handler classes could do with docs too, so could event_filter.

CelticMinstrel · 2021-04-13T17:58:12Z

Does that help?

src/formula/callable_objects.cpp

CelticMinstrel · 2021-04-14T06:24:58Z

I drafted up some documentation for the Lua API on the wiki.

I realize it's perhaps a little tucked away in this PR, but does anyone have any comments on the Lua unit_test API that I've added here?

stevecotton · 2021-04-14T06:59:47Z

I realize it's perhaps a little tucked away in this PR, but does anyone have any comments on the Lua unit_test API that I've added here?

I'll do a review this evening, but was going to ask if you'd split this into a separate PR.

Unlike the old wesnoth.game_events.on_event hook and the "convenient" on_event() wrapper for it, this new functionality supports all of the features of WML events, with the sole exception of serialization, since it's not possible to reliably serialize a Lua function. This commit also divorces menu items from the event that they trigger. The undocumented wesnoth.interface.set_menu_item function no longer adds an event for the menu item; the caller needs to separately register an event using the new functionality.

…t is registered from Lua.

* These functions take the full event data config as the final argument, rather than just the weapon info subconfigs. * The [fire_event] tag now supports a [data] tag that can add additional data to the event, for example damage_inflicted. The [primary_attack] and [secondary_attack] tags are still supported and are not deprecated. * wesnoth.current.event_context now has a data child which holds the full event data. It still duplicates common info (weapons and damage inflicted) in the same way as before.

- Don't try to serialize an event with a Lua filter (ie, add{filter = function() end}) - Write the serialization warning to in-game chat, but ONLY if the event was registered after preload

… a repeating event This is a somewhat controversial change! PROS: - This makes wesnoth.game_events.add a drop-in replacement for the on_event function - If you're registering events in preload, most of them WILL be repeating events CONS: - This makes the default depend on the call style; in positional style, it defaults to repeating, and in named-argument style, it defaults to non-repeating - This makes the default different from the WML default of non-repeating events

Basically this means each of the call modes of the old function is now a separate function. - add_repeating and add_menu take an ID and a function - add_wml takes variable substitution setting and a config - add takes the full options table

Since they're non-serializable, the code can't be shown, but the fact that unspecified Lua code is present will be shown.

These are taken as synonyms of the actual internal tag names, [first] and [second]. So, first is interchangeable with primary_attack and second is interchangeable with secondary_attack.

Although it's already possible to pass {formula = "whatever"}, this change allows mixing that with the simplified syntax, such as {formula = "whatever", side = {side = 3}}

This creates a filter that's read dynamically from the specified WML variable.

- Add default name for wesnoth.game_events.add when it's a menu item - Add error message when both ID and name are empty

Fixes wesnoth#6898. The issue is that non-WML events added through the new events API always disable undo with no equivalent of WML's `[allow_undo]`. The long-term fix is to add a way to do that; however until that's available then listeners for `moveto` need to use the old `on_event` API. The old `on_event` API can't be deprecated yet, and this is enforced by our unit tests (the build fails if there are unexpected deprecation warnings during the tests). Reverts most of 7e234f8. Does not revert files that only listen for non-undoable events such as `die` or `new turn`. Reverts the deprecation part of wesnoth#5663's 8cd1332.

Fixes #6898. The issue is that non-WML events added through the new events API always disable undo with no equivalent of WML's `[allow_undo]`. The long-term fix is to add a way to do that; however until that's available then listeners for `moveto` need to use the old `on_event` API. The old `on_event` API can't be deprecated yet, and this is enforced by our unit tests (the build fails if there are unexpected deprecation warnings during the tests). Reverts most of 7e234f8. Does not revert files that only listen for non-undoable events such as `die` or `new turn`. Reverts the deprecation part of #5663's 8cd1332.

Fixes wesnoth#6898. The issue is that non-WML events added through the new events API always disable undo with no equivalent of WML's `[allow_undo]`. The long-term fix is to add a way to do that; however until that's available then listeners for `moveto` need to use the old `on_event` API. The old `on_event` API can't be deprecated yet, and this is enforced by our unit tests (the build fails if there are unexpected deprecation warnings during the tests). Reverts most of 7e234f8. Does not revert files that only listen for non-undoable events such as `die` or `new turn`. Reverts the deprecation part of wesnoth#5663's 8cd1332.

CelticMinstrel requested review from Pentarctagon, Vultraz and gfgtdf April 4, 2021 05:16

stevecotton reviewed Apr 4, 2021

View reviewed changes

src/formula/callable_objects.hpp Outdated Show resolved Hide resolved

stevecotton reviewed Apr 4, 2021

View reviewed changes

src/game_events/handlers.cpp Outdated Show resolved Hide resolved

stevecotton reviewed Apr 4, 2021

View reviewed changes

src/game_events/handlers.cpp Outdated Show resolved Hide resolved

stevecotton reviewed Apr 5, 2021

View reviewed changes

src/game_events/manager_impl.cpp Outdated Show resolved Hide resolved

stevecotton reviewed Apr 5, 2021

View reviewed changes

src/game_events/manager_impl.hpp Show resolved Hide resolved

stevecotton reviewed Apr 5, 2021

View reviewed changes

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

stevecotton reviewed Apr 5, 2021

View reviewed changes

data/lua/wml-tags.lua Outdated Show resolved Hide resolved

CelticMinstrel force-pushed the lua_events branch from 227269a to 0805dfa Compare April 11, 2021 22:53

github-actions bot added Building Build-time issues. macOS OS-specific issues that apply to Apple macOS. Unit Tests Issues involving Wesnoth's unit test suite. labels Apr 11, 2021

stevecotton reviewed Apr 13, 2021

View reviewed changes

src/formula/callable_objects.cpp Outdated Show resolved Hide resolved

stevecotton requested changes Apr 13, 2021

View reviewed changes

CelticMinstrel commented Apr 14, 2021

View reviewed changes

src/formula/callable_objects.cpp Outdated Show resolved Hide resolved

CelticMinstrel added 21 commits July 18, 2022 20:10

Add [event]filter_formula= for WFL event filtering

34c2810

Support passing an arbitrary Lua function as the filter when the even…

2e13265

…t is registered from Lua.

Improve the handling of event serialization when Lua code is attached

5c1558e

- Don't try to serialize an event with a Lua filter (ie, add{filter = function() end}) - Write the serialization warning to in-game chat, but ONLY if the event was registered after preload

Unit tests for each way of assigning an action to an event handler

4ac7611

Unit tests for the new filter options

74f0b9e

Update [remove_event] to use new API

c12959d

Update schema for the new [event] options

59ee644

Fix dynamic filters behaviour change

5f1d7dd

Include non-serializable events in the gamestate inspector

0eef9b8

Since they're non-serializable, the code can't be shown, but the fact that unspecified Lua code is present will be shown.

Fix non-serializable events being serialized as an empty [event] tag

e7579d6

Add luadoc for the new event functions

3441e56

Support primary_attack and secondary_attack in wesnoth.game_events.fire

b1e894d

These are taken as synonyms of the actual internal tag names, [first] and [second]. So, first is interchangeable with primary_attack and second is interchangeable with secondary_attack.

Replace deprecated usages of on_event()

6a764c4

Fix WML errors showing as Lua errors

c5098e1

Support "formula" in the filter table

0ec27c2

Although it's already possible to pass {formula = "whatever"}, this change allows mixing that with the simplified syntax, such as {formula = "whatever", side = {side = 3}}

Support {filter_type = "variable_path"}

d0d9b00

This creates a filter that's read dynamically from the specified WML variable.

Some cleanup of the events API

1edd0cb

- Add default name for wesnoth.game_events.add when it's a menu item - Add error message when both ID and name are empty

CelticMinstrel force-pushed the lua_events branch from 0aad8dd to 1edd0cb Compare July 19, 2022 00:11

CelticMinstrel merged commit 5001999 into master Jul 19, 2022

CelticMinstrel mentioned this pull request Jul 19, 2022

Additional event attributes #6886

Draft

stevecotton mentioned this pull request Nov 16, 2022

Make undo work again, undeprecate on_event() #7136

Merged

Wedge009 mentioned this pull request Jul 10, 2023

Stun events leak to unrelated games #7778

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor the game event system to be based on Lua at the core #5663

Refactor the game event system to be based on Lua at the core #5663

CelticMinstrel commented Apr 4, 2021 •

edited

Pentarctagon commented Apr 4, 2021

Pentarctagon commented Apr 4, 2021

CelticMinstrel commented Apr 11, 2021 •

edited

stevecotton left a comment

CelticMinstrel commented Apr 13, 2021

stevecotton commented Apr 13, 2021

CelticMinstrel commented Apr 13, 2021

CelticMinstrel commented Apr 14, 2021

stevecotton commented Apr 14, 2021

Refactor the game event system to be based on Lua at the core #5663

Refactor the game event system to be based on Lua at the core #5663

Conversation

CelticMinstrel commented Apr 4, 2021 • edited

Pentarctagon commented Apr 4, 2021

Pentarctagon commented Apr 4, 2021

CelticMinstrel commented Apr 11, 2021 • edited

stevecotton left a comment

Choose a reason for hiding this comment

CelticMinstrel commented Apr 13, 2021

stevecotton commented Apr 13, 2021

CelticMinstrel commented Apr 13, 2021

CelticMinstrel commented Apr 14, 2021

stevecotton commented Apr 14, 2021

CelticMinstrel commented Apr 4, 2021 •

edited

CelticMinstrel commented Apr 11, 2021 •

edited