Skip to content

Commit

Permalink
fix get_user_choice_multiple_sides
Browse files Browse the repository at this point in the history
the comment in replay.cpp says we want "send data to others" so
calling "pull_remote_user_input" is not really correct. I added a
function synced_context::send_user_choice for this case.

this fixes a bug where players can get an oos error in
wesnoth.synconize_choice
I backported a simpler version to 1.12:
0e79ef9 because i don't know whether
this bug can happen in 1.12 too.
  • Loading branch information
gfgtdf committed Jun 15, 2014
1 parent 17974a4 commit 20267ce
Show file tree
Hide file tree
Showing 9 changed files with 146 additions and 59 deletions.
1 change: 1 addition & 0 deletions src/CMakeLists.txt
Expand Up @@ -878,6 +878,7 @@ set(wesnoth-main_SRC
synced_checkup.cpp
synced_context.cpp
synced_commands.cpp
syncmp_handler.cpp
team.cpp
terrain_filter.cpp
tod_manager.cpp
Expand Down
1 change: 1 addition & 0 deletions src/SConscript
Expand Up @@ -512,6 +512,7 @@ wesnoth_sources = Split("""
synced_checkup.cpp
synced_context.cpp
synced_commands.cpp
syncmp_handler.cpp
team.cpp
terrain_filter.cpp
tod_manager.cpp
Expand Down
82 changes: 47 additions & 35 deletions src/playmp_controller.cpp
Expand Up @@ -526,44 +526,13 @@ void playmp_controller::process_oos(const std::string& err_msg) const {
void playmp_controller::handle_generic_event(const std::string& name){
turn_data_.send_data();

if (name == "ai_user_interact"){
if (name == "ai_user_interact")
{
playsingle_controller::handle_generic_event(name);
turn_data_.send_data();
}
else if ((name == "ai_gamestate_changed") || (name == "ai_sync_network")){
int expected_controller_changes = 0;
turn_info::PROCESS_DATA_RESULT res = turn_data_.sync_network();
assert(res != turn_info::PROCESS_END_LINGER);
assert(res != turn_info::PROCESS_END_TURN);
if(res == turn_info::PROCESS_RESTART_TURN || res == turn_info::PROCESS_RESTART_TURN_TEMPORARY_LOCAL )
{
player_type_changed_ = true;
}
if(res == turn_info::PROCESS_RESTART_TURN_TEMPORARY_LOCAL || res == turn_info::PROCESS_SIDE_TEMPORARY_LOCAL)
{
expected_controller_changes++;
}
//If we still expect controler changes we cannot return.
//Becasue we might get into the situation that we want to do a decision that has already been name on another client.
//FIXME: if the server failed to process a transfer_side this is an infinite loop.
//as a temporary fix we abort the loop if it runs too long.
time_t time_start = time(NULL);
while((expected_controller_changes != 0) && (difftime(time(NULL), time_start) < 20))
{
playsingle_controller::handle_generic_event("ai_user_interact");
res = turn_data_.sync_network();
assert(res != turn_info::PROCESS_END_LINGER);
assert(res != turn_info::PROCESS_END_TURN);
if(res == turn_info::PROCESS_RESTART_TURN)
{
expected_controller_changes--;
}
else if(res == turn_info::PROCESS_RESTART_TURN_TEMPORARY_LOCAL || res == turn_info::PROCESS_SIDE_TEMPORARY_LOCAL)
{
expected_controller_changes++;
}
SDL_Delay(10);
}
else if (name == "ai_gamestate_changed")
{
turn_data_.send_data();
}
else if (name == "host_transfer"){
Expand Down Expand Up @@ -635,3 +604,46 @@ void playmp_controller::maybe_linger()
linger();
}
}

void playmp_controller::pull_remote_choice()
{
int expected_controller_changes = 0;
turn_info::PROCESS_DATA_RESULT res = turn_data_.sync_network();
assert(res != turn_info::PROCESS_END_LINGER);
assert(res != turn_info::PROCESS_END_TURN);
if(res == turn_info::PROCESS_RESTART_TURN || res == turn_info::PROCESS_RESTART_TURN_TEMPORARY_LOCAL )
{
player_type_changed_ = true;
}
if(res == turn_info::PROCESS_RESTART_TURN_TEMPORARY_LOCAL || res == turn_info::PROCESS_SIDE_TEMPORARY_LOCAL)
{
expected_controller_changes++;
}
//If we still expect controler changes we cannot return.
//Becasue we might get into the situation that we want to do a decision that has already been name on another client.
//FIXME: if the server failed to process a transfer_side this is an infinite loop.
//as a temporary fix we abort the loop if it runs too long.
time_t time_start = time(NULL);
while((expected_controller_changes != 0) && (difftime(time(NULL), time_start) < 20))
{
playsingle_controller::handle_generic_event("ai_user_interact");
res = turn_data_.sync_network();
assert(res != turn_info::PROCESS_END_LINGER);
assert(res != turn_info::PROCESS_END_TURN);
if(res == turn_info::PROCESS_RESTART_TURN)
{
expected_controller_changes--;
}
else if(res == turn_info::PROCESS_RESTART_TURN_TEMPORARY_LOCAL || res == turn_info::PROCESS_SIDE_TEMPORARY_LOCAL)
{
expected_controller_changes++;
}
SDL_Delay(10);
}
turn_data_.send_data();
}

void playmp_controller::send_user_choice()
{
turn_data_.send_data();
}
5 changes: 4 additions & 1 deletion src/playmp_controller.hpp
Expand Up @@ -17,10 +17,11 @@
#define PLAYMP_CONTROLLER_H_INCLUDED

#include "playsingle_controller.hpp"
#include "syncmp_handler.hpp"

class turn_info;

class playmp_controller : public playsingle_controller, public events::pump_monitor
class playmp_controller : public playsingle_controller, public events::pump_monitor, public syncmp_handler
{
public:
playmp_controller(const config& level, saved_game& state_of_game,
Expand All @@ -38,6 +39,8 @@ class playmp_controller : public playsingle_controller, public events::pump_moni
void maybe_linger();
void process_oos(const std::string& err_msg) const;

void pull_remote_choice();
void send_user_choice();
protected:
virtual void handle_generic_event(const std::string& name);

Expand Down
2 changes: 1 addition & 1 deletion src/replay.cpp
Expand Up @@ -1029,7 +1029,7 @@ static std::map<int, config> get_user_choice_internal(const std::string &name, c
//but if there wasn't any data sended during this turn, we don't want to bein wth that now.
if(synced_context::is_simultaneously() || current_side != local_side)
{
synced_context::pull_remote_user_input();
synced_context::send_user_choice();
}
continue;

Expand Down
27 changes: 9 additions & 18 deletions src/synced_context.cpp
Expand Up @@ -19,6 +19,8 @@
#include "play_controller.hpp"
#include "actions/undo.hpp"
#include "game_end_exceptions.hpp"
#include "syncmp_handler.hpp"

#include <boost/lexical_cast.hpp>

#include <cassert>
Expand Down Expand Up @@ -170,6 +172,7 @@ namespace
IMPLEMENT_LUA_JAILBREAK_EXCEPTION(lua_network_error)
};
}

void synced_context::pull_remote_user_input()
{
//we sended data over the network.
Expand All @@ -190,24 +193,7 @@ void synced_context::pull_remote_user_input()
//ignore, since it will be thwown throw again.
}
}
try
{
ai::manager::raise_sync_network();
}
catch(end_turn_exception&)
{
//ignore, since it will be thwown again.
}

try
{
// in some cases network::receive_data only returns the wanted result on the second try.
ai::manager::raise_sync_network();
}
catch(end_turn_exception&)
{
//ignore, since it will throw again.
}
syncmp_registry::pull_remote_choice();
}
catch(network::error& err)
{
Expand All @@ -216,6 +202,11 @@ void synced_context::pull_remote_user_input()

}

void synced_context::send_user_choice()
{
is_simultaneously_ = true;
syncmp_registry::send_user_choice();
}

boost::shared_ptr<random_new::rng> synced_context::get_rng_for_action()
{
Expand Down
8 changes: 4 additions & 4 deletions src/synced_context.hpp
Expand Up @@ -77,6 +77,10 @@ class synced_context
called from get_user_choice;
*/
static void pull_remote_user_input();
/*
called from get_user_choice;
*/
static void send_user_choice();
/*
a function to be passed to run_in_synced_context to assert false on error (the default).
*/
Expand Down Expand Up @@ -121,10 +125,6 @@ class synced_context
false = we are on a local turn and haven't sended anything yet.
*/
static bool is_simultaneously_;
/*
TODO: replace ai::manager::raise_sync_network with this event because ai::manager::raise_sync_network has nothing to do with ai anymore.
*/
static events::generic_event remote_user_input_required_;
};

/*
Expand Down
51 changes: 51 additions & 0 deletions src/syncmp_handler.cpp
@@ -0,0 +1,51 @@
#include "syncmp_handler.hpp"

#include <cassert>
#include <boost/foreach.hpp>

syncmp_handler::syncmp_handler()
{
syncmp_registry::add_handler(this);
}

syncmp_handler::~syncmp_handler()
{
syncmp_registry::remove_handler(this);
}

std::vector<syncmp_handler*>& syncmp_registry::handlers()
{
//using pointer in order to prevent destruction at programm end. Although in this simple case it shouldn't matter.
static t_handlers* handlers_ = new t_handlers();
return *handlers_;
}

void syncmp_registry::remove_handler(syncmp_handler* handler)
{
t_handlers::iterator elem = std::find(handlers().begin(), handlers().end(), handler);
assert(elem != handlers().end());
handlers().erase(elem);
}

void syncmp_registry::add_handler(syncmp_handler* handler)
{
t_handlers::iterator elem = std::find(handlers().begin(), handlers().end(), handler);
assert(elem == handlers().end());
handlers().push_back(handler);
}

void syncmp_registry::pull_remote_choice()
{
BOOST_FOREACH(syncmp_handler* phandler, handlers())
{
phandler->pull_remote_choice();
}
}

void syncmp_registry::send_user_choice()
{
BOOST_FOREACH(syncmp_handler* phandler, handlers())
{
phandler->send_user_choice();
}
}
28 changes: 28 additions & 0 deletions src/syncmp_handler.hpp
@@ -0,0 +1,28 @@

#include<vector>
/*
Automaticly registrates itself in the registry in the constructor.
*/
class syncmp_handler
{
public:
syncmp_handler();
virtual void pull_remote_choice() = 0;
virtual void send_user_choice() = 0;
virtual ~syncmp_handler();
};

class syncmp_registry
{
public:
//called by get_user_choice while waiting for a remote user choice.
static void pull_remote_choice();
//called when get_user_choice was called and the client wants to send the choice to the other clients immideately
static void send_user_choice();
private:
friend class syncmp_handler;
typedef std::vector<syncmp_handler*> t_handlers;
static void remove_handler(syncmp_handler* handler);
static void add_handler(syncmp_handler* handler);
static t_handlers& handlers();
};

0 comments on commit 20267ce

Please sign in to comment.