Skip to content

Commit

Permalink
fix OOS when undoing actions with synced choices
Browse files Browse the repository at this point in the history
The problm was that the old code tried to use `undo_stack().can_undo()`
to check whether the current action can be undone. But
`undo_stack().can_undo()` uses the undo stack which is only updated at
the end of each action, so it cannot be used to check whether the action
that is currently executed can be undone.

This code removes some assertion that were wrong due to the
justmentioned.
  • Loading branch information
gfgtdf authored and Vultraz committed Apr 3, 2018
1 parent 596f3f1 commit 79072b7
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 9 deletions.
8 changes: 0 additions & 8 deletions src/playmp_controller.cpp
Expand Up @@ -423,10 +423,6 @@ void playmp_controller::surrender(int side_number) {

void playmp_controller::pull_remote_choice()
{
// when using a remote user choice undoing must be impossible because that network traffic cannot be undone
// Also turn_data_.sync_network() (which calls turn_data_.send_data()) won't work if the undo stack isn't empty because undoable moves won't be sent
// Also undo_stack()clear() must be called synced so we cannot do that here.
assert(!current_team().is_local() || !undo_stack().can_undo());
turn_info::PROCESS_DATA_RESULT res = turn_data_.sync_network();
assert(res != turn_info::PROCESS_END_TURN);

Expand All @@ -443,10 +439,6 @@ void playmp_controller::pull_remote_choice()

void playmp_controller::send_user_choice()
{
// when using a remote user choice undoing must be impossible because that network traffic cannot be undone
// Also turn_data_.send_data() won't work if the undo stack isn't empty because undoable moves won't be sent
// Also undo_stack()clear() must be called synced so we cannot do that here.
assert(!undo_stack().can_undo());
turn_data_.send_data();
}

Expand Down
4 changes: 3 additions & 1 deletion src/playturn.cpp
Expand Up @@ -33,6 +33,7 @@
#include "replay.hpp" // for replay, recorder, do_replay, etc
#include "resources.hpp" // for gameboard, screen, etc
#include "serialization/string_utils.hpp" // for string_map
#include "synced_context.hpp"
#include "team.hpp" // for team, team::CONTROLLER::AI, etc
#include "wesnothd_connection_error.hpp"
#include "whiteboard/manager.hpp" // for manager
Expand Down Expand Up @@ -77,7 +78,8 @@ turn_info::PROCESS_DATA_RESULT turn_info::sync_network()

void turn_info::send_data()
{
if ( resources::undo_stack->can_undo() ) {
const bool send_everything = synced_context::is_unsynced() ? !resources::undo_stack->can_undo() : synced_context::is_simultaneously();
if ( !send_everything ) {
replay_sender_.sync_non_undoable();
} else {
replay_sender_.commit_and_sync();
Expand Down
5 changes: 5 additions & 0 deletions src/synced_context.cpp
Expand Up @@ -171,6 +171,11 @@ bool synced_context::is_synced()
return get_synced_state() == SYNCED;
}

bool synced_context::is_unsynced()
{
return get_synced_state() == UNSYNCED;
}

void synced_context::set_synced_state(synced_state newstate)
{
state_ = newstate;
Expand Down
4 changes: 4 additions & 0 deletions src/synced_context.hpp
Expand Up @@ -77,6 +77,10 @@ class synced_context
@return whether we are currently executing a synced action like recruit, start, recall, disband, movement, attack, init_side, end_turn, fire_event, lua_ai, auto_shroud or similar.
*/
static bool is_synced();
/**
@return whether we are not currently executing a synced action like recruit, start, recall, disband, movement, attack, init_side, end_turn, fire_event, lua_ai, auto_shroud or similar. and not doing a local choice.
*/
static bool is_unsynced();
/*
should only be called form set_scontext_synced, set_scontext_local_choice
*/
Expand Down

0 comments on commit 79072b7

Please sign in to comment.