From caa490642cc62cc6c7192ba05290742b76f4fe56 Mon Sep 17 00:00:00 2001 From: Matt Date: Fri, 15 Feb 2019 08:43:05 +0100 Subject: [PATCH 1/6] Refactor GameActions to allow non-top level actions. --- src/openrct2/actions/GameAction.cpp | 56 +++++++++++++++++------------ src/openrct2/actions/GameAction.h | 4 +-- 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/openrct2/actions/GameAction.cpp b/src/openrct2/actions/GameAction.cpp index efcdfd207b..b6ec252a75 100644 --- a/src/openrct2/actions/GameAction.cpp +++ b/src/openrct2/actions/GameAction.cpp @@ -137,7 +137,7 @@ namespace GameActions return false; } - GameActionResult::Ptr Query(const GameAction* action) + GameActionResult::Ptr Query(const GameAction* action, bool topLevel /* = true */) { Guard::ArgumentNotNull(action); @@ -155,9 +155,13 @@ namespace GameActions auto result = action->Query(); - gCommandPosition.x = result->Position.x; - gCommandPosition.y = result->Position.y; - gCommandPosition.z = result->Position.z; + // Only top level actions affect the command position. + if (topLevel == true) + { + gCommandPosition.x = result->Position.x; + gCommandPosition.y = result->Position.y; + gCommandPosition.z = result->Position.z; + } if (result->Error == GA_ERROR::OK) { @@ -223,7 +227,7 @@ namespace GameActions network_append_server_log(text); } - GameActionResult::Ptr Execute(const GameAction* action) + GameActionResult::Ptr Execute(const GameAction* action, bool topLevel /* = true */) { Guard::ArgumentNotNull(action); @@ -247,31 +251,34 @@ namespace GameActions } } - GameActionResult::Ptr result = Query(action); + GameActionResult::Ptr result = Query(action, topLevel); if (result->Error == GA_ERROR::OK) { - // Networked games send actions to the server to be run - if (network_get_mode() == NETWORK_MODE_CLIENT) + if (topLevel) { - // As a client we have to wait or send it first. - if (!(actionFlags & GA_FLAGS::CLIENT_ONLY) && !(flags & GAME_COMMAND_FLAG_NETWORKED)) + // Networked games send actions to the server to be run + if (network_get_mode() == NETWORK_MODE_CLIENT) { - log_verbose("[%s] GameAction::Execute %s (Out)", GetRealm(), action->GetName()); - network_send_game_action(action); + // As a client we have to wait or send it first. + if (!(actionFlags & GA_FLAGS::CLIENT_ONLY) && !(flags & GAME_COMMAND_FLAG_NETWORKED)) + { + log_verbose("[%s] GameAction::Execute %s (Out)", GetRealm(), action->GetName()); + network_send_game_action(action); - return result; + return result; + } } - } - else if (network_get_mode() == NETWORK_MODE_SERVER) - { - // If player is the server it would execute right away as where clients execute the commands - // at the beginning of the frame, so we have to put them into the queue. - if (!(actionFlags & GA_FLAGS::CLIENT_ONLY) && !(flags & GAME_COMMAND_FLAG_NETWORKED)) + else if (network_get_mode() == NETWORK_MODE_SERVER) { - log_verbose("[%s] GameAction::Execute %s (Queue)", GetRealm(), action->GetName()); - network_enqueue_game_action(action); + // If player is the server it would execute right away as where clients execute the commands + // at the beginning of the frame, so we have to put them into the queue. + if (!(actionFlags & GA_FLAGS::CLIENT_ONLY) && !(flags & GAME_COMMAND_FLAG_NETWORKED)) + { + log_verbose("[%s] GameAction::Execute %s (Queue)", GetRealm(), action->GetName()); + network_enqueue_game_action(action); - return result; + return result; + } } } @@ -283,6 +290,10 @@ namespace GameActions LogActionFinish(logContext, action, result); + // If not top level just give away the result. + if (topLevel == false) + return result; + gCommandPosition.x = result->Position.x; gCommandPosition.y = result->Position.y; gCommandPosition.z = result->Position.z; @@ -349,6 +360,7 @@ namespace GameActions std::copy(result->ErrorMessageArgs.begin(), result->ErrorMessageArgs.end(), gCommonFormatArgs); context_show_error(result->ErrorTitle, result->ErrorMessage); } + return result; } diff --git a/src/openrct2/actions/GameAction.h b/src/openrct2/actions/GameAction.h index daeb95cbb5..ec2b166e6f 100644 --- a/src/openrct2/actions/GameAction.h +++ b/src/openrct2/actions/GameAction.h @@ -244,8 +244,8 @@ namespace GameActions bool IsValidId(uint32_t id); GameAction::Ptr Create(uint32_t id); GameAction::Ptr Clone(const GameAction* action); - GameActionResult::Ptr Query(const GameAction* action); - GameActionResult::Ptr Execute(const GameAction* action); + GameActionResult::Ptr Query(const GameAction* action, bool topLevel = true); + GameActionResult::Ptr Execute(const GameAction* action, bool topLevel = true); GameActionFactory Register(uint32_t id, GameActionFactory action); template static GameActionFactory Register() From 46c9bcc4bcf55d1e97e0d66f8548f742907342d2 Mon Sep 17 00:00:00 2001 From: Matt Date: Fri, 15 Feb 2019 08:47:07 +0100 Subject: [PATCH 2/6] Remove direct calls to Execute Query on action. --- src/openrct2/actions/ClearAction.hpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/openrct2/actions/ClearAction.hpp b/src/openrct2/actions/ClearAction.hpp index ba5e28ddee..331e268f43 100644 --- a/src/openrct2/actions/ClearAction.hpp +++ b/src/openrct2/actions/ClearAction.hpp @@ -155,7 +155,9 @@ private: auto footpathRemoveAction = FootpathRemoveAction(x * 32, y * 32, tileElement->base_height); footpathRemoveAction.SetFlags(GetFlags()); - auto res = executing ? footpathRemoveAction.Execute() : footpathRemoveAction.Query(); + auto res = executing ? GameActions::Execute(&footpathRemoveAction, false) + : GameActions::Query(&footpathRemoveAction, false); + if (res->Error != GA_ERROR::OK) return MONEY32_UNDEFINED; @@ -171,7 +173,9 @@ private: tileElement->AsSmallScenery()->GetEntryIndex()); removeSceneryAction.SetFlags(GetFlags()); - auto res = executing ? removeSceneryAction.Execute() : removeSceneryAction.Query(); + auto res = executing ? GameActions::Execute(&removeSceneryAction, false) + : GameActions::Query(&removeSceneryAction, false); + if (res->Error != GA_ERROR::OK) return MONEY32_UNDEFINED; @@ -186,7 +190,9 @@ private: auto wallRemoveAction = WallRemoveAction(wallLocation); wallRemoveAction.SetFlags(GetFlags()); - auto res = executing ? wallRemoveAction.Execute() : wallRemoveAction.Query(); + auto res = executing ? GameActions::Execute(&wallRemoveAction, false) + : GameActions::Query(&wallRemoveAction, false); + if (res->Error != GA_ERROR::OK) return MONEY32_UNDEFINED; @@ -202,7 +208,9 @@ private: tileElement->AsLargeScenery()->GetSequenceIndex()); removeSceneryAction.SetFlags(GetFlags() | GAME_COMMAND_FLAG_PATH_SCENERY); - auto res = executing ? removeSceneryAction.Execute() : removeSceneryAction.Query(); + auto res = executing ? GameActions::Execute(&removeSceneryAction, false) + : GameActions::Query(&removeSceneryAction, false); + if (res->Error != GA_ERROR::OK) return MONEY32_UNDEFINED; From 22811e0e857fd8bda5b7152d4683f7f56208875b Mon Sep 17 00:00:00 2001 From: Matt Date: Fri, 15 Feb 2019 08:50:48 +0100 Subject: [PATCH 3/6] Remove direct calls to Execute Query on action. --- src/openrct2/actions/RideDemolishAction.hpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/openrct2/actions/RideDemolishAction.hpp b/src/openrct2/actions/RideDemolishAction.hpp index acdbb8f986..8e7ba79e45 100644 --- a/src/openrct2/actions/RideDemolishAction.hpp +++ b/src/openrct2/actions/RideDemolishAction.hpp @@ -264,14 +264,10 @@ private: auto setMazeTrack = MazeSetTrackAction(x, y, z, false, direction, _rideIndex, GC_SET_MAZE_TRACK_FILL); setMazeTrack.SetFlags(GetFlags()); - auto queryRes = setMazeTrack.Query(); - if (queryRes->Error == GA_ERROR::OK) + auto execRes = GameActions::Execute(&setMazeTrack, false); + if (execRes->Error == GA_ERROR::OK) { - auto execRes = setMazeTrack.Execute(); - if (execRes->Error == GA_ERROR::OK) - { - return execRes->Cost; - } + return execRes->Cost; } return MONEY32_UNDEFINED; From 60ec1da0f84dbc51e5b85edcb08af8ccf6fb8808 Mon Sep 17 00:00:00 2001 From: Matt Date: Fri, 15 Feb 2019 08:55:01 +0100 Subject: [PATCH 4/6] Remove direct calls to Execute Query on action. --- src/openrct2/world/Map.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/openrct2/world/Map.cpp b/src/openrct2/world/Map.cpp index 5db7eb3d48..46bf38e5aa 100644 --- a/src/openrct2/world/Map.cpp +++ b/src/openrct2/world/Map.cpp @@ -1328,8 +1328,10 @@ static money32 lower_land( newSlope &= TILE_ELEMENT_SURFACE_SLOPE_MASK; auto landSetHeightAction = LandSetHeightAction({ x_coord, y_coord }, height, newSlope); - auto res = (flags & GAME_COMMAND_FLAG_APPLY) ? landSetHeightAction.Execute() : landSetHeightAction.Query(); + landSetHeightAction.SetFlags(flags); + auto res = (flags & GAME_COMMAND_FLAG_APPLY) ? GameActions::Execute(&landSetHeightAction, false) + : GameActions::Query(&landSetHeightAction, false); if (res->Error != GA_ERROR::OK) { return MONEY32_UNDEFINED; @@ -1580,7 +1582,9 @@ static money32 smooth_land_tile( } auto landSetHeightAction = LandSetHeightAction({ x, y }, targetBaseZ, slope); - auto res = (flags & GAME_COMMAND_FLAG_APPLY) ? landSetHeightAction.Execute() : landSetHeightAction.Query(); + landSetHeightAction.SetFlags(flags); + auto res = (flags & GAME_COMMAND_FLAG_APPLY) ? GameActions::Execute(&landSetHeightAction, false) + : GameActions::Query(&landSetHeightAction, false); if (res->Error == GA_ERROR::OK) { @@ -1725,8 +1729,9 @@ static money32 smooth_land_row_by_edge( } } auto landSetHeightAction = LandSetHeightAction({ x, y }, targetBaseZ, slope); - auto res = (flags & GAME_COMMAND_FLAG_APPLY) ? landSetHeightAction.Execute() : landSetHeightAction.Query(); - + landSetHeightAction.SetFlags(flags); + auto res = (flags & GAME_COMMAND_FLAG_APPLY) ? GameActions::Execute(&landSetHeightAction, false) + : GameActions::Query(&landSetHeightAction, false); if (res->Error == GA_ERROR::OK) { totalCost += res->Cost; From 16e371c79269a38629b6b1e7c0ccb75940a4512c Mon Sep 17 00:00:00 2001 From: Matt Date: Fri, 15 Feb 2019 09:01:38 +0100 Subject: [PATCH 5/6] Bump up network version. --- src/openrct2/network/Network.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openrct2/network/Network.cpp b/src/openrct2/network/Network.cpp index 77b9f4faf4..89690a141b 100644 --- a/src/openrct2/network/Network.cpp +++ b/src/openrct2/network/Network.cpp @@ -30,7 +30,7 @@ // This string specifies which version of network stream current build uses. // It is used for making sure only compatible builds get connected, even within // single OpenRCT2 version. -#define NETWORK_STREAM_VERSION "36" +#define NETWORK_STREAM_VERSION "37" #define NETWORK_STREAM_ID OPENRCT2_VERSION "-" NETWORK_STREAM_VERSION static rct_peep* _pickup_peep = nullptr; From 00be865ff1ed2ed08413143b5456e833ce7c2561 Mon Sep 17 00:00:00 2001 From: Matt Date: Fri, 15 Feb 2019 16:32:11 +0100 Subject: [PATCH 6/6] Use ExecuteNested/QueryNested instead of passing a parameter. --- src/openrct2/actions/ClearAction.hpp | 16 ++++++------ src/openrct2/actions/GameAction.cpp | 28 ++++++++++++++++++--- src/openrct2/actions/GameAction.h | 11 ++++++-- src/openrct2/actions/RideDemolishAction.hpp | 2 +- src/openrct2/world/Map.cpp | 12 ++++----- 5 files changed, 48 insertions(+), 21 deletions(-) diff --git a/src/openrct2/actions/ClearAction.hpp b/src/openrct2/actions/ClearAction.hpp index 331e268f43..2f30df8a9f 100644 --- a/src/openrct2/actions/ClearAction.hpp +++ b/src/openrct2/actions/ClearAction.hpp @@ -155,8 +155,8 @@ private: auto footpathRemoveAction = FootpathRemoveAction(x * 32, y * 32, tileElement->base_height); footpathRemoveAction.SetFlags(GetFlags()); - auto res = executing ? GameActions::Execute(&footpathRemoveAction, false) - : GameActions::Query(&footpathRemoveAction, false); + auto res = executing ? GameActions::ExecuteNested(&footpathRemoveAction) + : GameActions::QueryNested(&footpathRemoveAction); if (res->Error != GA_ERROR::OK) return MONEY32_UNDEFINED; @@ -173,8 +173,8 @@ private: tileElement->AsSmallScenery()->GetEntryIndex()); removeSceneryAction.SetFlags(GetFlags()); - auto res = executing ? GameActions::Execute(&removeSceneryAction, false) - : GameActions::Query(&removeSceneryAction, false); + auto res = executing ? GameActions::ExecuteNested(&removeSceneryAction) + : GameActions::QueryNested(&removeSceneryAction); if (res->Error != GA_ERROR::OK) return MONEY32_UNDEFINED; @@ -190,8 +190,8 @@ private: auto wallRemoveAction = WallRemoveAction(wallLocation); wallRemoveAction.SetFlags(GetFlags()); - auto res = executing ? GameActions::Execute(&wallRemoveAction, false) - : GameActions::Query(&wallRemoveAction, false); + auto res = executing ? GameActions::ExecuteNested(&wallRemoveAction) + : GameActions::QueryNested(&wallRemoveAction); if (res->Error != GA_ERROR::OK) return MONEY32_UNDEFINED; @@ -208,8 +208,8 @@ private: tileElement->AsLargeScenery()->GetSequenceIndex()); removeSceneryAction.SetFlags(GetFlags() | GAME_COMMAND_FLAG_PATH_SCENERY); - auto res = executing ? GameActions::Execute(&removeSceneryAction, false) - : GameActions::Query(&removeSceneryAction, false); + auto res = executing ? GameActions::ExecuteNested(&removeSceneryAction) + : GameActions::QueryNested(&removeSceneryAction); if (res->Error != GA_ERROR::OK) return MONEY32_UNDEFINED; diff --git a/src/openrct2/actions/GameAction.cpp b/src/openrct2/actions/GameAction.cpp index b6ec252a75..b564891e0e 100644 --- a/src/openrct2/actions/GameAction.cpp +++ b/src/openrct2/actions/GameAction.cpp @@ -137,7 +137,7 @@ namespace GameActions return false; } - GameActionResult::Ptr Query(const GameAction* action, bool topLevel /* = true */) + static GameActionResult::Ptr QueryInternal(const GameAction* action, bool topLevel) { Guard::ArgumentNotNull(action); @@ -156,7 +156,7 @@ namespace GameActions auto result = action->Query(); // Only top level actions affect the command position. - if (topLevel == true) + if (topLevel) { gCommandPosition.x = result->Position.x; gCommandPosition.y = result->Position.y; @@ -175,6 +175,16 @@ namespace GameActions return result; } + GameActionResult::Ptr Query(const GameAction* action) + { + return QueryInternal(action, true); + } + + GameActionResult::Ptr QueryNested(const GameAction* action) + { + return QueryInternal(action, false); + } + static const char* GetRealm() { if (network_get_mode() == NETWORK_MODE_CLIENT) @@ -227,7 +237,7 @@ namespace GameActions network_append_server_log(text); } - GameActionResult::Ptr Execute(const GameAction* action, bool topLevel /* = true */) + static GameActionResult::Ptr ExecuteInternal(const GameAction* action, bool topLevel) { Guard::ArgumentNotNull(action); @@ -251,7 +261,7 @@ namespace GameActions } } - GameActionResult::Ptr result = Query(action, topLevel); + GameActionResult::Ptr result = Query(action); if (result->Error == GA_ERROR::OK) { if (topLevel) @@ -364,4 +374,14 @@ namespace GameActions return result; } + GameActionResult::Ptr Execute(const GameAction* action) + { + return ExecuteInternal(action, true); + } + + GameActionResult::Ptr ExecuteNested(const GameAction* action) + { + return ExecuteInternal(action, false); + } + } // namespace GameActions diff --git a/src/openrct2/actions/GameAction.h b/src/openrct2/actions/GameAction.h index ec2b166e6f..aebc20f36a 100644 --- a/src/openrct2/actions/GameAction.h +++ b/src/openrct2/actions/GameAction.h @@ -244,8 +244,15 @@ namespace GameActions bool IsValidId(uint32_t id); GameAction::Ptr Create(uint32_t id); GameAction::Ptr Clone(const GameAction* action); - GameActionResult::Ptr Query(const GameAction* action, bool topLevel = true); - GameActionResult::Ptr Execute(const GameAction* action, bool topLevel = true); + + // This should be used if a round trip is to be expected. + GameActionResult::Ptr Query(const GameAction* action); + GameActionResult::Ptr Execute(const GameAction* action); + + // This should be used from within game actions. + GameActionResult::Ptr QueryNested(const GameAction* action); + GameActionResult::Ptr ExecuteNested(const GameAction* action); + GameActionFactory Register(uint32_t id, GameActionFactory action); template static GameActionFactory Register() diff --git a/src/openrct2/actions/RideDemolishAction.hpp b/src/openrct2/actions/RideDemolishAction.hpp index 8e7ba79e45..feb63ec330 100644 --- a/src/openrct2/actions/RideDemolishAction.hpp +++ b/src/openrct2/actions/RideDemolishAction.hpp @@ -264,7 +264,7 @@ private: auto setMazeTrack = MazeSetTrackAction(x, y, z, false, direction, _rideIndex, GC_SET_MAZE_TRACK_FILL); setMazeTrack.SetFlags(GetFlags()); - auto execRes = GameActions::Execute(&setMazeTrack, false); + auto execRes = GameActions::ExecuteNested(&setMazeTrack); if (execRes->Error == GA_ERROR::OK) { return execRes->Cost; diff --git a/src/openrct2/world/Map.cpp b/src/openrct2/world/Map.cpp index 46bf38e5aa..0c56bc9f1f 100644 --- a/src/openrct2/world/Map.cpp +++ b/src/openrct2/world/Map.cpp @@ -1330,8 +1330,8 @@ static money32 lower_land( auto landSetHeightAction = LandSetHeightAction({ x_coord, y_coord }, height, newSlope); landSetHeightAction.SetFlags(flags); - auto res = (flags & GAME_COMMAND_FLAG_APPLY) ? GameActions::Execute(&landSetHeightAction, false) - : GameActions::Query(&landSetHeightAction, false); + auto res = (flags & GAME_COMMAND_FLAG_APPLY) ? GameActions::ExecuteNested(&landSetHeightAction) + : GameActions::QueryNested(&landSetHeightAction); if (res->Error != GA_ERROR::OK) { return MONEY32_UNDEFINED; @@ -1583,8 +1583,8 @@ static money32 smooth_land_tile( auto landSetHeightAction = LandSetHeightAction({ x, y }, targetBaseZ, slope); landSetHeightAction.SetFlags(flags); - auto res = (flags & GAME_COMMAND_FLAG_APPLY) ? GameActions::Execute(&landSetHeightAction, false) - : GameActions::Query(&landSetHeightAction, false); + auto res = (flags & GAME_COMMAND_FLAG_APPLY) ? GameActions::ExecuteNested(&landSetHeightAction) + : GameActions::QueryNested(&landSetHeightAction); if (res->Error == GA_ERROR::OK) { @@ -1730,8 +1730,8 @@ static money32 smooth_land_row_by_edge( } auto landSetHeightAction = LandSetHeightAction({ x, y }, targetBaseZ, slope); landSetHeightAction.SetFlags(flags); - auto res = (flags & GAME_COMMAND_FLAG_APPLY) ? GameActions::Execute(&landSetHeightAction, false) - : GameActions::Query(&landSetHeightAction, false); + auto res = (flags & GAME_COMMAND_FLAG_APPLY) ? GameActions::ExecuteNested(&landSetHeightAction) + : GameActions::QueryNested(&landSetHeightAction); if (res->Error == GA_ERROR::OK) { totalCost += res->Cost;