summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorPatric Stout <truebrain@openttd.org>2021-02-27 11:01:57 +0100
committerPatric Stout <github@truebrain.nl>2021-02-28 12:27:04 +0100
commit2d9062bfc193227866b033f55ef2b842da6e72a0 (patch)
treedb55a8d9b46c8acdd3f0f27eab0bfe769e58f35c /src
parent13889b6554e029554556af6524c9ec5542c593e2 (diff)
downloadopenttd-2d9062bfc193227866b033f55ef2b842da6e72a0.tar.xz
Fix: [Network] don't desync if client leaves before you finish downloading map
When you are downloading a map, all the commands are queued up for you. Clients joining/leaving is done by the network protocol, and as such are processed immediately. This means that by the time you are processing the commands, a client that triggered it, might already have left. So, all commands that do something with ClientID, shouldn't error on an invalid ClientID when DC_EXEC is set, but gracefully handle the command anyway, to make sure the game-state is kept in sync with all the clients that did execute the DoCommand while the now-gone client was still there. Additionally, in the small chance a client disconnects between the server validating a DoCommand and the command being executed, also just process the command as if the client was still there. Otherwise, lag or latency can cause clients that did not receive the disconnect yet to desync.
Diffstat (limited to 'src')
-rw-r--r--src/company_cmd.cpp11
-rw-r--r--src/goal.cpp5
2 files changed, 7 insertions, 9 deletions
diff --git a/src/company_cmd.cpp b/src/company_cmd.cpp
index 21f7125b7..4715efc9a 100644
--- a/src/company_cmd.cpp
+++ b/src/company_cmd.cpp
@@ -822,13 +822,6 @@ CommandCost CmdCompanyCtrl(TileIndex tile, DoCommandFlag flags, uint32 p1, uint3
ClientID client_id = (ClientID)p2;
NetworkClientInfo *ci = NetworkClientInfo::GetByClientID(client_id);
-#ifndef DEBUG_DUMP_COMMANDS
- /* When replaying the client ID is not a valid client; there
- * are actually no clients at all. However, the company has to
- * be created, otherwise we cannot rerun the game properly.
- * So only allow a nullptr client info in that case. */
- if (ci == nullptr) return CommandCost();
-#endif /* NOT DEBUG_DUMP_COMMANDS */
/* Delete multiplayer progress bar */
DeleteWindowById(WC_NETWORK_STATUS_WINDOW, WN_NETWORK_STATUS_WINDOW_JOIN);
@@ -837,7 +830,9 @@ CommandCost CmdCompanyCtrl(TileIndex tile, DoCommandFlag flags, uint32 p1, uint3
/* A new company could not be created, revert to being a spectator */
if (c == nullptr) {
- if (_network_server) {
+ /* We check for "ci != nullptr" as a client could have left by
+ * the time we execute this command. */
+ if (_network_server && ci != nullptr) {
ci->client_playas = COMPANY_SPECTATOR;
NetworkUpdateClientInfo(ci->client_id);
}
diff --git a/src/goal.cpp b/src/goal.cpp
index 8ce8e2337..63fdd4589 100644
--- a/src/goal.cpp
+++ b/src/goal.cpp
@@ -256,7 +256,10 @@ CommandCost CmdGoalQuestion(TileIndex tile, DoCommandFlag flags, uint32 p1, uint
if (_current_company != OWNER_DEITY) return CMD_ERROR;
if (StrEmpty(text)) return CMD_ERROR;
if (is_client) {
- if (NetworkClientInfo::GetByClientID(client) == nullptr) return CMD_ERROR;
+ /* Only check during pre-flight; the client might have left between
+ * testing and executing. In that case it is fine to just ignore the
+ * fact the client is no longer here. */
+ if (!(flags & DC_EXEC) && _network_server && NetworkClientInfo::GetByClientID(client) == nullptr) return CMD_ERROR;
} else {
if (company != INVALID_COMPANY && !Company::IsValidID(company)) return CMD_ERROR;
}