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

attempting to chat during [delay] in a start event blocks the game #1857

Closed
gfgtdf opened this issue Jul 23, 2017 · 7 comments
Closed

attempting to chat during [delay] in a start event blocks the game #1857

gfgtdf opened this issue Jul 23, 2017 · 7 comments
Assignees
Labels
Blocker: New Stable Issues that must be resolved prior to the next stable series being released. Bug Issues involving unexpected behavior. MP Issues with multiplayer support or bundled multiplayer content. Ready for testing Issues for which a potential fix is available but untested. Regression Issues that were not present in previous releases. UI User interface issues, including both back-end and front-end issues.
Milestone

Comments

@gfgtdf
Copy link
Contributor

gfgtdf commented Jul 23, 2017

i just tested with an addon that does

	[event]
		name=start
		[lua]
		code = <<
				wesnoth.delay(10000)
		>>
		[/lua]
	[/event]

when you now enter a chat message from player 1 during thay [delay] the follwing happens:

  1. The chat message immidiatley appears at the client 1s screen
  2. After 10 second the chat message immidiatley appears at the client 2s screen
  3. The 'new turn' dialogs never shows and it seems like the game never reaches 'turn 1' on client 1: The 'end turn' button stas greyed out and you cnnnot recruit/move units.
@gfgtdf gfgtdf added Blocker: New Stable Issues that must be resolved prior to the next stable series being released. Bug Issues involving unexpected behavior. MP Issues with multiplayer support or bundled multiplayer content. UI User interface issues, including both back-end and front-end issues. labels Jul 23, 2017
@gfgtdf
Copy link
Contributor Author

gfgtdf commented Jul 23, 2017

Ok i investigated this issue, and it seem like when you chat the chatmessage is put on the replay so that then at https://github.com/wesnoth/wesnoth/blob/1.13.8/src/play_controller.cpp#L412 the condition replay_->at_end() is false so that the side is never initiated.

gfgtdf added a commit that referenced this issue Jul 23, 2017
the `replay_->at_end()` check was added in 81b6c3a and is not needed anymore after 9ca6678

This also removes the `gamestate().gamedata_.phase() != game_data::PLAY` check which was previously there to fix the case when a [change_controller] was received while waiting for a serversided random seed during a start event. But that's not needed anymore after 11daa51 since now the client doesn't anymore call maybe_do_init_side whenever he reveives a [change_controller]
@gfgtdf
Copy link
Contributor Author

gfgtdf commented Jul 24, 2017

ok while 75cc7e6 somwhow fixes the client getting stuck it now gives an assertion error (replay.cpp assert(at_end())). So this is not fully solved yet. Fixing #1856 would probably fix this one aswell and might actually not be much harder so i'm considering fixing that one first.

@gfgtdf
Copy link
Contributor Author

gfgtdf commented Aug 6, 2017

since this still quite the corner case, i'll set this for 1.14 (meanign it doesn't block 1.13.9)

@gfgtdf gfgtdf added this to the 1.14.0 milestone Aug 6, 2017
@gfgtdf gfgtdf self-assigned this Aug 12, 2017
gfgtdf added a commit that referenced this issue Sep 6, 2017
See #1857

This commit does 2 things:
1) Ensure that if the replay pos was at_end before a speak was added it will still be at end after the speak was added, fixes the mentioned assertion failure.
2) Make unsyced map_labels use the same logic as speak, this fixes OOS issues when the 'back to turn' mp feature was used in mp, since previously adding map labels while watching a mp replay (is the really possible? not 100% sure) might cause mismatches of replay_pos of other savegames with the current replay data.
@gfgtdf
Copy link
Contributor Author

gfgtdf commented Sep 6, 2017

This is probably fixed by d08f402, still have to test it

@gfgtdf gfgtdf added the Ready for testing Issues for which a potential fix is available but untested. label Sep 6, 2017
@gfgtdf gfgtdf added the Regression Issues that were not present in previous releases. label Sep 30, 2017
@gfgtdf
Copy link
Contributor Author

gfgtdf commented Nov 4, 2017

ok tested that this is fixed

@gfgtdf gfgtdf closed this as completed Nov 4, 2017
@CelticMinstrel
Copy link
Member

Is #1856 fixed too, then?

@gfgtdf
Copy link
Contributor Author

gfgtdf commented Nov 4, 2017

it isn't

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocker: New Stable Issues that must be resolved prior to the next stable series being released. Bug Issues involving unexpected behavior. MP Issues with multiplayer support or bundled multiplayer content. Ready for testing Issues for which a potential fix is available but untested. Regression Issues that were not present in previous releases. UI User interface issues, including both back-end and front-end issues.
Projects
None yet
Development

No branches or pull requests

2 participants