(svn r19933) -Fix [FS#3804]: Keep _current_company and _local_company in sync during GUI operation.

7 files changed, 54 insertions, 39 deletions

diff --git a/src/command.cpp b/src/command.cpp
index 19fd1a69c..169c2b6d0 100644
--- a/src/command.cpp
+++ b/src/command.cpp
@@ -26,6 +26,7 @@
 #include "company_func.h"
 #include "company_base.h"
 #include "signal_func.h"
+#include "core/backup_type.hpp"
 
 #include "table/strings.h"
 
@@ -590,17 +591,18 @@ CommandCost DoCommandPInternal(TileIndex tile, uint32 p1, uint32 p2, uint32 cmd,
 	if (tile != 0 && (tile >= MapSize() || (!IsValidTile(tile) && (cmd_flags & CMD_ALL_TILES) == 0))) return_dcpi(CMD_ERROR, false);
 
 	/* Always execute server and spectator commands as spectator */
-	if (cmd_flags & (CMD_SPECTATOR | CMD_SERVER)) _current_company = COMPANY_SPECTATOR;
-
-	CompanyID old_company = _current_company;
+	bool exec_as_spectator = cmd_flags & (CMD_SPECTATOR | CMD_SERVER);
 
 	/* If the company isn't valid it may only do server command or start a new company!
 	 * The server will ditch any server commands a client sends to it, so effectively
 	 * this guards the server from executing functions for an invalid company. */
-	if (_game_mode == GM_NORMAL && (cmd_flags & (CMD_SPECTATOR | CMD_SERVER)) == 0 && !Company::IsValidID(_current_company)) {
+	if (_game_mode == GM_NORMAL && !exec_as_spectator && !Company::IsValidID(_current_company)) {
 		return_dcpi(CMD_ERROR, false);
 	}
 
+	Backup<CompanyByte> cur_company(_current_company, FILE_LINE);
+	if (exec_as_spectator) cur_company.Change(COMPANY_SPECTATOR);
+
 	bool test_and_exec_can_differ = (cmd_flags & CMD_NO_TEST) != 0;
 	bool skip_test = _networking && (cmd & CMD_NO_TEST_IF_IN_NETWORK) != 0;
 
@@ -622,7 +624,7 @@ CommandCost DoCommandPInternal(TileIndex tile, uint32 p1, uint32 p2, uint32 cmd,
 		SetTownRatingTestMode(false);
 
 		/* Make sure we're not messing things up here. */
-		assert(cmd_id == CMD_COMPANY_CTRL || old_company == _current_company);
+		assert(exec_as_spectator ? _current_company == COMPANY_SPECTATOR : cur_company.Verify());
 
 		/* If the command fails, we're doing an estimate
 		 * or the player does not have enough money
@@ -631,6 +633,7 @@ CommandCost DoCommandPInternal(TileIndex tile, uint32 p1, uint32 p2, uint32 cmd,
 		 * we bail out here. */
 		if (res.Failed() || estimate_only ||
 				(!test_and_exec_can_differ && !CheckCompanyHasMoney(res))) {
+			cur_company.Restore();
 			return_dcpi(res, false);
 		}
 	}
@@ -642,6 +645,7 @@ CommandCost DoCommandPInternal(TileIndex tile, uint32 p1, uint32 p2, uint32 cmd,
 	 */
 	if (_networking && !(cmd & CMD_NETWORK_COMMAND)) {
 		NetworkSend_Command(tile, p1, p2, cmd & ~CMD_FLAGS_MASK, callback, text, _current_company);
+		cur_company.Restore();
 
 		/* Don't return anything special here; no error, no costs.
 		 * This way it's not handled by DoCommand and only the
@@ -656,8 +660,17 @@ CommandCost DoCommandPInternal(TileIndex tile, uint32 p1, uint32 p2, uint32 cmd,
 	 * use the construction one */
 	CommandCost res2 = proc(tile, flags | DC_EXEC, p1, p2, text);
 
-	/* Make sure nothing bad happened, like changing the current company. */
-	assert(cmd_id == CMD_COMPANY_CTRL || old_company == _current_company);
+	if (cmd_id == CMD_COMPANY_CTRL) {
+		cur_company.Trash();
+		/* We are a new company                  -> Switch to new local company.
+		 * We were closed down                   -> Switch to spectator
+		 * Some other company opened/closed down -> The outside function will switch back */
+		_current_company = _local_company;
+	} else {
+		/* Make sure nothing bad happened, like changing the current company. */
+		assert(exec_as_spectator ? _current_company == COMPANY_SPECTATOR : cur_company.Verify());
+		cur_company.Restore();
+	}
 
 	/* If the test and execution can differ, or we skipped the test
 	 * we have to check the return of the command. Otherwise we can
diff --git a/src/company_cmd.cpp b/src/company_cmd.cpp
index 93178dd80..fe3dc29a4 100644
--- a/src/company_cmd.cpp
+++ b/src/company_cmd.cpp
@@ -99,7 +99,9 @@ void SetLocalCompany(CompanyID new_company)
 	InvalidateWindowData(WC_SEND_NETWORK_MSG, DESTTYPE_TEAM, _local_company);
 #endif
 
-	_local_company = new_company;
+	assert(_current_company == _local_company);
+
+	_current_company = _local_company = new_company;
 
 	/* Delete any construction windows... */
 	DeleteConstructionWindows();
@@ -736,8 +738,6 @@ void CompanyNewsInformation::FillData(const Company *c, const Company *other)
  */
 CommandCost CmdCompanyCtrl(TileIndex tile, DoCommandFlag flags, uint32 p1, uint32 p2, const char *text)
 {
-	if (flags & DC_EXEC) _current_company = OWNER_NONE;
-
 	InvalidateWindowData(WC_COMPANY_LEAGUE, 0, 0);
 
 	switch (p1) {
@@ -783,8 +783,6 @@ CommandCost CmdCompanyCtrl(TileIndex tile, DoCommandFlag flags, uint32 p1, uint3
 					NetworkChangeCompanyPassword(_settings_client.network.default_company_pass);
 				}
 
-				_current_company = _local_company;
-
 				/* Now that we have a new company, broadcast our company settings to
 				 * all clients so everything is in sync */
 				SyncCompanySettings();
diff --git a/src/genworld.cpp b/src/genworld.cpp
index 3b0f2260d..e433fea33 100644
--- a/src/genworld.cpp
+++ b/src/genworld.cpp
@@ -34,6 +34,7 @@
 #include "town.h"
 #include "newgrf.h"
 #include "core/random_func.hpp"
+#include "core/backup_type.hpp"
 
 #include "table/sprites.h"
 
@@ -96,6 +97,9 @@ static void CleanupGeneration()
  */
 static void _GenerateWorld(void *)
 {
+	/* Make sure everything is done via OWNER_NONE. */
+	Backup<CompanyByte> _cur_company(_current_company, OWNER_NONE, FILE_LINE);
+
 	try {
 		_generating_world = true;
 		_genworld_mapgen_mutex->BeginCritical();
@@ -132,6 +136,7 @@ static void _GenerateWorld(void *)
 			/* only generate towns, tree and industries in newgame mode. */
 			if (_game_mode != GM_EDITOR) {
 				if (!GenerateTowns(_settings_game.economy.town_layout)) {
+					_cur_company.Restore();
 					HandleGeneratingWorldAbortion();
 					return;
 				}
@@ -164,7 +169,8 @@ static void _GenerateWorld(void *)
 		}
 
 		ResetObjectToPlace();
-		_local_company = _gw.lc;
+		_cur_company.Trash();
+		_current_company = _local_company = _gw.lc;
 
 		SetGeneratingWorldProgress(GWP_GAME_START, 1);
 		/* Call any callback */
@@ -185,6 +191,7 @@ static void _GenerateWorld(void *)
 			SaveOrLoad(name, SL_SAVE, AUTOSAVE_DIR);
 		}
 	} catch (...) {
+		if (_cur_company.IsValid()) _cur_company.Restore();
 		_generating_world = false;
 		_genworld_mapgen_mutex->EndCritical();
 		throw;
@@ -287,8 +294,6 @@ void GenerateWorld(GenWorldMode mode, uint size_x, uint size_y, bool reset_setti
 
 	/* This disables some commands and stuff */
 	SetLocalCompany(COMPANY_SPECTATOR);
-	/* Make sure everything is done via OWNER_NONE */
-	_current_company = OWNER_NONE;
 
 	/* Set the date before loading sprites as some newgrfs check it */
 	SetDate(ConvertYMDToDate(_settings_game.game_creation.starting_year, 0, 1));
diff --git a/src/network/network_command.cpp b/src/network/network_command.cpp
index 71724b509..2869d8b8b 100644
--- a/src/network/network_command.cpp
+++ b/src/network/network_command.cpp
@@ -153,6 +153,8 @@ void NetworkSyncCommandQueue(NetworkClientSocket *cs)
  */
 void NetworkExecuteLocalCommandQueue()
 {
+	assert(_current_company == _local_company);
+
 	while (_local_command_queue != NULL) {
 
 		/* The queue is always in order, which means
@@ -175,6 +177,9 @@ void NetworkExecuteLocalCommandQueue()
 		_local_command_queue = _local_command_queue->next;
 		free(cp);
 	}
+
+	/* Local company may have changed, so we should not restore the old value */
+	_current_company = _local_company;
 }
 
 /**
diff --git a/src/openttd.cpp b/src/openttd.cpp
index 0fca283a5..6e8e44973 100644
--- a/src/openttd.cpp
+++ b/src/openttd.cpp
@@ -809,7 +809,6 @@ static void MakeNewGameDone()
 	IConsoleCmdExec("exec scripts/game_start.scr 0");
 
 	SetLocalCompany(COMPANY_FIRST);
-	_current_company = _local_company;
 
 	InitializeRailGUI();
 
@@ -895,7 +894,6 @@ static void StartScenario()
 	StartupDisasters();
 
 	SetLocalCompany(COMPANY_FIRST);
-	_current_company = _local_company;
 	Company *c = Company::Get(COMPANY_FIRST);
 	c->settings = _settings_client.company;
 
@@ -1231,6 +1229,8 @@ void StateGameLoop()
 		NewsLoop();
 		cur_company.Restore();
 	}
+
+	assert(_current_company == _local_company);
 }
 
 /** Create an autosave. The default name is "autosave#.sav". However with
diff --git a/src/video/dedicated_v.cpp b/src/video/dedicated_v.cpp
index 37797f7e4..2ce43f4c1 100644
--- a/src/video/dedicated_v.cpp
+++ b/src/video/dedicated_v.cpp
@@ -263,7 +263,7 @@ void VideoDriver_Dedicated::MainLoop()
 	/* Load the dedicated server stuff */
 	_is_network_server = true;
 	_network_dedicated = true;
-	_local_company = COMPANY_SPECTATOR;
+	_current_company = _local_company = COMPANY_SPECTATOR;
 
 	/* If SwitchMode is SM_LOAD, it means that the user used the '-g' options */
 	if (_switch_mode != SM_LOAD) {
diff --git a/src/window.cpp b/src/window.cpp
index 4f147a670..f15293ea3 100644
--- a/src/window.cpp
+++ b/src/window.cpp
@@ -1918,16 +1918,9 @@ static bool MaybeBringWindowToFront(Window *w)
  */
 void HandleKeypress(uint32 raw_key)
 {
-	/*
-	 * During the generation of the world, there might be
-	 * another thread that is currently building for example
-	 * a road. To not interfere with those tasks, we should
-	 * NOT change the _current_company here.
-	 *
-	 * This is not necessary either, as the only events that
-	 * can be handled are the 'close application' events
-	 */
-	if (!IsGeneratingWorld()) _current_company = _local_company;
+	/* World generation is multithreaded and messes with companies.
+	 * But there is no company related window open anyway, so _current_company is not used. */
+	assert(IsGeneratingWorld() || _local_company == _current_company);
 
 	/* Setup event */
 	uint16 key     = GB(raw_key,  0, 16);
@@ -2083,6 +2076,10 @@ static void HandleKeyScrolling()
 
 static void MouseLoop(MouseClick click, int mousewheel)
 {
+	/* World generation is multithreaded and messes with companies.
+	 * But there is no company related window open anyway, so _current_company is not used. */
+	assert(IsGeneratingWorld() || _local_company == _current_company);
+
 	HandlePlacePresize();
 	UpdateTileSelection();
 
@@ -2183,21 +2180,14 @@ static void MouseLoop(MouseClick click, int mousewheel)
  */
 void HandleMouseEvents()
 {
+	/* World generation is multithreaded and messes with companies.
+	 * But there is no company related window open anyway, so _current_company is not used. */
+	assert(IsGeneratingWorld() || _local_company == _current_company);
+
 	static int double_click_time = 0;
 	static int double_click_x = 0;
 	static int double_click_y = 0;
 
-	/*
-	 * During the generation of the world, there might be
-	 * another thread that is currently building for example
-	 * a road. To not interfere with those tasks, we should
-	 * NOT change the _current_company here.
-	 *
-	 * This is not necessary either, as the only events that
-	 * can be handled are the 'close application' events
-	 */
-	if (!IsGeneratingWorld()) _current_company = _local_company;
-
 	/* Mouse event? */
 	MouseClick click = MC_NONE;
 	if (_left_button_down && !_left_button_clicked) {
@@ -2280,6 +2270,10 @@ static void CheckSoftLimit()
  */
 void InputLoop()
 {
+	/* World generation is multithreaded and messes with companies.
+	 * But there is no company related window open anyway, so _current_company is not used. */
+	assert(IsGeneratingWorld() || _local_company == _current_company);
+
 	CheckSoftLimit();
 	HandleKeyScrolling();