From d3b29bcacb4589922a3afc85cf648609b49679dc Mon Sep 17 00:00:00 2001 From: tron Date: Sat, 24 Feb 2007 09:42:39 +0000 Subject: [PATCH] (svn r8876) -Fix Replace tests with magic numbers by a simple extraction template for command parameters --- src/cmd_helper.h | 28 ++++++++++++++++++++++++++++ src/depot.h | 4 ++-- src/rail_cmd.cpp | 10 ++++++---- src/road_cmd.cpp | 26 ++++++++++++-------------- src/station_cmd.cpp | 5 +++-- src/water_cmd.cpp | 13 +++++++------ 6 files changed, 58 insertions(+), 28 deletions(-) create mode 100644 src/cmd_helper.h diff --git a/src/cmd_helper.h b/src/cmd_helper.h new file mode 100644 index 0000000000..115b0e1cff --- /dev/null +++ b/src/cmd_helper.h @@ -0,0 +1,28 @@ +/* $Id$ */ + +#ifndef CMD_HELPER_H +#define CMD_HELPER_H + +#include "direction.h" +#include "macros.h" +#include "road.h" + + +template static inline void ExtractValid(); +template<> static inline void ExtractValid<1>() {} + + +template struct ExtractBits; +template<> struct ExtractBits { static const uint Count = 1; }; +template<> struct ExtractBits { static const uint Count = 2; }; +template<> struct ExtractBits { static const uint Count = 4; }; + + +template static inline T Extract(U v) +{ + // Check if there are enough bits in v + ExtractValid::Count <= sizeof(U) * 8>(); + return (T)GB(v, N, ExtractBits::Count); +} + +#endif diff --git a/src/depot.h b/src/depot.h index 8e60f674e8..61272670bf 100644 --- a/src/depot.h +++ b/src/depot.h @@ -85,7 +85,7 @@ static inline bool IsTileDepotType(TileIndex tile, TransportType type) /** * Find out if the slope of the tile is suitable to build a depot of given direction - * @param direction The direction in which the depot's exit points. Starts with 0 as NE and goes Clockwise + * @param direction The direction in which the depot's exit points * @param tileh The slope of the tile in question * @return true if the construction is possible @@ -98,7 +98,7 @@ static inline bool IsTileDepotType(TileIndex tile, TransportType type) * 03 (exit towards NW) we need either bit 0 or 4 set in tileh: 0x4C >> 3 = 1001

* So ((0x4C >> direction) & tileh) determines whether the depot can be built on the current tileh */ -static inline bool CanBuildDepotByTileh(uint32 direction, Slope tileh) +static inline bool CanBuildDepotByTileh(DiagDirection direction, Slope tileh) { return ((0x4C >> direction) & tileh) != 0; } diff --git a/src/rail_cmd.cpp b/src/rail_cmd.cpp index d74b30096c..cb4d08c80d 100644 --- a/src/rail_cmd.cpp +++ b/src/rail_cmd.cpp @@ -3,6 +3,7 @@ #include "stdafx.h" #include "openttd.h" #include "bridge_map.h" +#include "cmd_helper.h" #include "debug.h" #include "functions.h" #include "rail_map.h" @@ -540,7 +541,7 @@ int32 CmdRemoveRailroadTrack(TileIndex tile, uint32 flags, uint32 p1, uint32 p2) /** Build a train depot * @param tile position of the train depot * @param p1 rail type - * @param p2 entrance direction (DiagDirection) + * @param p2 bit 0..1 entrance direction (DiagDirection) * * @todo When checking for the tile slope, * distingush between "Flat land required" and "land sloped in wrong direction" @@ -554,10 +555,12 @@ int32 CmdBuildTrainDepot(TileIndex tile, uint32 flags, uint32 p1, uint32 p2) SET_EXPENSES_TYPE(EXPENSES_CONSTRUCTION); /* check railtype and valid direction for depot (0 through 3), 4 in total */ - if (!ValParamRailtype(p1) || p2 > 3) return CMD_ERROR; + if (!ValParamRailtype(p1)) return CMD_ERROR; tileh = GetTileSlope(tile, NULL); + DiagDirection dir = Extract(p2); + /* Prohibit construction if * The tile is non-flat AND * 1) The AI is "old-school" @@ -570,7 +573,7 @@ int32 CmdBuildTrainDepot(TileIndex tile, uint32 flags, uint32 p1, uint32 p2) _is_old_ai_player || !_patches.build_on_slopes || IsSteepSlope(tileh) || - !CanBuildDepotByTileh(p2, tileh) + !CanBuildDepotByTileh(dir, tileh) )) { return_cmd_error(STR_0007_FLAT_LAND_REQUIRED); } @@ -585,7 +588,6 @@ int32 CmdBuildTrainDepot(TileIndex tile, uint32 flags, uint32 p1, uint32 p2) if (d == NULL) return CMD_ERROR; if (flags & DC_EXEC) { - DiagDirection dir = (DiagDirection)p2; MakeRailDepot(tile, _current_player, dir, (RailType)p1); MarkTileDirtyByTile(tile); diff --git a/src/road_cmd.cpp b/src/road_cmd.cpp index 73b2550960..caf24eb296 100644 --- a/src/road_cmd.cpp +++ b/src/road_cmd.cpp @@ -3,6 +3,7 @@ #include "stdafx.h" #include "openttd.h" #include "bridge_map.h" +#include "cmd_helper.h" #include "rail_map.h" #include "road_map.h" #include "sprite.h" @@ -85,7 +86,7 @@ static bool CheckAllowRemoveRoad(TileIndex tile, RoadBits remove, bool *edge_roa /** Delete a piece of road. * @param tile tile where to remove road from - * @param p1 road piece flags + * @param p1 bit 0..3 road pieces to remove (RoadBits) * @param p2 unused */ int32 CmdRemoveRoad(TileIndex tile, uint32 flags, uint32 p1, uint32 p2) @@ -98,14 +99,9 @@ int32 CmdRemoveRoad(TileIndex tile, uint32 flags, uint32 p1, uint32 p2) /* true if the roadpiece was always removeable, * false if it was a center piece. Affects town ratings drop */ bool edge_road; - RoadBits pieces; SET_EXPENSES_TYPE(EXPENSES_CONSTRUCTION); - /* Road pieces are max 4 bitset values (NE, NW, SE, SW) */ - if (p1 >> 4) return CMD_ERROR; - pieces = (RoadBits)p1; - if (!IsTileType(tile, MP_STREET)) return CMD_ERROR; owner = IsLevelCrossingTile(tile) ? GetCrossingRoadOwner(tile) : GetTileOwner(tile); @@ -116,6 +112,8 @@ int32 CmdRemoveRoad(TileIndex tile, uint32 flags, uint32 p1, uint32 p2) t = NULL; } + RoadBits pieces = Extract(p1); + if (!CheckAllowRemoveRoad(tile, pieces, &edge_road)) return CMD_ERROR; if (!EnsureNoVehicle(tile)) return CMD_ERROR; @@ -249,7 +247,7 @@ static uint32 CheckRoadSlope(Slope tileh, RoadBits* pieces, RoadBits existing) /** Build a piece of road. * @param tile tile where to build road - * @param p1 road piece flags + * @param p1 bit 0..3 road pieces to build (RoadBits) * @param p2 the town that is building the road (0 if not applicable) */ int32 CmdBuildRoad(TileIndex tile, uint32 flags, uint32 p1, uint32 p2) @@ -257,15 +255,15 @@ int32 CmdBuildRoad(TileIndex tile, uint32 flags, uint32 p1, uint32 p2) int32 cost = 0; int32 ret; RoadBits existing = ROAD_NONE; - RoadBits pieces; Slope tileh; SET_EXPENSES_TYPE(EXPENSES_CONSTRUCTION); /* Road pieces are max 4 bitset values (NE, NW, SE, SW) and town can only be non-zero * if a non-player is building the road */ - if ((p1 >> 4) || (IsValidPlayer(_current_player) && p2 != 0) || (_current_player == OWNER_TOWN && !IsValidTownID(p2))) return CMD_ERROR; - pieces = (RoadBits)p1; + if ((IsValidPlayer(_current_player) && p2 != 0) || (_current_player == OWNER_TOWN && !IsValidTownID(p2))) return CMD_ERROR; + + RoadBits pieces = Extract(p1); tileh = GetTileSlope(tile, NULL); @@ -502,7 +500,7 @@ int32 CmdRemoveLongRoad(TileIndex end_tile, uint32 flags, uint32 p1, uint32 p2) /** Build a road depot. * @param tile tile where to build the depot - * @param p1 entrance direction (DiagDirection) + * @param p1 bit 0..1 entrance direction (DiagDirection) * @param p2 unused * * @todo When checking for the tile slope, @@ -516,13 +514,13 @@ int32 CmdBuildRoadDepot(TileIndex tile, uint32 flags, uint32 p1, uint32 p2) SET_EXPENSES_TYPE(EXPENSES_CONSTRUCTION); - if (p1 > 3) return CMD_ERROR; // check direction + DiagDirection dir = Extract(p1); tileh = GetTileSlope(tile, NULL); if (tileh != SLOPE_FLAT && ( !_patches.build_on_slopes || IsSteepSlope(tileh) || - !CanBuildDepotByTileh(p1, tileh) + !CanBuildDepotByTileh(dir, tileh) )) { return_cmd_error(STR_0007_FLAT_LAND_REQUIRED); } @@ -539,7 +537,7 @@ int32 CmdBuildRoadDepot(TileIndex tile, uint32 flags, uint32 p1, uint32 p2) dep->xy = tile; dep->town_index = ClosestTownFromTile(tile, (uint)-1)->index; - MakeRoadDepot(tile, _current_player, (DiagDirection)p1); + MakeRoadDepot(tile, _current_player, dir); MarkTileDirtyByTile(tile); } return cost + _price.build_road_depot; diff --git a/src/station_cmd.cpp b/src/station_cmd.cpp index f60e60ec1e..7bf37354c2 100644 --- a/src/station_cmd.cpp +++ b/src/station_cmd.cpp @@ -6,6 +6,7 @@ #include "openttd.h" #include "aircraft.h" #include "bridge_map.h" +#include "cmd_helper.h" #include "debug.h" #include "functions.h" #include "station_map.h" @@ -787,7 +788,7 @@ static void GetStationLayout(byte *layout, int numtracks, int plat_len, const St /** Build railroad station * @param tile_org starting position of station dragging/placement * @param p1 various bitstuffed elements - * - p1 = (bit 0) - orientation (p1 & 1) + * - p1 = (bit 0) - orientation (Axis) * - p1 = (bit 8-15) - number of tracks * - p1 = (bit 16-23) - platform length * @param p2 various bitstuffed elements @@ -807,7 +808,7 @@ int32 CmdBuildRailroadStation(TileIndex tile_org, uint32 flags, uint32 p1, uint3 if (!ValParamRailtype(p2 & 0xF)) return CMD_ERROR; /* unpack parameters */ - Axis axis = (Axis)GB(p1, 0, 1); + Axis axis = Extract(p1); uint numtracks = GB(p1, 8, 8); uint plat_len = GB(p1, 16, 8); if (axis == AXIS_X) { diff --git a/src/water_cmd.cpp b/src/water_cmd.cpp index b6905ad33a..61db28030a 100644 --- a/src/water_cmd.cpp +++ b/src/water_cmd.cpp @@ -3,6 +3,7 @@ #include "stdafx.h" #include "openttd.h" #include "bridge_map.h" +#include "cmd_helper.h" #include "station_map.h" #include "table/sprites.h" #include "table/strings.h" @@ -45,7 +46,7 @@ static void FloodVehicle(Vehicle *v); /** Build a ship depot. * @param tile tile where ship depot is built - * @param p1 depot direction (0 == X or 1 == Y) + * @param p1 bit 0 depot orientation (Axis) * @param p2 unused */ int32 CmdBuildShipDepot(TileIndex tile, uint32 flags, uint32 p1, uint32 p2) @@ -57,11 +58,11 @@ int32 CmdBuildShipDepot(TileIndex tile, uint32 flags, uint32 p1, uint32 p2) SET_EXPENSES_TYPE(EXPENSES_CONSTRUCTION); - if (p1 > 1) return CMD_ERROR; - if (!EnsureNoVehicle(tile)) return CMD_ERROR; - tile2 = tile + (p1 ? TileDiffXY(0, 1) : TileDiffXY(1, 0)); + Axis axis = Extract(p1); + + tile2 = tile + (axis == AXIS_X ? TileDiffXY(1, 0) : TileDiffXY(0, 1)); if (!EnsureNoVehicle(tile2)) return CMD_ERROR; if (!IsClearWaterTile(tile) || !IsClearWaterTile(tile2)) @@ -84,8 +85,8 @@ int32 CmdBuildShipDepot(TileIndex tile, uint32 flags, uint32 p1, uint32 p2) depot->xy = tile; depot->town_index = ClosestTownFromTile(tile, (uint)-1)->index; - MakeShipDepot(tile, _current_player, DEPOT_NORTH, (Axis)p1); - MakeShipDepot(tile2, _current_player, DEPOT_SOUTH, (Axis)p1); + MakeShipDepot(tile, _current_player, DEPOT_NORTH, axis); + MakeShipDepot(tile2, _current_player, DEPOT_SOUTH, axis); MarkTileDirtyByTile(tile); MarkTileDirtyByTile(tile2); }