diff --git a/src/command.cpp b/src/command.cpp --- a/src/command.cpp +++ b/src/command.cpp @@ -274,53 +274,26 @@ void CommandHelperBase::InternalPostResu } } -/** Helper to format command parameters into a hex string. */ -static std::string CommandParametersToHexString(TileIndex tile, uint32 p1, uint32 p2, const std::string &text) +/** Helper to make a desync log for a command. */ +void CommandHelperBase::LogCommandExecution(Commands cmd, StringID err_message, TileIndex tile, const CommandDataBuffer &args, bool failed) { - return FormatArrayAsHex(EndianBufferWriter<>::FromValue(std::make_tuple(tile, p1, p2, text))); + Debug(desync, 1, "{}: {:08x}; {:02x}; {:02x}; {:08x}; {:08x}; {:06x}; {} ({})", failed ? "cmdf" : "cmd", _date, _date_fract, (int)_current_company, cmd, err_message, tile, FormatArrayAsHex(args), GetCommandName(cmd)); } -/*! - * Helper function for the toplevel network safe docommand function for the current company. - * - * @param cmd The command to execute (a CMD_* value) - * @param err_message Message prefix to show on error - * @param callback A callback function to call after the command is finished - * @param my_cmd indicator if the command is from a company or server (to display error messages for a user) - * @param estimate_only whether to give only the estimate or also execute the command - * @param tile The tile to perform a command on (see #CommandProc) - * @param p1 Additional data for the command (see #CommandProc) - * @param p2 Additional data for the command (see #CommandProc) - * @param text The text to pass - * @return the command cost of this function. +/** + * Prepare for the test run of a command proc call. + * @param cmd_flags Command flags. + * @param tile Tile of command execution. + * @param[in,out] cur_company Backup of current company at start of command execution. + * @return True if test run can go ahead, false on error. */ -CommandCost DoCommandPInternal(Commands cmd, StringID err_message, CommandCallback *callback, bool my_cmd, bool estimate_only, bool network_command, TileIndex tile, uint32 p1, uint32 p2, const std::string &text) +bool CommandHelperBase::InternalExecutePrepTest(CommandFlags cmd_flags, TileIndex tile, Backup &cur_company) { - RecursiveCommandCounter counter{}; - - /* Prevent recursion; it gives a mess over the network */ - assert(counter.IsTopLevel()); - /* Reset the state. */ _additional_cash_required = 0; - /* Get pointer to command handler */ - assert(cmd < _command_proc_table.size()); - CommandProc *proc = _command_proc_table[cmd].proc; - /* Shouldn't happen, but you never know when someone adds - * NULLs to the _command_proc_table. */ - assert(proc != nullptr); - - /* Command flags are used internally */ - CommandFlags cmd_flags = GetCommandFlags(cmd); - /* Flags get send to the DoCommand */ - DoCommandFlag flags = CommandFlagsToDCFlags(cmd_flags); - - /* Make sure p2 is properly set to a ClientID. */ - assert(!(cmd_flags & CMD_CLIENT_ID) || p2 != 0); - /* Do not even think about executing out-of-bounds tile-commands */ - if (tile != 0 && (tile >= MapSize() || (!IsValidTile(tile) && (cmd_flags & CMD_ALL_TILES) == 0))) return CMD_ERROR; + if (tile != 0 && (tile >= MapSize() || (!IsValidTile(tile) && (cmd_flags & CMD_ALL_TILES) == 0))) return false; /* Always execute server and spectator commands as spectator */ bool exec_as_spectator = (cmd_flags & (CMD_SPECTATOR | CMD_SERVER)) != 0; @@ -329,62 +302,68 @@ CommandCost DoCommandPInternal(Commands * 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 && !exec_as_spectator && !Company::IsValidID(_current_company) && !(_current_company == OWNER_DEITY && (cmd_flags & CMD_DEITY) != 0)) { - return CMD_ERROR; + return 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; - - /* Test the command. */ + /* Enter test mode. */ _cleared_object_areas.clear(); SetTownRatingTestMode(true); BasePersistentStorageArray::SwitchMode(PSM_ENTER_TESTMODE); - CommandCost res = proc(flags, tile, p1, p2, text); + return true; +} + +/** + * Validate result of test run and prepare for real execution. + * @param cmd_flags Command flags. + * @param[in,out] res Command result of test run, may be modified. + * @param estimate_only Is this just cost estimation? + * @param network_command Does this command come from the network? + * @param[in,out] cur_company Backup of current company at start of command execution. + * @return True if test run can go ahead, false on error. + */ +std::tuple CommandHelperBase::InternalExecuteValidateTestAndPrepExec(CommandCost &res, CommandFlags cmd_flags, bool estimate_only, bool network_command, Backup &cur_company) +{ BasePersistentStorageArray::SwitchMode(PSM_LEAVE_TESTMODE); SetTownRatingTestMode(false); /* Make sure we're not messing things up here. */ - assert(exec_as_spectator ? _current_company == COMPANY_SPECTATOR : cur_company.Verify()); + assert((cmd_flags & (CMD_SPECTATOR | CMD_SERVER)) != 0 ? _current_company == COMPANY_SPECTATOR : cur_company.Verify()); /* If the command fails, we're doing an estimate * or the player does not have enough money * (unless it's a command where the test and * execution phase might return different costs) * we bail out here. */ - if (res.Failed() || estimate_only || - (!test_and_exec_can_differ && !CheckCompanyHasMoney(res))) { - if (!_networking || _generating_world || network_command) { - /* Log the failed command as well. Just to be able to be find - * causes of desyncs due to bad command test implementations. */ - Debug(desync, 1, "cmdf: {:08x}; {:02x}; {:02x}; {:08x}; {:08x}; {:06x}; {} ({})", _date, _date_fract, (int)_current_company, cmd, err_message, tile, CommandParametersToHexString(tile, p1, p2, text), GetCommandName(cmd)); - } - cur_company.Restore(); - return res; + bool test_and_exec_can_differ = (cmd_flags & CMD_NO_TEST) != 0; + if (res.Failed() || estimate_only || (!test_and_exec_can_differ && !CheckCompanyHasMoney(res))) { + return { true, !_networking || _generating_world || network_command, false }; + } + + bool send_net = _networking && !_generating_world && !network_command; + + if (!send_net) { + /* Prepare for command execution. */ + _cleared_object_areas.clear(); + BasePersistentStorageArray::SwitchMode(PSM_ENTER_COMMAND); } - /* - * If we are in network, and the command is not from the network - * send it to the command-queue and abort execution - */ - if (_networking && !_generating_world && !network_command) { - NetworkSendCommand(cmd, err_message, callback, _current_company, tile, p1, p2, text); - cur_company.Restore(); + return { false, _debug_desync_level >= 1, send_net }; +} - /* Don't return anything special here; no error, no costs. - * This way it's not handled by DoCommand and only the - * actual execution of the command causes messages. Also - * reset the storages as we've not executed the command. */ - return CommandCost(); - } - Debug(desync, 1, "cmd: {:08x}; {:02x}; {:02x}; {:08x}; {:08x}; {:06x}; {} ({})", _date, _date_fract, (int)_current_company, cmd, err_message, tile, CommandParametersToHexString(tile, p1, p2, text), GetCommandName(cmd)); - - /* Actually try and execute the command. If no cost-type is given - * use the construction one */ - _cleared_object_areas.clear(); - BasePersistentStorageArray::SwitchMode(PSM_ENTER_COMMAND); - CommandCost res2 = proc(flags | DC_EXEC, tile, p1, p2, text); +/** + * Process the result of a command test run and execution run. + * @param cmd Command that was executed. + * @param cmd_flags Command flags. + * @param res_test Command result of test run. + * @param tes_exec Command result of real run. + * @param tile Tile of command execution. + * @param[in,out] cur_company Backup of current company at start of command execution. + * @return Final command result. + */ +CommandCost CommandHelperBase::InternalExecuteProcessResult(Commands cmd, CommandFlags cmd_flags, const CommandCost &res_test, const CommandCost &res_exec, TileIndex tile, Backup &cur_company) +{ BasePersistentStorageArray::SwitchMode(PSM_LEAVE_COMMAND); if (cmd == CMD_COMPANY_CTRL) { @@ -395,7 +374,7 @@ CommandCost DoCommandPInternal(Commands _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()); + assert((cmd_flags & (CMD_SPECTATOR | CMD_SERVER)) != 0 ? _current_company == COMPANY_SPECTATOR : cur_company.Verify()); cur_company.Restore(); } @@ -403,15 +382,16 @@ CommandCost DoCommandPInternal(Commands * return of the command. Otherwise we can check whether the * test and execution have yielded the same result, * i.e. cost and error state are the same. */ + bool test_and_exec_can_differ = (cmd_flags & CMD_NO_TEST) != 0; if (!test_and_exec_can_differ) { - assert(res.GetCost() == res2.GetCost() && res.Failed() == res2.Failed()); // sanity check - } else if (res2.Failed()) { - return res2; + assert(res_test.GetCost() == res_exec.GetCost() && res_test.Failed() == res_exec.Failed()); // sanity check + } else if (res_exec.Failed()) { + return res_exec; } /* If we're needing more money and we haven't done * anything yet, ask for the money! */ - if (_additional_cash_required != 0 && res2.GetCost() == 0) { + if (_additional_cash_required != 0 && res_exec.GetCost() == 0) { /* It could happen we removed rail, thus gained money, and deleted something else. * So make sure the signal buffer is empty even in this case */ UpdateSignalsInBuffer(); @@ -425,12 +405,12 @@ CommandCost DoCommandPInternal(Commands if (c != nullptr) c->last_build_coordinate = tile; } - SubtractMoneyFromCompany(res2); + SubtractMoneyFromCompany(res_exec); /* update signals if needed */ UpdateSignalsInBuffer(); - return res2; + return res_exec; } diff --git a/src/command_func.h b/src/command_func.h --- a/src/command_func.h +++ b/src/command_func.h @@ -12,6 +12,8 @@ #include "command_type.h" #include "company_type.h" +#include "company_func.h" +#include "core/backup_type.hpp" #include "misc/endian_buffer.hpp" #include "tile_map.h" @@ -34,9 +36,6 @@ static const CommandCost CMD_ERROR = Com */ #define return_cmd_error(errcode) return CommandCost(errcode); -CommandCost DoCommandPInternal(Commands cmd, StringID err_message, CommandCallback *callback, bool my_cmd, bool estimate_only, bool network_command, TileIndex tile, uint32 p1, uint32 p2, const std::string &text); - -void NetworkSendCommand(Commands cmd, StringID err_message, CommandCallback *callback, CompanyID company, TileIndex tile, uint32 p1, uint32 p2, const std::string &text); void NetworkSendCommand(Commands cmd, StringID err_message, CommandCallback *callback, CompanyID company, TileIndex location, const CommandDataBuffer &cmd_data); extern Money _additional_cash_required; @@ -87,6 +86,10 @@ protected: static void InternalDoAfter(CommandCost &res, DoCommandFlag flags, bool top_level, bool test); static std::tuple InternalPostBefore(Commands cmd, CommandFlags flags, TileIndex tile, StringID err_message, bool network_command); static void InternalPostResult(const CommandCost &res, TileIndex tile, bool estimate_only, bool only_sending, StringID err_message, bool my_cmd); + static bool InternalExecutePrepTest(CommandFlags cmd_flags, TileIndex tile, Backup &cur_company); + static std::tuple InternalExecuteValidateTestAndPrepExec(CommandCost &res, CommandFlags cmd_flags, bool estimate_only, bool network_command, Backup &cur_company); + static CommandCost InternalExecuteProcessResult(Commands cmd, CommandFlags cmd_flags, const CommandCost &res_test, const CommandCost &res_exec, TileIndex tile, Backup &cur_company); + static void LogCommandExecution(Commands cmd, StringID err_message, TileIndex tile, const CommandDataBuffer &args, bool failed); }; /** @@ -186,6 +189,41 @@ public: return InternalPost(err_message, callback, my_cmd, true, location, std::move(args)); } + /** + * Prepare a command to be send over the network + * @param cmd The command to execute (a CMD_* value) + * @param err_message Message prefix to show on error + * @param callback A callback function to call after the command is finished + * @param company The company that wants to send the command + * @param args Parameters for the command + */ + static void SendNet(StringID err_message, CommandCallback *callback, CompanyID company, Targs... args) + { + auto args_tuple = std::forward_as_tuple(args...); + + TileIndex tile{}; + if constexpr (std::is_same_v>) { + tile = std::get<0>(args_tuple); + } + + ::NetworkSendCommand(Tcmd, err_message, callback, _current_company, tile, EndianBufferWriter::FromValue(args_tuple)); + } + + /** + * Top-level network safe command execution without safety checks. + * @param err_message Message prefix to show on error + * @param callback A callback function to call after the command is finished + * @param my_cmd indicator if the command is from a company or server (to display error messages for a user) + * @param estimate_only whether to give only the estimate or also execute the command + * @param location Tile location for user feedback. + * @param args Parameters for the command + * @return the command cost of this function. + */ + static CommandCost Unsafe(StringID err_message, CommandCallback *callback, bool my_cmd, bool estimate_only, TileIndex location, std::tuple args) + { + return Execute(err_message, callback, my_cmd, estimate_only, false, location, std::move(args)); + } + protected: static bool InternalPost(StringID err_message, CommandCallback *callback, bool my_cmd, bool network_command, std::tuple args) { @@ -206,7 +244,7 @@ protected: /* Only set p2 when the command does not come from the network. */ if (!network_command && GetCommandFlags() & CMD_CLIENT_ID && std::get<2>(args) == 0) std::get<2>(args) = CLIENT_ID_SERVER; - CommandCost res = std::apply(DoCommandPInternal, std::tuple_cat(std::make_tuple(Tcmd, err_message, callback, my_cmd, estimate_only, network_command), args)); + CommandCost res = Execute(err_message, callback, my_cmd, estimate_only, network_command, tile, args); InternalPostResult(res, tile, estimate_only, only_sending, err_message, my_cmd); if (!estimate_only && !only_sending && callback != nullptr) { @@ -215,6 +253,56 @@ protected: return res.Succeeded(); } + + static CommandCost Execute(StringID err_message, CommandCallback *callback, bool my_cmd, bool estimate_only, bool network_command, TileIndex tile, std::tuple args) + { + /* Prevent recursion; it gives a mess over the network */ + RecursiveCommandCounter counter{}; + assert(counter.IsTopLevel()); + + /* Command flags are used internally */ + CommandFlags cmd_flags = GetCommandFlags(); + + /* Make sure p2 is properly set to a ClientID also when processing external commands. */ + assert(!(cmd_flags & CMD_CLIENT_ID) || std::get<2>(args) != 0); + + Backup cur_company(_current_company, FILE_LINE); + if (!InternalExecutePrepTest(cmd_flags, tile, cur_company)) { + cur_company.Trash(); + return CMD_ERROR; + } + + /* Test the command. */ + DoCommandFlag flags = CommandFlagsToDCFlags(cmd_flags); + CommandCost res = std::apply(CommandTraits::proc, std::tuple_cat(std::make_tuple(flags), args)); + + auto [exit_test, desync_log, send_net] = InternalExecuteValidateTestAndPrepExec(res, cmd_flags, estimate_only, network_command, cur_company); + if (exit_test) { + if (desync_log) LogCommandExecution(Tcmd, err_message, tile, EndianBufferWriter::FromValue(args), true); + cur_company.Restore(); + return res; + } + + /* If we are in network, and the command is not from the network + * send it to the command-queue and abort execution. */ + if (send_net) { + ::NetworkSendCommand(Tcmd, err_message, callback, _current_company, tile, EndianBufferWriter::FromValue(args)); + cur_company.Restore(); + + /* Don't return anything special here; no error, no costs. + * This way it's not handled by DoCommand and only the + * actual execution of the command causes messages. Also + * reset the storages as we've not executed the command. */ + return CommandCost(); + } + + if (desync_log) LogCommandExecution(Tcmd, err_message, tile, EndianBufferWriter::FromValue(args), false); + + /* Actually try and execute the command. */ + CommandCost res2 = std::apply(CommandTraits::proc, std::tuple_cat(std::make_tuple(flags | DC_EXEC), args)); + + return InternalExecuteProcessResult(Tcmd, cmd_flags, res, res2, tile, cur_company); + } }; template diff --git a/src/network/network_client.cpp b/src/network/network_client.cpp --- a/src/network/network_client.cpp +++ b/src/network/network_client.cpp @@ -18,6 +18,7 @@ #include "../company_func.h" #include "../company_base.h" #include "../company_gui.h" +#include "../company_cmd.h" #include "../core/random_func.hpp" #include "../date_func.h" #include "../gfx_func.h" @@ -839,7 +840,7 @@ NetworkRecvStatus ClientNetworkGameSocke * the server will give us a client-id and let us in */ _network_join_status = NETWORK_JOIN_STATUS_REGISTERING; ShowJoinStatusWindow(); - NetworkSendCommand(CMD_COMPANY_CTRL, STR_NULL, nullptr, _local_company, 0, CCA_NEW, 0, {}); + Command::SendNet(STR_NULL, nullptr, _local_company, 0, CCA_NEW, 0, {}); } } else { /* take control over an existing company */ 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 @@ -176,22 +176,6 @@ static CommandQueue _local_wait_queue; /** Local queue of packets waiting for execution. */ static CommandQueue _local_execution_queue; -/** - * Prepare a DoCommand to be send over the network - * @param cmd The command to execute (a CMD_* value) - * @param err_message Message prefix to show on error - * @param callback A callback function to call after the command is finished - * @param company The company that wants to send the command - * @param tile The tile to perform a command on (see #CommandProc) - * @param p1 Additional data for the command (see #CommandProc) - * @param p2 Additional data for the command (see #CommandProc) - * @param text The text to pass - */ -void NetworkSendCommand(Commands cmd, StringID err_message, CommandCallback *callback, CompanyID company, TileIndex tile, uint32 p1, uint32 p2, const std::string &text) -{ - auto data = EndianBufferWriter::FromValue(std::make_tuple(tile, p1, p2, text)); - NetworkSendCommand(cmd, err_message, callback, company, tile, data); -} /** * Prepare a DoCommand to be send over the network diff --git a/src/network/network_gui.cpp b/src/network/network_gui.cpp --- a/src/network/network_gui.cpp +++ b/src/network/network_gui.cpp @@ -1538,7 +1538,7 @@ private: if (_network_server) { Command::Post(0, CCA_NEW, _network_own_client_id, {}); } else { - NetworkSendCommand(CMD_COMPANY_CTRL, STR_NULL, nullptr, _local_company, 0, CCA_NEW, 0, {}); + Command::SendNet(STR_NULL, nullptr, _local_company, 0, CCA_NEW, 0, {}); } } diff --git a/src/network/network_server.cpp b/src/network/network_server.cpp --- a/src/network/network_server.cpp +++ b/src/network/network_server.cpp @@ -2081,7 +2081,7 @@ void NetworkServerNewCompany(const Compa /* ci is nullptr when replaying, or for AIs. In neither case there is a client. */ ci->client_playas = c->index; NetworkUpdateClientInfo(ci->client_id); - NetworkSendCommand(CMD_RENAME_PRESIDENT, STR_NULL, nullptr, c->index, 0, 0, 0, ci->client_name); + Command::SendNet(STR_NULL, nullptr, c->index, 0, 0, 0, ci->client_name); } /* Announce new company on network. */ diff --git a/src/order_backup.cpp b/src/order_backup.cpp --- a/src/order_backup.cpp +++ b/src/order_backup.cpp @@ -200,7 +200,7 @@ CommandCost CmdClearOrderBackup(DoComman /* We need to circumvent the "prevention" from this command being executed * while the game is paused, so use the internal method. Nor do we want * this command to get its cost estimated when shift is pressed. */ - DoCommandPInternal(CMD_CLEAR_ORDER_BACKUP, STR_NULL, nullptr, true, false, false, ob->tile, 0, user, {}); + Command::Unsafe(STR_NULL, nullptr, true, false, ob->tile, CommandTraits::Args{ ob->tile, 0, user, {} }); } else { /* The command came from the game logic, i.e. the clearing of a tile. * In that case we have no need to actually sync this, just do it. */ diff --git a/src/script/api/script_object.hpp b/src/script/api/script_object.hpp --- a/src/script/api/script_object.hpp +++ b/src/script/api/script_object.hpp @@ -371,7 +371,7 @@ bool ScriptObject::ScriptDoCommandHelper if (!estimate_only && networking) ScriptObject::SetLastCommand(tile, EndianBufferWriter::FromValue(args), Tcmd); /* Try to perform the command. */ - CommandCost res = std::apply(&DoCommandPInternal, std::tuple_cat(std::make_tuple(Tcmd, (StringID)0, networking ? ScriptObject::GetDoCommandCallback() : nullptr, false, estimate_only, false), args)); + CommandCost res = ::Command::Unsafe((StringID)0, networking ? ScriptObject::GetDoCommandCallback() : nullptr, false, estimate_only, tile, args); return ScriptObject::DoCommandProcessResult(res, callback, estimate_only); } diff --git a/src/settings.cpp b/src/settings.cpp --- a/src/settings.cpp +++ b/src/settings.cpp @@ -1604,7 +1604,7 @@ void SyncCompanySettings() const SettingDesc *sd = GetSettingDesc(desc); uint32 old_value = (uint32)sd->AsIntSetting()->Read(new_object); uint32 new_value = (uint32)sd->AsIntSetting()->Read(old_object); - if (old_value != new_value) NetworkSendCommand(CMD_CHANGE_COMPANY_SETTING, STR_NULL, nullptr, _local_company, 0, 0, new_value, sd->GetName()); + if (old_value != new_value) Command::SendNet(STR_NULL, nullptr, _local_company, 0, 0, new_value, sd->GetName()); } }