# HG changeset patch # User frosch # Date 1275744762 0 # Node ID c3d64c33d3d4479167dd00395e3daf4d77863c57 # Parent bebb7d6af4a3b5b7e0689aab2111f0970305c4cf (svn r19933) -Fix [FS#3804]: Keep _current_company and _local_company in sync during GUI operation. diff --git a/src/command.cpp b/src/command.cpp --- 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 @@ 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 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 @@ 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 @@ * 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 @@ */ 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 @@ * 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 --- a/src/company_cmd.cpp +++ b/src/company_cmd.cpp @@ -99,7 +99,9 @@ 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 @@ */ 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 @@ 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 --- 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 _GenerateWorld(void *) { + /* Make sure everything is done via OWNER_NONE. */ + Backup _cur_company(_current_company, OWNER_NONE, FILE_LINE); + try { _generating_world = true; _genworld_mapgen_mutex->BeginCritical(); @@ -132,6 +136,7 @@ /* 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 @@ } 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 @@ 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 @@ /* 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 --- a/src/network/network_command.cpp +++ b/src/network/network_command.cpp @@ -153,6 +153,8 @@ */ void NetworkExecuteLocalCommandQueue() { + assert(_current_company == _local_company); + while (_local_command_queue != NULL) { /* The queue is always in order, which means @@ -175,6 +177,9 @@ _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 --- a/src/openttd.cpp +++ b/src/openttd.cpp @@ -809,7 +809,6 @@ IConsoleCmdExec("exec scripts/game_start.scr 0"); SetLocalCompany(COMPANY_FIRST); - _current_company = _local_company; InitializeRailGUI(); @@ -895,7 +894,6 @@ StartupDisasters(); SetLocalCompany(COMPANY_FIRST); - _current_company = _local_company; Company *c = Company::Get(COMPANY_FIRST); c->settings = _settings_client.company; @@ -1231,6 +1229,8 @@ 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 --- a/src/video/dedicated_v.cpp +++ b/src/video/dedicated_v.cpp @@ -263,7 +263,7 @@ /* 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 --- a/src/window.cpp +++ b/src/window.cpp @@ -1918,16 +1918,9 @@ */ 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 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 @@ */ 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 @@ */ 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();