Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make undo work again, undeprecate on_event() #7136

Merged
merged 1 commit into from
Nov 19, 2022

Conversation

stevecotton
Copy link
Contributor

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.
@stevecotton stevecotton added Bug Issues involving unexpected behavior. Engine General game engine issues that do not fit in any other category. labels Nov 16, 2022
@stevecotton stevecotton self-assigned this Nov 16, 2022
@github-actions github-actions bot added Lua API Issues with the Lua engine and API. MP Issues with multiplayer support or bundled multiplayer content. labels Nov 16, 2022
@CelticMinstrel
Copy link
Member

CelticMinstrel commented Nov 16, 2022

Reverting (most of) 7e234f8 I get, as that was my recommended quick fix for this, but why revert the deprecation too? There shouldn't be anything wrong with continuing to use things that are deprecated.

EDIT: After posting I noticed the detail on your commit message that explains this. Well… it's not the end of the world to leave it not deprecated for the time being, so whatever.

@Pentarctagon
Copy link
Member

Pentarctagon commented Nov 17, 2022

There should be an issue opened to keep track of the missing undo functionality from the new API, even if #6898 is closed, since even though this fixes it in mainline UMC will start running into this as well.

@soliton-
Copy link
Member

Would be great to get this merged ASAP. Having a major game feature broken for this long is pretty bad even if it's a dev version.

@stevecotton
Copy link
Contributor Author

@CelticMinstrel I'm assuming your "it's not the end of the world" is an approval, will merge this evening unless you say otherwise.

@stevecotton stevecotton merged commit e3deff8 into wesnoth:master Nov 19, 2022
@stevecotton stevecotton deleted the i6898_fix_undo branch November 19, 2022 04:23
@CelticMinstrel
Copy link
Member

It's not an approval so much as a lack of disapproval? But close enough, I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues involving unexpected behavior. Engine General game engine issues that do not fit in any other category. Lua API Issues with the Lua engine and API. MP Issues with multiplayer support or bundled multiplayer content.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Undo command not working
4 participants