From 072ecadd4810b62c9a96a4d599663cf16036d9e7 Mon Sep 17 00:00:00 2001 From: ZehMatt Date: Thu, 20 Jul 2017 19:28:56 +0200 Subject: [PATCH] Fix leaking memory creating game actions. Specialized class use for game action results. --- src/openrct2/actions/GameAction.cpp | 55 +++++++++--------- src/openrct2/actions/GameAction.h | 11 ++-- src/openrct2/actions/GameActionCompat.cpp | 12 ++-- src/openrct2/actions/IGameAction.h | 25 ++++----- .../actions/PlaceParkEntranceAction.hpp | 28 ++++++---- src/openrct2/actions/RideCreateAction.hpp | 56 +++++++++---------- src/openrct2/actions/RideSetStatus.hpp | 22 ++++---- .../actions/SetParkEntranceFeeAction.hpp | 12 ++-- src/openrct2/network/Network.cpp | 34 ++++------- src/openrct2/network/network.h | 27 ++------- 10 files changed, 129 insertions(+), 153 deletions(-) diff --git a/src/openrct2/actions/GameAction.cpp b/src/openrct2/actions/GameAction.cpp index 0f2773c892..4fd4dc107e 100644 --- a/src/openrct2/actions/GameAction.cpp +++ b/src/openrct2/actions/GameAction.cpp @@ -63,7 +63,7 @@ namespace GameActions initialized = true; } - IGameAction * Create(uint32 id) + std::unique_ptr Create(uint32 id) { Initialize(); @@ -76,7 +76,7 @@ namespace GameActions result = factory(); } } - return result; + return std::unique_ptr(result); } static bool CheckActionInPausedMode(uint32 actionFlags) @@ -95,42 +95,43 @@ namespace GameActions return false; } - GameActionResult Query(const IGameAction * action) + GameActionResult::Ptr Query(const IGameAction * action) { Guard::ArgumentNotNull(action); - GameActionResult result; uint16 actionFlags = action->GetActionFlags(); if (!CheckActionInPausedMode(actionFlags)) { - result.Error = GA_ERROR::GAME_PAUSED; - result.ErrorMessage = STR_CONSTRUCTION_NOT_POSSIBLE_WHILE_GAME_IS_PAUSED; + GameActionResult::Ptr result = std::make_unique(); + + result->Error = GA_ERROR::GAME_PAUSED; + result->ErrorMessage = STR_CONSTRUCTION_NOT_POSSIBLE_WHILE_GAME_IS_PAUSED; + + return result; } - else + + auto result = action->Query(); + if (result->Error == GA_ERROR::OK) { - result = action->Query(); - if (result.Error == GA_ERROR::OK) + if (!CheckActionAffordability(result.get())) { - if (!CheckActionAffordability(&result)) - { - result.Error = GA_ERROR::INSUFFICIENT_FUNDS; - result.ErrorMessage = STR_NOT_ENOUGH_CASH_REQUIRES; - set_format_arg_on(result.ErrorMessageArgs, 0, uint32, result.Cost); - } + result->Error = GA_ERROR::INSUFFICIENT_FUNDS; + result->ErrorMessage = STR_NOT_ENOUGH_CASH_REQUIRES; + set_format_arg_on(result->ErrorMessageArgs, 0, uint32, result->Cost); } } return result; } - GameActionResult Execute(const IGameAction * action) + GameActionResult::Ptr Execute(const IGameAction * action) { Guard::ArgumentNotNull(action); uint16 actionFlags = action->GetActionFlags(); uint32 flags = action->GetFlags(); - GameActionResult result = Query(action); - if (result.Error == GA_ERROR::OK) + GameActionResult::Ptr result = Query(action); + if (result->Error == GA_ERROR::OK) { // Networked games send actions to the server to be run if (network_get_mode() == NETWORK_MODE_CLIENT) @@ -164,21 +165,21 @@ namespace GameActions result = action->Execute(); // Update money balance - if (!(gParkFlags & PARK_FLAGS_NO_MONEY) && result.Cost != 0) + if (!(gParkFlags & PARK_FLAGS_NO_MONEY) && result->Cost != 0) { - finance_payment(result.Cost, result.ExpenditureType); - money_effect_create(result.Cost); + finance_payment(result->Cost, result->ExpenditureType); + money_effect_create(result->Cost); } if (!(actionFlags & GA_FLAGS::CLIENT_ONLY)) { - if (network_get_mode() == NETWORK_MODE_SERVER && result.Error == GA_ERROR::OK) + if (network_get_mode() == NETWORK_MODE_SERVER && result->Error == GA_ERROR::OK) { uint32 playerId = action->GetPlayer(); network_set_player_last_action(network_get_player_index(playerId), action->GetType()); - if(result.Cost != 0) - network_add_player_money_spent(playerId, result.Cost); + if(result->Cost != 0) + network_add_player_money_spent(playerId, result->Cost); } } @@ -196,11 +197,11 @@ namespace GameActions cb(action, result); } - if (result.Error != GA_ERROR::OK && !(flags & GAME_COMMAND_FLAG_GHOST)) + if (result->Error != GA_ERROR::OK && !(flags & GAME_COMMAND_FLAG_GHOST)) { // Show the error box - Memory::Copy(gCommonFormatArgs, result.ErrorMessageArgs, sizeof(result.ErrorMessageArgs)); - window_error_open(result.ErrorTitle, result.ErrorMessage); + Memory::Copy(gCommonFormatArgs, result->ErrorMessageArgs, sizeof(result->ErrorMessageArgs)); + window_error_open(result->ErrorTitle, result->ErrorMessage); } return result; } diff --git a/src/openrct2/actions/GameAction.h b/src/openrct2/actions/GameAction.h index 28e564e662..3d31fe6454 100644 --- a/src/openrct2/actions/GameAction.h +++ b/src/openrct2/actions/GameAction.h @@ -20,6 +20,7 @@ extern "C" { #include "../platform/platform.h" } +#include #include "IGameAction.h" typedef IGameAction *(*GameActionFactory)(); @@ -108,21 +109,21 @@ public: /** * Query the result of the game action without changing the game state. */ - virtual GameActionResult Query() const override abstract; + virtual GameActionResult::Ptr Query() const override abstract; /** * Apply the game action and change the game state. */ - virtual GameActionResult Execute() const override abstract; + virtual GameActionResult::Ptr Execute() const override abstract; }; namespace GameActions { void Initialize(); void Register(); -IGameAction * Create(uint32 id); -GameActionResult Query(const IGameAction * action); -GameActionResult Execute(const IGameAction * action); +IGameAction::Ptr Create(uint32 id); +GameActionResult::Ptr Query(const IGameAction * action); +GameActionResult::Ptr Execute(const IGameAction * action); GameActionFactory Register(uint32 id, GameActionFactory action); diff --git a/src/openrct2/actions/GameActionCompat.cpp b/src/openrct2/actions/GameActionCompat.cpp index 0666148863..906cdbba0c 100644 --- a/src/openrct2/actions/GameActionCompat.cpp +++ b/src/openrct2/actions/GameActionCompat.cpp @@ -14,7 +14,7 @@ extern "C" gameAction.z = z; gameAction.direction = direction; auto result = GameActions::Execute(&gameAction); - if (result.Error == GA_ERROR::OK) + if (result->Error == GA_ERROR::OK) { return 0; } @@ -55,7 +55,7 @@ extern "C" gameAction.SetFlags(GAME_COMMAND_FLAG_GHOST); auto result = GameActions::Execute(&gameAction); - if (result.Error == GA_ERROR::OK) + if (result->Error == GA_ERROR::OK) { gParkEntranceGhostPosition.x = x; gParkEntranceGhostPosition.y = y; @@ -63,7 +63,7 @@ extern "C" gParkEntranceGhostDirection = direction; gParkEntranceGhostExists = true; } - return result.Cost; + return result->Cost; } #pragma endregion @@ -93,12 +93,12 @@ extern "C" auto gameAction = RideCreateAction(); gameAction.rideType = listItem.type; gameAction.rideSubType = listItem.entry_index; - gameAction.SetCallback([](const IGameAction *ga, GameActionResult& res) + gameAction.SetCallback([](const IGameAction *ga, const GameActionResult::Ptr& res) { - if (res.Error != GA_ERROR::OK) + if (res->Error != GA_ERROR::OK) return; - ride_construct(static_cast(res).RideIndex()); + ride_construct(static_cast(res.get())->rideIndex); }); GameActions::Execute(&gameAction); diff --git a/src/openrct2/actions/IGameAction.h b/src/openrct2/actions/IGameAction.h index 1fd26ceff0..4840a22089 100644 --- a/src/openrct2/actions/IGameAction.h +++ b/src/openrct2/actions/IGameAction.h @@ -17,6 +17,8 @@ #pragma once #include +#include + #include "../common.h" #include "../core/IStream.hpp" #include "../core/DataSerialiser.h" @@ -62,6 +64,8 @@ constexpr uint16 EDITOR_ONLY = 1 << 2; */ struct GameActionResult { + typedef std::unique_ptr Ptr; + GA_ERROR Error = GA_ERROR::OK; rct_string_id ErrorTitle = (rct_string_id)-1; rct_string_id ErrorMessage = (rct_string_id)-1; @@ -70,23 +74,12 @@ struct GameActionResult money32 Cost = 0; uint16 ExpenditureType = 0; - union - { - sint8 _sint8; - sint16 _sint16; - sint32 _sint32; - sint64 _sint64; - uint8 _uint8; - uint16 _uint16; - uint32 _uint32; - uint64 _uint64; - } Results[4]; - GameActionResult(); GameActionResult(GA_ERROR error, rct_string_id message); + GameActionResult(const GameActionResult&) = delete; }; -typedef std::function GameActionCallback_t; +typedef std::function GameActionCallback_t; /** * Represents an action that changes the state of the game. Can be serialised and @@ -95,6 +88,8 @@ typedef std::function GameAc struct IGameAction { public: + typedef std::unique_ptr Ptr; + /** * Gets the GA_FLAGS flags that are enabled for this game action. */ @@ -135,11 +130,11 @@ public: /** * Query the result of the game action without changing the game state. */ - virtual GameActionResult Query() const abstract; + virtual GameActionResult::Ptr Query() const abstract; /** * Apply the game action and change the game state. */ virtual ~IGameAction() {}; - virtual GameActionResult Execute() const abstract; + virtual GameActionResult::Ptr Execute() const abstract; }; diff --git a/src/openrct2/actions/PlaceParkEntranceAction.hpp b/src/openrct2/actions/PlaceParkEntranceAction.hpp index dc854ebac9..a4c2c6a830 100644 --- a/src/openrct2/actions/PlaceParkEntranceAction.hpp +++ b/src/openrct2/actions/PlaceParkEntranceAction.hpp @@ -28,8 +28,10 @@ extern "C" #include "../world/footpath.h" } -struct PlaceParkEntranceGameActionResult : public GameActionResult { - PlaceParkEntranceGameActionResult(GA_ERROR error, rct_string_id message) :GameActionResult(error, message) +struct PlaceParkEntranceGameActionResult : public GameActionResult +{ + PlaceParkEntranceGameActionResult() : GameActionResult(GA_ERROR::OK, 0) {} + PlaceParkEntranceGameActionResult(GA_ERROR error, rct_string_id message) : GameActionResult(error, message) { ErrorTitle = STR_CANT_BUILD_PARK_ENTRANCE_HERE; } @@ -50,11 +52,11 @@ public: stream << x << y << z << direction; } - GameActionResult Query() const override + GameActionResult::Ptr Query() const override { if (!(gScreenFlags & SCREEN_FLAGS_EDITOR) && !gCheatsSandboxMode) { - return PlaceParkEntranceGameActionResult(GA_ERROR::NOT_IN_EDITOR_MODE, STR_NONE); + return std::make_unique(GA_ERROR::NOT_IN_EDITOR_MODE, STR_NONE); } gCommandExpenditureType = RCT_EXPENDITURE_TYPE_LAND_PURCHASE; @@ -65,12 +67,12 @@ public: if (!map_check_free_elements_and_reorganise(3)) { - return PlaceParkEntranceGameActionResult(GA_ERROR::NO_FREE_ELEMENTS, STR_NONE); + return std::make_unique(GA_ERROR::NO_FREE_ELEMENTS, STR_NONE); } if (x <= 32 || y <= 32 || x >= (gMapSizeUnits - 32) || y >= (gMapSizeUnits - 32)) { - return PlaceParkEntranceGameActionResult(GA_ERROR::INVALID_PARAMETERS, STR_TOO_CLOSE_TO_EDGE_OF_MAP); + return std::make_unique(GA_ERROR::INVALID_PARAMETERS, STR_TOO_CLOSE_TO_EDGE_OF_MAP); } sint8 entranceNum = -1; @@ -85,7 +87,7 @@ public: if (entranceNum == -1) { - return PlaceParkEntranceGameActionResult(GA_ERROR::INVALID_PARAMETERS, STR_ERR_TOO_MANY_PARK_ENTRANCES); + return std::make_unique(GA_ERROR::INVALID_PARAMETERS, STR_ERR_TOO_MANY_PARK_ENTRANCES); } sint8 zLow = z * 2; @@ -108,7 +110,7 @@ public: { if (!map_can_construct_at(entranceLoc.x, entranceLoc.y, zLow, zHigh, 0xF)) { - return PlaceParkEntranceGameActionResult(GA_ERROR::NO_CLEARANCE, STR_NONE); + return std::make_unique(GA_ERROR::NO_CLEARANCE, STR_NONE); } } @@ -116,13 +118,14 @@ public: rct_map_element* entranceElement = map_get_park_entrance_element_at(entranceLoc.x, entranceLoc.y, zLow, false); if (entranceElement != NULL) { - return PlaceParkEntranceGameActionResult(GA_ERROR::ITEM_ALREADY_PLACED, STR_NONE); + return std::make_unique(GA_ERROR::ITEM_ALREADY_PLACED, STR_NONE); } } - return GameActionResult(); + + return std::make_unique(); } - GameActionResult Execute() const override + GameActionResult::Ptr Execute() const override { uint32 flags = GetFlags(); @@ -204,6 +207,7 @@ public: map_animation_create(MAP_ANIMATION_TYPE_PARK_ENTRANCE, entranceLoc.x, entranceLoc.y, zLow); } } - return GameActionResult(); + + return std::make_unique(); } }; diff --git a/src/openrct2/actions/RideCreateAction.hpp b/src/openrct2/actions/RideCreateAction.hpp index 00c78dcd8c..38a25c8ca8 100644 --- a/src/openrct2/actions/RideCreateAction.hpp +++ b/src/openrct2/actions/RideCreateAction.hpp @@ -32,18 +32,11 @@ extern "C" } struct RideCreateGameActionResult : public GameActionResult { - RideCreateGameActionResult() {} + RideCreateGameActionResult() : GameActionResult(GA_ERROR::OK, 0) {} RideCreateGameActionResult(GA_ERROR error, rct_string_id message) : GameActionResult(error, message) {} - sint32& RideIndex() - { - return Results[0]._sint32; - } - - uint32& RideColor() - { - return Results[1]._uint32; - } + sint32 rideIndex; + uint32 rideColor; }; struct RideCreateAction : public GameAction @@ -59,19 +52,19 @@ public: stream << rideType << rideSubType; } - GameActionResult Query() const override + GameActionResult::Ptr Query() const override { if (rideType >= RIDE_TYPE_COUNT) { - return RideCreateGameActionResult(GA_ERROR::INVALID_PARAMETERS, STR_INVALID_RIDE_TYPE); + return std::make_unique(GA_ERROR::INVALID_PARAMETERS, STR_INVALID_RIDE_TYPE); } - return GameActionResult(); + return std::make_unique(); } - GameActionResult Execute() const override + GameActionResult::Ptr Execute() const override { - RideCreateGameActionResult res; + auto res = std::make_unique(); rct_ride *ride; rct_ride_entry *rideEntry; @@ -99,38 +92,45 @@ public: { subType = availableRideEntries[0]; if (subType == RIDE_ENTRY_INDEX_NULL) { - return RideCreateGameActionResult(GA_ERROR::INVALID_PARAMETERS, STR_INVALID_RIDE_TYPE); + res->Error = GA_ERROR::INVALID_PARAMETERS; + res->ErrorMessage = STR_INVALID_RIDE_TYPE; + return res; } } } - rideEntryIndex = subType; - rideIndex = ride_get_empty_slot(); - if (subType >= 128) { - return RideCreateGameActionResult(GA_ERROR::INVALID_PARAMETERS, STR_INVALID_RIDE_TYPE); + res->Error = GA_ERROR::INVALID_PARAMETERS; + res->ErrorMessage = STR_INVALID_RIDE_TYPE; + return res; } + rideEntryIndex = subType; + rideIndex = ride_get_empty_slot(); if (rideIndex == -1) { - return RideCreateGameActionResult(GA_ERROR::DISALLOWED, STR_TOO_MANY_RIDES); + res->Error = GA_ERROR::DISALLOWED; + res->ErrorMessage = STR_TOO_MANY_RIDES; + return res; } - res.RideIndex() = rideIndex; - res.RideColor() = ride_get_random_colour_preset_index(rideType) | (ride_get_unused_preset_vehicle_colour(rideType, subType) << 8); + res->rideIndex = rideIndex; + res->rideColor = ride_get_random_colour_preset_index(rideType) | (ride_get_unused_preset_vehicle_colour(rideType, subType) << 8); ride = get_ride(rideIndex); rideEntry = get_ride_entry(rideEntryIndex); if (rideEntry == (rct_ride_entry *)-1) { log_warning("Invalid request for ride %u", rideIndex); - return RideCreateGameActionResult(GA_ERROR::UNKNOWN, STR_UNKNOWN_OBJECT_TYPE); + res->Error = GA_ERROR::UNKNOWN; + res->ErrorMessage = STR_UNKNOWN_OBJECT_TYPE; + return res; } ride->type = rideType; ride->subtype = rideEntryIndex; - ride_set_colour_preset(ride, res.RideColor() & 0xFF); + ride_set_colour_preset(ride, res->rideColor & 0xFF); ride->overall_view.xy = RCT_XY8_UNDEFINED; // Ride name @@ -279,11 +279,11 @@ public: ride->num_circuits = 1; ride->mode = ride_get_default_mode(ride); ride->min_max_cars_per_train = (rideEntry->min_cars_in_train << 4) | rideEntry->max_cars_in_train; - ride_set_vehicle_colours_to_random_preset(ride, 0xFF & (res.RideColor() >> 8)); + ride_set_vehicle_colours_to_random_preset(ride, 0xFF & (res->rideColor >> 8)); window_invalidate_by_class(WC_RIDE_LIST); - res.ExpenditureType = RCT_EXPENDITURE_TYPE_RIDE_CONSTRUCTION; - res.Position.x = (uint16)0x8000; + res->ExpenditureType = RCT_EXPENDITURE_TYPE_RIDE_CONSTRUCTION; + res->Position.x = (uint16)0x8000; return res; } diff --git a/src/openrct2/actions/RideSetStatus.hpp b/src/openrct2/actions/RideSetStatus.hpp index b4f053d958..69a4d9f43c 100644 --- a/src/openrct2/actions/RideSetStatus.hpp +++ b/src/openrct2/actions/RideSetStatus.hpp @@ -42,34 +42,34 @@ public: stream << Status; } - GameActionResult Query() const override + GameActionResult::Ptr Query() const override { if (RideIndex >= MAX_RIDES) { log_warning("Invalid game command for ride %u", RideIndex); - return GameActionResult(GA_ERROR::INVALID_PARAMETERS, STR_INVALID_SELECTION_OF_OBJECTS); + return std::make_unique(GA_ERROR::INVALID_PARAMETERS, STR_INVALID_SELECTION_OF_OBJECTS); } - return GameActionResult(); + return std::make_unique(); } - GameActionResult Execute() const override + GameActionResult::Ptr Execute() const override { - GameActionResult res; - res.ExpenditureType = RCT_EXPENDITURE_TYPE_RIDE_RUNNING_COSTS; + GameActionResult::Ptr res = std::make_unique(); + res->ExpenditureType = RCT_EXPENDITURE_TYPE_RIDE_RUNNING_COSTS; rct_ride *ride = get_ride(RideIndex); if (ride->type == RIDE_TYPE_NULL) { log_warning("Invalid game command for ride %u", RideIndex); - return GameActionResult(GA_ERROR::INVALID_PARAMETERS, STR_INVALID_SELECTION_OF_OBJECTS); + return std::make_unique(GA_ERROR::INVALID_PARAMETERS, STR_INVALID_SELECTION_OF_OBJECTS); } if (ride->overall_view.xy != RCT_XY8_UNDEFINED) { - res.Position.x = ride->overall_view.x * 32 + 16; - res.Position.y = ride->overall_view.y * 32 + 16; - res.Position.z = map_element_height(res.Position.x, res.Position.y); + res->Position.x = ride->overall_view.x * 32 + 16; + res->Position.y = ride->overall_view.y * 32 + 16; + res->Position.z = map_element_height(res->Position.x, res->Position.y); } switch (Status) @@ -126,6 +126,6 @@ public: Guard::Assert(false, "Invalid status passed: %u", Status); } - return GameActionResult(); + return res; } }; diff --git a/src/openrct2/actions/SetParkEntranceFeeAction.hpp b/src/openrct2/actions/SetParkEntranceFeeAction.hpp index 9614dae8da..7784fa5548 100644 --- a/src/openrct2/actions/SetParkEntranceFeeAction.hpp +++ b/src/openrct2/actions/SetParkEntranceFeeAction.hpp @@ -39,26 +39,26 @@ public: stream << Fee; } - GameActionResult Query() const override + GameActionResult::Ptr Query() const override { bool noMoney = (gParkFlags & PARK_FLAGS_NO_MONEY) != 0; bool forceFreeEntry = (gParkFlags & PARK_FLAGS_PARK_FREE_ENTRY) && !gCheatsUnlockAllPrices; if (noMoney || forceFreeEntry) { - return GameActionResult(GA_ERROR::DISALLOWED, STR_NONE); + return std::make_unique(GA_ERROR::DISALLOWED, STR_NONE); } if (Fee < MONEY_FREE || Fee > MONEY(100,00)) { - return GameActionResult(GA_ERROR::INVALID_PARAMETERS, STR_NONE); + return std::make_unique(GA_ERROR::INVALID_PARAMETERS, STR_NONE); } - return GameActionResult(); + return std::make_unique(); } - GameActionResult Execute() const override + GameActionResult::Ptr Execute() const override { gParkEntranceFee = Fee; window_invalidate_by_class(WC_PARK_INFORMATION); - return GameActionResult(); + return std::make_unique(); } }; diff --git a/src/openrct2/network/Network.cpp b/src/openrct2/network/Network.cpp index da15fe6949..6813d36c3f 100644 --- a/src/openrct2/network/Network.cpp +++ b/src/openrct2/network/Network.cpp @@ -1415,20 +1415,20 @@ void Network::ProcessGameCommandQueue() if (gc.action != nullptr) { - IGameAction *action = gc.action; + IGameAction *action = gc.action.get(); action->SetFlags(action->GetFlags() | GAME_COMMAND_FLAG_NETWORKED); Guard::Assert(action != nullptr); - GameActionResult result = GameActions::Execute(action); - if (result.Error == GA_ERROR::OK) + GameActionResult::Ptr result = GameActions::Execute(action); + if (result->Error == GA_ERROR::OK) { game_commands_processed_this_tick++; NetworkPlayer* player = GetPlayerByID(gc.playerid); if (player) { player->LastAction = NetworkActions::FindCommand(action->GetType()); player->LastActionTime = platform_get_ticks(); - player->AddMoneySpent(result.Cost); + player->AddMoneySpent(result->Cost); } Server_Send_GAME_ACTION(action); @@ -1482,16 +1482,14 @@ void Network::EnqueueGameAction(const IGameAction *action) DataSerialiser dsOut(true, stream); action->Serialise(dsOut); - IGameAction *ga = GameActions::Create(action->GetType()); + std::unique_ptr ga = GameActions::Create(action->GetType()); ga->SetCallback(action->GetCallback()); stream.SetPosition(0); DataSerialiser dsIn(false, stream); ga->Serialise(dsIn); - GameCommand gc(gCurrentTicks, ga); - gc.commandIndex = _commandId++; - game_command_queue.insert(gc); + game_command_queue.emplace(gCurrentTicks, std::move(ga), _commandId++); } void Network::AddClient(ITcpSocket * socket) @@ -2112,9 +2110,7 @@ void Network::Client_Handle_GAMECMD(NetworkConnection& connection, NetworkPacket uint8 callback; packet >> tick >> args[0] >> args[1] >> args[2] >> args[3] >> args[4] >> args[5] >> args[6] >> playerid >> callback; - GameCommand gc(tick, args, playerid, callback); - gc.commandIndex = _commandId++; - game_command_queue.insert(gc); + game_command_queue.emplace(tick, args, playerid, callback, _commandId++); } void Network::Client_Handle_GAME_ACTION(NetworkConnection& connection, NetworkPacket& packet) @@ -2130,7 +2126,7 @@ void Network::Client_Handle_GAME_ACTION(NetworkConnection& connection, NetworkPa DataSerialiser ds(false, stream); - IGameAction *action = GameActions::Create(type); + IGameAction::Ptr action = GameActions::Create(type); if (!action) { // TODO: Handle error. @@ -2145,9 +2141,7 @@ void Network::Client_Handle_GAME_ACTION(NetworkConnection& connection, NetworkPa _gameActionCallbacks.erase(itr); } - GameCommand gc(tick, action); - gc.commandIndex = _commandId++; - game_command_queue.insert(gc); + game_command_queue.emplace(tick, std::move(action), _commandId++); } void Network::Server_Handle_GAME_ACTION(NetworkConnection& connection, NetworkPacket& packet) @@ -2205,7 +2199,7 @@ void Network::Server_Handle_GAME_ACTION(NetworkConnection& connection, NetworkPa } // Run game command, and if it is successful send to clients - IGameAction *ga = GameActions::Create(type); + IGameAction::Ptr ga = GameActions::Create(type); if (!ga) { // TODO: Handle error. @@ -2220,9 +2214,7 @@ void Network::Server_Handle_GAME_ACTION(NetworkConnection& connection, NetworkPa // Set player to sender, should be 0 if sent from client. ga->SetPlayer(connection.Player->Id); - GameCommand gc(tick, ga); - gc.commandIndex = _commandId++; - game_command_queue.insert(gc); + game_command_queue.emplace(tick, std::move(ga), _commandId++); } void Network::Server_Handle_GAMECMD(NetworkConnection& connection, NetworkPacket& packet) @@ -2285,9 +2277,7 @@ void Network::Server_Handle_GAMECMD(NetworkConnection& connection, NetworkPacket return; } - GameCommand gc = GameCommand(tick, args, playerid, callback); - gc.commandIndex = _commandId++; - game_command_queue.insert(gc); + game_command_queue.emplace(tick, args, playerid, callback, _commandId++); } void Network::Client_Handle_TICK(NetworkConnection& connection, NetworkPacket& packet) diff --git a/src/openrct2/network/network.h b/src/openrct2/network/network.h index 0a5e8a5614..bb185df37d 100644 --- a/src/openrct2/network/network.h +++ b/src/openrct2/network/network.h @@ -196,33 +196,18 @@ private: struct GameCommand { - GameCommand(uint32 t, uint32* args, uint8 p, uint8 cb) { + GameCommand(uint32 t, uint32* args, uint8 p, uint8 cb, uint32 id) { tick = t; eax = args[0]; ebx = args[1]; ecx = args[2]; edx = args[3]; esi = args[4]; edi = args[5]; ebp = args[6]; playerid = p; callback = cb; action = nullptr; + commandIndex = id; } - GameCommand(uint32 t, IGameAction *ga) + GameCommand(uint32 t, std::unique_ptr&& ga, uint32 id) { tick = t; - action = ga; - } - - GameCommand(const GameCommand &source) { - tick = source.tick; - playerid = source.playerid; - action = source.action; - callback = source.callback; - if (action == nullptr) - { - eax = source.eax; - ebx = source.ebx; - ecx = source.ecx; - edx = source.edx; - esi = source.esi; - edi = source.edi; - ebp = source.ebp; - } + action = std::move(ga); + commandIndex = id; } ~GameCommand() @@ -231,7 +216,7 @@ private: uint32 tick; uint32 eax, ebx, ecx, edx, esi, edi, ebp; - IGameAction *action; + IGameAction::Ptr action; uint8 playerid; uint8 callback; uint32 commandIndex;