refactor set_scontext_(un)synced classes

renamed set_scontext_local_choice to leave_synced_context and
set_scontext_leave_for_draw to set_scontext_unsynced.
The old names do not fit anymore because set_scontext_leave_for_draw is
also used for wml menu items.

Also simplified the implementation of set_scontext_leave_for_draw.

src/actions/undo.cpp

@@ -460,7 +460,7 @@ bool undo_list::apply_shroud_changes() const
 		return false;
 	}
 	// If we clear fog or shroud outside a synced context we get OOS
-	assert(synced_context::get_synced_state() == synced_context::SYNCED);
+	assert(synced_context::is_synced());
 	shroud_clearer clearer;
 	bool cleared_shroud = false;
 	const size_t list_size = undos_.size();

src/ai/actions.cpp

@@ -283,8 +283,8 @@ void attack_result::do_execute()
 		return;
 	}
 
-	//to get rid of an unused member variable warning, FIXME: find a way to 'ask' the ai which advancement should be chosen from synced_commands.cpp .
-	if(synced_context::get_synced_state() != synced_context::SYNCED) //RAII block for set_scontext_synced
+	//FIXME: find a way to 'ask' the ai which advancement should be chosen from synced_commands.cpp .
+	if(!synced_context::is_synced()) //RAII block for set_scontext_synced
 	{
 		//we don't use synced_context::run_in_synced_context because that wouldn't allow us to pass advancements_
 		resources::recorder->add_synced_command("attack", replay_helper::get_attack(attacker_loc_, defender_loc_, attacker_weapon, defender_weapon, a_->type_id(),

src/display.cpp

@@ -2711,7 +2711,7 @@ void display::draw(bool update,bool force) {
 	if (screen_.update_locked()) {
 		return;
 	}
-	set_scontext_leave_for_draw leave_synced_context;
+	set_scontext_unsynced leave_synced_context;
 	local_tod_light_ = has_time_area() && preferences::get("local_tod_lighting", true);
 
 	draw_init();

src/game_display.cpp

@@ -189,7 +189,7 @@ void game_display::select_hex(map_location hex)
 
 void game_display::highlight_hex(map_location hex)
 {
-	wb::future_map_if future(synced_context::get_synced_state() != synced_context::SYNCED); /**< Lasts for whole method. */
+	wb::future_map_if future(!synced_context::is_synced()); /**< Lasts for whole method. */
 
 	const unit *u = resources::gameboard->get_visible_unit(hex, dc_->teams()[viewing_team()], !dont_show_all_);
 	if (u) {
@@ -216,7 +216,7 @@ void game_display::display_unit_hex(map_location hex)
 	if (!hex.valid())
 		return;
 
-	wb::future_map_if future(synced_context::get_synced_state() != synced_context::SYNCED); /**< Lasts for whole method. */
+	wb::future_map_if future(!synced_context::is_synced()); /**< Lasts for whole method. */
 
 	const unit *u = resources::gameboard->get_visible_unit(hex, dc_->teams()[viewing_team()], !dont_show_all_);
 	if (u) {

src/game_events/menu_item.cpp

@@ -208,7 +208,7 @@ void wml_menu_item::fire_event(const map_location & event_hex, const game_data &
 {
 	if(!this->is_synced())
 	{
-		set_scontext_leave_for_draw leave_synced_context;
+		set_scontext_unsynced leave_synced_context;
 		if (boost::shared_ptr<manager * const> man = my_manager_.lock()) {
 			(**man).pump().fire(event_name_, event_hex);
 		} else {

src/replay.cpp

@@ -671,7 +671,7 @@ REPLAY_RETURN do_replay_handle(bool one_move)
 	while(true)
 	{
 		const config *cfg = resources::recorder->get_next_action();
-		const bool is_synced = (synced_context::get_synced_state() == synced_context::SYNCED);
+		const bool is_synced = synced_context::is_synced();
 
 		DBG_REPLAY << "in do replay with is_synced=" << is_synced << "\n";
 
@@ -950,7 +950,7 @@ static std::map<int, config> get_user_choice_internal(const std::string &name, c
 
 		if (is_replay_end && has_local_side)
 		{
-			set_scontext_local_choice sync;
+			leave_synced_context sync;
 			/* At least one of the decisions is ours, and it will be inserted
 			   into the replay. */
 			DBG_REPLAY << "MP synchronization: local choice\n";
@@ -1026,7 +1026,7 @@ std::map<int,config> mp_sync::get_user_choice_multiple_sides(const std::string &
 	std::set<int> sides)
 {
 	//pass sides by copy because we need a copy.
-	const bool is_synced = synced_context::get_synced_state() == synced_context::SYNCED;
+	const bool is_synced = synced_context::is_synced();
 	const int max_side  = static_cast<int>(resources::teams->size());
 	//we currently don't check for too early because luas sync choice doesn't necessarily show screen dialogs.
 	//It (currently) in the responsibility of the user of sync choice to not use dialogs during prestart events..
@@ -1072,7 +1072,7 @@ config mp_sync::get_user_choice(const std::string &name, const mp_sync::user_cho
 	int side)
 {
 	const bool is_too_early = resources::gamedata->phase() != game_data::START && resources::gamedata->phase() != game_data::PLAY;
-	const bool is_synced = synced_context::get_synced_state() == synced_context::SYNCED;
+	const bool is_synced = synced_context::is_synced();
 	const bool is_mp_game = network::nconnections() != 0;//Only used in debugging output below
 	const int max_side  = static_cast<int>(resources::teams->size());
 	bool is_side_null_controlled;

src/synced_context.cpp

@@ -167,6 +167,11 @@ synced_context::synced_state synced_context::get_synced_state()
 	return state_;
 }
 
+bool synced_context::is_synced()
+{
+	return get_synced_state() == SYNCED;
+}
+
 void synced_context::set_synced_state(synced_state newstate)
 {
 	state_ = newstate;
@@ -220,7 +225,7 @@ void  synced_context::set_is_simultaneously()
 bool synced_context::can_undo()
 {
 	//this method should only works in a synced context.
-	assert(get_synced_state() == SYNCED);
+	assert(is_synced());
 	//if we called the rng or if we sended data of this action over the network already, undoing is impossible.
 	return (!is_simultaneously_) && (random_new::generator->get_random_calls() == 0);
 }
@@ -233,7 +238,7 @@ void synced_context::set_last_unit_id(int id)
 int synced_context::get_unit_id_diff()
 {
 	//this method only works in a synced context.
-	assert(get_synced_state() == SYNCED);
+	assert(is_synced());
 	return n_unit::id_manager::instance().get_save_id() - last_unit_id_;
 }
 
@@ -303,7 +308,7 @@ config synced_context::ask_server_choice(const server_choice& sch)
 {
 	set_is_simultaneously();
 	resources::controller->increase_server_request_number();
-	assert(get_synced_state() == synced_context::SYNCED);
+	assert(is_synced());
 	const bool is_mp_game = network::nconnections() != 0;
 	bool did_require = false;
 
@@ -325,6 +330,7 @@ config synced_context::ask_server_choice(const server_choice& sch)
 			/* The decision is ours, and it will be inserted
 			into the replay. */
 			DBG_REPLAY << "MP synchronization: local server choice\n";
+			leave_synced_context sync;
 			config cfg = sch.local_choice();
 			//-1 for "server" todo: change that.
 			resources::recorder->user_input(sch.name(), cfg, -1);
@@ -496,52 +502,27 @@ int set_scontext_synced::get_random_calls()
 }
 
 
-set_scontext_local_choice::set_scontext_local_choice()
+leave_synced_context::leave_synced_context()
+	: new_rng_()
+	, old_rng_(random_new::generator)
 {
-	//TODO: should we also reset the synced checkup?
 	assert(synced_context::get_synced_state() == synced_context::SYNCED);
 	synced_context::set_synced_state(synced_context::LOCAL_CHOICE);
 
-
-	old_rng_ = random_new::generator;
 	//calling the synced rng form inside a local_choice would cause oos.
-	//TODO use a member variable instead if new/delete
-	random_new::generator = new random_new::rng();
+	//TODO: should we also reset the synced checkup?
+	random_new::generator = &new_rng_;
 }
-set_scontext_local_choice::~set_scontext_local_choice()
+
+leave_synced_context::~leave_synced_context()
 {
 	assert(synced_context::get_synced_state() == synced_context::LOCAL_CHOICE);
 	synced_context::set_synced_state(synced_context::SYNCED);
-	delete random_new::generator;
 	random_new::generator = old_rng_;
 }
 
-set_scontext_leave_for_draw::set_scontext_leave_for_draw()
-	: previous_state_(synced_context::get_synced_state())
+set_scontext_unsynced::set_scontext_unsynced()
+	: leaver_(synced_context::is_synced() ? new leave_synced_context() : NULL)
 {
-	if(previous_state_ != synced_context::SYNCED)
-	{
-		old_rng_= NULL;
-		return;
-	}
-	synced_context::set_synced_state(synced_context::LOCAL_CHOICE);
 
-	assert(random_new::generator);
-	old_rng_ = random_new::generator;
-	//calling the synced rng form inside a local_choice would cause oos.
-	//TODO use a member variable instead if new/delete
-	random_new::generator = new random_new::rng();
-}
-set_scontext_leave_for_draw::~set_scontext_leave_for_draw()
-{
-	if(previous_state_ != synced_context::SYNCED)
-	{
-		return;
-	}
-	assert(old_rng_);
-	assert(random_new::generator);
-	assert(synced_context::get_synced_state() == synced_context::LOCAL_CHOICE);
-	synced_context::set_synced_state(synced_context::SYNCED);
-	delete random_new::generator;
-	random_new::generator = old_rng_;
 }

src/synced_context.hpp

@@ -30,7 +30,12 @@ class config;
 class synced_context
 {
 public:
-	enum synced_state {UNSYNCED, SYNCED, LOCAL_CHOICE};
+	enum synced_state
+	{
+		UNSYNCED,
+		SYNCED, 
+		LOCAL_CHOICE
+	};
 	/**
 
 		Sets the context to 'synced', initialises random context, and calls the given function.
@@ -67,6 +72,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 synced_state get_synced_state();
+	/**
+		@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();
 	/*
 		should only be called form set_scontext_synced, set_scontext_local_choice
 	*/
@@ -190,29 +199,27 @@ class set_scontext_synced : set_scontext_synced_base
 	a RAII object to temporary leave the synced context like in  wesnoth.synchronize_choice. Can only be used from inside a synced context.
 */
 
-class set_scontext_local_choice
+class leave_synced_context
 {
 public:
-	set_scontext_local_choice();
-	~set_scontext_local_choice();
+	leave_synced_context();
+	~leave_synced_context();
 private:
+	random_new::rng new_rng_;
 	random_new::rng* old_rng_;
 };
 
 /**
-	an object to leave the synced context during draw when we don’t know whether we are in a synced context or not.
-	if we are in a sanced context we leave the synced context otherwise it has no effect.
+	an object to leave the synced context during draw or unsynced wml items when we don’t know whether we are in a synced context or not.
+	if we are in a synced context we leave the synced context, otherwise it has no effect.
 	we need this because we might call lua's wesnoth.theme_items during draw and we don’t want to have that an effect on the gamestate in this case.
 */
-
-class set_scontext_leave_for_draw
+class set_scontext_unsynced
 {
 public:
-	set_scontext_leave_for_draw();
-	~set_scontext_leave_for_draw();
+	set_scontext_unsynced();
 private:
-	random_new::rng* old_rng_;
-	synced_context::synced_state previous_state_;
+	boost::scoped_ptr<leave_synced_context> leaver_;
 };
 
 #endif

src/unit.cpp

@@ -2117,7 +2117,7 @@ bool unit::is_visible_to_team(team const& team, gamemap const& map, bool const s
 
 void unit::set_underlying_id() {
 	if(underlying_id_.value == 0) {
-		if(synced_context::get_synced_state() == synced_context::SYNCED || !resources::gamedata || resources::gamedata->phase() == game_data::INITIAL) {
+		if(synced_context::is_synced() || !resources::gamedata || resources::gamedata->phase() == game_data::INITIAL) {
 			underlying_id_ = n_unit::id_manager::instance().next_id();
 		}
 		else {
@@ -2136,7 +2136,7 @@ unit& unit::clone(bool is_temporary)
 	if(is_temporary) {
 		underlying_id_ = n_unit::id_manager::instance().next_fake_id();
 	} else {
-		if(synced_context::get_synced_state() == synced_context::SYNCED || !resources::gamedata || resources::gamedata->phase() == game_data::INITIAL) {
+		if(synced_context::is_synced() || !resources::gamedata || resources::gamedata->phase() == game_data::INITIAL) {
 			underlying_id_ = n_unit::id_manager::instance().next_id();
 		}
 		else {