Merge pull request #8692 from ZehMatt/gameaction-things

Handle unregistered game actions with error logging.
This commit is contained in:
ζeh Matt 2019-02-07 23:39:11 +01:00 committed by GitHub
commit 436f15f22e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 20 additions and 14 deletions

View File

@ -90,7 +90,9 @@ namespace GameActions
result = factory(); result = factory();
} }
} }
#ifdef _DEBUG
Guard::ArgumentNotNull(result, "Attempting to create unregistered gameaction: %u", id); Guard::ArgumentNotNull(result, "Attempting to create unregistered gameaction: %u", id);
#endif
return std::unique_ptr<GameAction>(result); return std::unique_ptr<GameAction>(result);
} }

View File

@ -2697,8 +2697,8 @@ void Network::Client_Handle_GAMECMD([[maybe_unused]] NetworkConnection& connecti
void Network::Client_Handle_GAME_ACTION([[maybe_unused]] NetworkConnection& connection, NetworkPacket& packet) void Network::Client_Handle_GAME_ACTION([[maybe_unused]] NetworkConnection& connection, NetworkPacket& packet)
{ {
uint32_t tick; uint32_t tick;
uint32_t type; uint32_t actionType;
packet >> tick >> type; packet >> tick >> actionType;
MemoryStream stream; MemoryStream stream;
size_t size = packet.Size - packet.BytesRead; size_t size = packet.Size - packet.BytesRead;
@ -2707,10 +2707,11 @@ void Network::Client_Handle_GAME_ACTION([[maybe_unused]] NetworkConnection& conn
DataSerialiser ds(false, stream); DataSerialiser ds(false, stream);
GameAction::Ptr action = GameActions::Create(type); GameAction::Ptr action = GameActions::Create(actionType);
if (!action) if (action == nullptr)
{ {
// TODO: Handle error. log_error("Received unregistered game action type: 0x%08X", actionType);
return;
} }
action->Serialise(ds); action->Serialise(ds);
@ -2733,20 +2734,20 @@ void Network::Client_Handle_GAME_ACTION([[maybe_unused]] NetworkConnection& conn
void Network::Server_Handle_GAME_ACTION(NetworkConnection& connection, NetworkPacket& packet) void Network::Server_Handle_GAME_ACTION(NetworkConnection& connection, NetworkPacket& packet)
{ {
uint32_t tick; uint32_t tick;
uint32_t type; uint32_t actionType;
if (!connection.Player) if (!connection.Player)
{ {
return; return;
} }
packet >> tick >> type; packet >> tick >> actionType;
// tick count is different by time last_action_time is set, keep same value // tick count is different by time last_action_time is set, keep same value
// Check if player's group permission allows command to run // Check if player's group permission allows command to run
uint32_t ticks = platform_get_ticks(); uint32_t ticks = platform_get_ticks();
NetworkGroup* group = GetGroupByID(connection.Player->Group); NetworkGroup* group = GetGroupByID(connection.Player->Group);
if (!group || !group->CanPerformCommand(type)) if (!group || !group->CanPerformCommand(actionType))
{ {
Server_Send_SHOWERROR(connection, STR_CANT_DO_THIS, STR_PERMISSION_DENIED); Server_Send_SHOWERROR(connection, STR_CANT_DO_THIS, STR_PERMISSION_DENIED);
return; return;
@ -2755,7 +2756,7 @@ void Network::Server_Handle_GAME_ACTION(NetworkConnection& connection, NetworkPa
// In case someone modifies the code / memory to enable cluster build, // In case someone modifies the code / memory to enable cluster build,
// require a small delay in between placing scenery to provide some security, as // require a small delay in between placing scenery to provide some security, as
// cluster mode is a for loop that runs the place_scenery code multiple times. // cluster mode is a for loop that runs the place_scenery code multiple times.
if (type == GAME_COMMAND_PLACE_SCENERY) if (actionType == GAME_COMMAND_PLACE_SCENERY)
{ {
if (ticks - connection.Player->LastPlaceSceneryTime < ACTION_COOLDOWN_TIME_PLACE_SCENERY && if (ticks - connection.Player->LastPlaceSceneryTime < ACTION_COOLDOWN_TIME_PLACE_SCENERY &&
// Incase platform_get_ticks() wraps after ~49 days, ignore larger logged times. // Incase platform_get_ticks() wraps after ~49 days, ignore larger logged times.
@ -2770,7 +2771,7 @@ void Network::Server_Handle_GAME_ACTION(NetworkConnection& connection, NetworkPa
} }
// This is to prevent abuse of demolishing rides. Anyone that is not the server // This is to prevent abuse of demolishing rides. Anyone that is not the server
// host will have to wait a small amount of time in between deleting rides. // host will have to wait a small amount of time in between deleting rides.
else if (type == GAME_COMMAND_DEMOLISH_RIDE) else if (actionType == GAME_COMMAND_DEMOLISH_RIDE)
{ {
if (ticks - connection.Player->LastDemolishRideTime < ACTION_COOLDOWN_TIME_DEMOLISH_RIDE && if (ticks - connection.Player->LastDemolishRideTime < ACTION_COOLDOWN_TIME_DEMOLISH_RIDE &&
// Incase platform_get_ticks()() wraps after ~49 days, ignore larger logged times. // Incase platform_get_ticks()() wraps after ~49 days, ignore larger logged times.
@ -2781,16 +2782,19 @@ void Network::Server_Handle_GAME_ACTION(NetworkConnection& connection, NetworkPa
} }
} }
// Don't let clients send pause or quit // Don't let clients send pause or quit
else if (type == GAME_COMMAND_TOGGLE_PAUSE || type == GAME_COMMAND_LOAD_OR_QUIT) else if (actionType == GAME_COMMAND_TOGGLE_PAUSE || actionType == GAME_COMMAND_LOAD_OR_QUIT)
{ {
return; return;
} }
// Run game command, and if it is successful send to clients // Run game command, and if it is successful send to clients
GameAction::Ptr ga = GameActions::Create(type); GameAction::Ptr ga = GameActions::Create(actionType);
if (!ga) if (ga == nullptr)
{ {
// TODO: Handle error. log_error(
"Received unregistered game action type: 0x%08X from player: (%d) %s", actionType, connection.Player->Id,
connection.Player->Name.c_str());
return;
} }
DataSerialiser stream(false); DataSerialiser stream(false);