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

OOS in multiplayer when [modify_side] changes a controller during "side_turn_end" event. #2563

Open
gfgtdf opened this issue Feb 26, 2018 · 5 comments · May be fixed by #8246
Open

OOS in multiplayer when [modify_side] changes a controller during "side_turn_end" event. #2563

gfgtdf opened this issue Feb 26, 2018 · 5 comments · May be fixed by #8246
Labels
Bug Issues involving unexpected behavior. MP Issues with multiplayer support or bundled multiplayer content. WML Issues involving the WML engine or WML APIs.

Comments

@gfgtdf
Copy link
Contributor

gfgtdf commented Feb 26, 2018

if a code like

[modify_side]
  side = $($current_side + 1)
  controller=human
[/modify_side]

is executed during an "side_turn_end" OOS can happen (assumng that side was previously null-controlled)
What happens is that the [end_turn] command is sended to the server before that event is executed and the server updates the current_player_ field when it recieves [end_turn]. So what happens on the server is that a the time when current_player_ filed is updated the next side is skipped becasue it is empty, on the clients however current_side_ is updated after the 'side_turn_end' event has finished so the next side will not be skipped since modify_side activated it.

@gfgtdf gfgtdf added Bug Issues involving unexpected behavior. MP Issues with multiplayer support or bundled multiplayer content. labels Feb 26, 2018
@soliton-
Copy link
Member

Is this likely to happen/be wanted? Perhaps it‘d be enough to document the issue on the wiki.

@gfgtdf
Copy link
Contributor Author

gfgtdf commented Feb 27, 2018

well before 1.13.2 (2d80c97) we lived fine without support for [modify_side] controller= in mp at all unrelated to in which event it happens, so this is clearly not a blocker. I still think it'd be nice to have it since there is no reason why using it in turn end event would make less sense than in any other event.

I also didn't find any cases in mainline campaign that ere effected by it (i did find a possible invalid [filter] though in https://github.com/wesnoth/wesnoth/blob/1.13.11/data/campaigns/Two_Brothers/scenarios/02_The_Chase.cfg#L423)

EDIT: i just found my first commit that fixes [modify_side] controller= in MP. 6dee136 . It also gives us a testing scenario for this one, just replace side turn with side turn end

@Vultraz
Copy link
Member

Vultraz commented Mar 4, 2018

@gfgtdf that filter seems to work fine... odd... the wiki doesn't say [filter] is accepted under [modify_side].

@CelticMinstrel
Copy link
Member

The filter tag is used to match which sides to modify. Without it it would be impossible to distinguish the filter keys from the change keys.

@CelticMinstrel
Copy link
Member

(But if there's no filter tag the side key is supposed to be used instead.)

@cooljeanius cooljeanius added the WML Issues involving the WML engine or WML APIs. label Jan 1, 2023
gfgtdf added a commit to gfgtdf/wesnoth that referenced this issue Jan 15, 2024
This matches the behavior on the client where the events that
happen after end_turn (the "turn end" wml events) happen during
the last sides turn not the next sides turn. In particular this
fixes wesnoth#2563. It shodul also make it easier to fix wesnoth#3899 later.
It also changes how descption is updated, since carrying that
return value arround specifying whether it was updated was becoming
annoying.
@gfgtdf gfgtdf linked a pull request Jan 15, 2024 that will close this issue
gfgtdf added a commit to gfgtdf/wesnoth that referenced this issue Jan 16, 2024
This matches the behavior on the client where the events that
happen after end_turn (the "turn end" wml events) happen during
the last sides turn not the next sides turn. In particular this
fixes wesnoth#2563. It shodul also make it easier to fix wesnoth#3899 later.
It also changes how descption is updated, since carrying that
return value arround specifying whether it was updated was becoming
annoying.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues involving unexpected behavior. MP Issues with multiplayer support or bundled multiplayer content. WML Issues involving the WML engine or WML APIs.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants