From 1cb0cbcb6cb5939d3372127d2424fbc35eb324a0 Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Tue, 19 Aug 2025 18:52:36 +0100 Subject: [PATCH] Codechange: Standardise how AllocateSpecToStation/RoadStop are called. (#14525) Replace use of negative value with std::optional and update doxygen. --- src/newgrf_roadstop.cpp | 16 ++++++++++++++-- src/newgrf_roadstop.h | 2 +- src/newgrf_station.cpp | 9 ++++----- src/newgrf_station.h | 7 +------ src/saveload/waypoint_sl.cpp | 3 ++- src/station_cmd.cpp | 14 +++++++------- src/waypoint_cmd.cpp | 20 ++++++++++---------- 7 files changed, 39 insertions(+), 32 deletions(-) diff --git a/src/newgrf_roadstop.cpp b/src/newgrf_roadstop.cpp index ad0cc06a8b..da022d14f3 100644 --- a/src/newgrf_roadstop.cpp +++ b/src/newgrf_roadstop.cpp @@ -559,7 +559,14 @@ const RoadStopSpec *GetRoadStopSpec(TileIndex t) return specindex < st->roadstop_speclist.size() ? st->roadstop_speclist[specindex].spec : nullptr; } -int AllocateSpecToRoadStop(const RoadStopSpec *statspec, BaseStation *st, bool exec) +/** + * Allocate a RoadStopSpec to a Station. This is called once per build operation. + * @param statspec RoadStopSpec to allocate. + * @param st Station to allocate it to. + * @param exec Whether to actually allocate the spec. + * @return Index within the Station's road stop spec list, or std::nullopt if the allocation failed. + */ +std::optional AllocateSpecToRoadStop(const RoadStopSpec *statspec, BaseStation *st, bool exec) { uint i; @@ -577,7 +584,7 @@ int AllocateSpecToRoadStop(const RoadStopSpec *statspec, BaseStation *st, bool e if (i == NUM_ROADSTOPSPECS_PER_STATION) { /* Full, give up */ - return -1; + return std::nullopt; } if (exec) { @@ -592,6 +599,11 @@ int AllocateSpecToRoadStop(const RoadStopSpec *statspec, BaseStation *st, bool e return i; } +/** + * Deallocate a RoadStopSpec from a Station. Called when removing a single roadstop tile. + * @param st Station to work with. + * @param specindex Index of the custom roadstop within the Station's roadstop spec list. + */ void DeallocateSpecFromRoadStop(BaseStation *st, uint8_t specindex) { /* specindex of 0 (default) is never freeable */ diff --git a/src/newgrf_roadstop.h b/src/newgrf_roadstop.h index 690c9adadb..731772b2e9 100644 --- a/src/newgrf_roadstop.h +++ b/src/newgrf_roadstop.h @@ -178,7 +178,7 @@ bool GetIfClassHasNewStopsByType(const RoadStopClass *roadstopclass, RoadStopTyp bool GetIfStopIsForType(const RoadStopSpec *roadstopspec, RoadStopType rs, RoadType roadtype); const RoadStopSpec *GetRoadStopSpec(TileIndex t); -int AllocateSpecToRoadStop(const RoadStopSpec *statspec, BaseStation *st, bool exec); +std::optional AllocateSpecToRoadStop(const RoadStopSpec *statspec, BaseStation *st, bool exec); void DeallocateSpecFromRoadStop(BaseStation *st, uint8_t specindex); void RoadStopUpdateCachedTriggers(BaseStation *st); diff --git a/src/newgrf_station.cpp b/src/newgrf_station.cpp index 4c16bb7155..2adc68750b 100644 --- a/src/newgrf_station.cpp +++ b/src/newgrf_station.cpp @@ -697,9 +697,9 @@ CommandCost PerformStationTileSlopeCheck(TileIndex north_tile, TileIndex cur_til * @param statspec StationSpec to allocate. * @param st Station to allocate it to. * @param exec Whether to actually allocate the spec. - * @return Index within the Station's spec list, or -1 if the allocation failed. + * @return Index within the Station's station spec list, or std::nullopt if the allocation failed. */ -int AllocateSpecToStation(const StationSpec *statspec, BaseStation *st, bool exec) +std::optional AllocateSpecToStation(const StationSpec *statspec, BaseStation *st, bool exec) { uint i; @@ -719,7 +719,7 @@ int AllocateSpecToStation(const StationSpec *statspec, BaseStation *st, bool exe if (st->speclist[i].spec == statspec) return i; } - return -1; + return std::nullopt; } if (exec) { @@ -738,8 +738,7 @@ int AllocateSpecToStation(const StationSpec *statspec, BaseStation *st, bool exe /** * Deallocate a StationSpec from a Station. Called when removing a single station tile. * @param st Station to work with. - * @param specindex Index of the custom station within the Station's spec list. - * @return Indicates whether the StationSpec was deallocated. + * @param specindex Index of the custom station within the Station's station spec list. */ void DeallocateSpecFromStation(BaseStation *st, uint8_t specindex) { diff --git a/src/newgrf_station.h b/src/newgrf_station.h index 1b74f90e80..cf8e210c72 100644 --- a/src/newgrf_station.h +++ b/src/newgrf_station.h @@ -213,13 +213,8 @@ SpriteID GetCustomStationFoundationRelocation(const StationSpec *statspec, BaseS uint16_t GetStationCallback(CallbackID callback, uint32_t param1, uint32_t param2, const StationSpec *statspec, BaseStation *st, TileIndex tile, std::span regs100 = {}); CommandCost PerformStationTileSlopeCheck(TileIndex north_tile, TileIndex cur_tile, const StationSpec *statspec, Axis axis, uint8_t plat_len, uint8_t numtracks); -/* Allocate a StationSpec to a Station. This is called once per build operation. */ -int AllocateSpecToStation(const StationSpec *statspec, BaseStation *st, bool exec); - -/* Deallocate a StationSpec from a Station. Called when removing a single station tile. */ +std::optional AllocateSpecToStation(const StationSpec *statspec, BaseStation *st, bool exec); void DeallocateSpecFromStation(BaseStation *st, uint8_t specindex); - -/* Draw representation of a station tile for GUI purposes. */ bool DrawStationTile(int x, int y, RailType railtype, Axis axis, StationClassID sclass, uint station); void AnimateStationTile(TileIndex tile); diff --git a/src/saveload/waypoint_sl.cpp b/src/saveload/waypoint_sl.cpp index 74c51befcb..58d014a15d 100644 --- a/src/saveload/waypoint_sl.cpp +++ b/src/saveload/waypoint_sl.cpp @@ -136,7 +136,8 @@ void MoveWaypointsToBaseStations() SetRailStationReservation(tile, reserved); if (wp.spec != nullptr) { - SetCustomStationSpecIndex(tile, AllocateSpecToStation(wp.spec, new_wp, true)); + auto specindex = AllocateSpecToStation(wp.spec, new_wp, true); + SetCustomStationSpecIndex(tile, specindex.value_or(0)); } new_wp->rect.BeforeAddTile(tile, StationRect::ADD_FORCE); diff --git a/src/station_cmd.cpp b/src/station_cmd.cpp index d73bbc1d13..61848a2308 100644 --- a/src/station_cmd.cpp +++ b/src/station_cmd.cpp @@ -1378,8 +1378,8 @@ CommandCost CmdBuildRailStation(DoCommandFlags flags, TileIndex tile_org, RailTy /* Check if we can allocate a custom stationspec to this station */ const StationSpec *statspec = StationClass::Get(spec_class)->GetSpec(spec_index); - int specindex = AllocateSpecToStation(statspec, st, flags.Test(DoCommandFlag::Execute)); - if (specindex == -1) return CommandCost(STR_ERROR_TOO_MANY_STATION_SPECS); + auto specindex = AllocateSpecToStation(statspec, st, flags.Test(DoCommandFlag::Execute)); + if (!specindex.has_value()) return CommandCost(STR_ERROR_TOO_MANY_STATION_SPECS); if (statspec != nullptr) { /* Perform NewStation checks */ @@ -1445,7 +1445,7 @@ CommandCost CmdBuildRailStation(DoCommandFlags flags, TileIndex tile_org, RailTy /* Free the spec if we overbuild something */ DeallocateSpecFromStation(st, old_specindex); - SetCustomStationSpecIndex(tile, specindex); + SetCustomStationSpecIndex(tile, *specindex); SetStationTileRandomBits(tile, GB(Random(), 0, 4)); SetAnimationFrame(tile, 0); @@ -2005,8 +2005,8 @@ CommandCost CmdBuildRoadStop(DoCommandFlags flags, TileIndex tile, uint8_t width if (ret.Failed()) return ret; /* Check if we can allocate a custom stationspec to this station */ - int specindex = AllocateSpecToRoadStop(roadstopspec, st, flags.Test(DoCommandFlag::Execute)); - if (specindex == -1) return CommandCost(STR_ERROR_TOO_MANY_STATION_SPECS); + auto specindex = AllocateSpecToRoadStop(roadstopspec, st, flags.Test(DoCommandFlag::Execute)); + if (!specindex.has_value()) return CommandCost(STR_ERROR_TOO_MANY_STATION_SPECS); if (roadstopspec != nullptr) { /* Perform NewGRF checks */ @@ -2028,7 +2028,7 @@ CommandCost CmdBuildRoadStop(DoCommandFlags flags, TileIndex tile, uint8_t width Owner tram_owner = tram_rt != INVALID_ROADTYPE ? GetRoadOwner(cur_tile, RTT_TRAM) : _current_company; if (IsTileType(cur_tile, MP_STATION) && IsStationRoadStop(cur_tile)) { - RemoveRoadStop(cur_tile, flags, specindex); + RemoveRoadStop(cur_tile, flags, *specindex); } if (roadstopspec != nullptr) { @@ -2076,7 +2076,7 @@ CommandCost CmdBuildRoadStop(DoCommandFlags flags, TileIndex tile, uint8_t width UpdateCompanyRoadInfrastructure(tram_rt, tram_owner, ROAD_STOP_TRACKBIT_FACTOR); Company::Get(st->owner)->infrastructure.station++; - SetCustomRoadStopSpecIndex(cur_tile, specindex); + SetCustomRoadStopSpecIndex(cur_tile, *specindex); if (roadstopspec != nullptr) { st->SetRoadStopRandomBits(cur_tile, GB(Random(), 0, 8)); TriggerRoadStopAnimation(st, cur_tile, StationAnimationTrigger::Built); diff --git a/src/waypoint_cmd.cpp b/src/waypoint_cmd.cpp index 4780ee2cbf..a26b82a0c6 100644 --- a/src/waypoint_cmd.cpp +++ b/src/waypoint_cmd.cpp @@ -264,6 +264,10 @@ CommandCost CmdBuildRailWaypoint(DoCommandFlags flags, TileIndex start_tile, Axi if (!Waypoint::CanAllocateItem()) return CommandCost(STR_ERROR_TOO_MANY_STATIONS_LOADING); } + const StationSpec *spec = StationClass::Get(spec_class)->GetSpec(spec_index); + auto specindex = AllocateSpecToStation(spec, wp, flags.Test(DoCommandFlag::Execute)); + if (!specindex.has_value()) return CommandCost(STR_ERROR_TOO_MANY_STATION_SPECS); + if (flags.Test(DoCommandFlag::Execute)) { if (wp == nullptr) { wp = new Waypoint(start_tile); @@ -285,9 +289,6 @@ CommandCost CmdBuildRailWaypoint(DoCommandFlags flags, TileIndex start_tile, Axi wp->UpdateVirtCoord(); - const StationSpec *spec = StationClass::Get(spec_class)->GetSpec(spec_index); - uint8_t map_spec_index = AllocateSpecToStation(spec, wp, true); - RailStationTileLayout stl{spec, count, 1}; auto it = stl.begin(); @@ -300,7 +301,7 @@ CommandCost CmdBuildRailWaypoint(DoCommandFlags flags, TileIndex start_tile, Axi HasBit(GetRailReservationTrackBits(tile), AxisToTrack(axis)) : HasStationReservation(tile); MakeRailWaypoint(tile, wp->owner, wp->index, axis, *it++, GetRailType(tile)); - SetCustomStationSpecIndex(tile, map_spec_index); + SetCustomStationSpecIndex(tile, *specindex); SetRailStationTileFlags(tile, spec); @@ -385,8 +386,9 @@ CommandCost CmdBuildRoadWaypoint(DoCommandFlags flags, TileIndex start_tile, Axi if (!Waypoint::CanAllocateItem()) return CommandCost(STR_ERROR_TOO_MANY_STATIONS_LOADING); } - /* Check if we can allocate a custom stationspec to this station */ - if (AllocateSpecToRoadStop(roadstopspec, wp, false) == -1) return CommandCost(STR_ERROR_TOO_MANY_STATION_SPECS); + /* Check if we can allocate a custom roadstopspec to this station */ + auto specindex = AllocateSpecToRoadStop(roadstopspec, wp, flags.Test(DoCommandFlag::Execute)); + if (!specindex.has_value()) return CommandCost(STR_ERROR_TOO_MANY_STATION_SPECS); if (flags.Test(DoCommandFlag::Execute)) { if (wp == nullptr) { @@ -415,8 +417,6 @@ CommandCost CmdBuildRoadWaypoint(DoCommandFlags flags, TileIndex start_tile, Axi wp->UpdateVirtCoord(); - uint8_t map_spec_index = AllocateSpecToRoadStop(roadstopspec, wp, true); - /* Check every tile in the area. */ for (TileIndex cur_tile : roadstop_area) { /* Get existing road types and owners before any tile clearing */ @@ -426,7 +426,7 @@ CommandCost CmdBuildRoadWaypoint(DoCommandFlags flags, TileIndex start_tile, Axi Owner tram_owner = tram_rt != INVALID_ROADTYPE ? GetRoadOwner(cur_tile, RTT_TRAM) : _current_company; if (IsRoadWaypointTile(cur_tile)) { - RemoveRoadWaypointStop(cur_tile, flags, map_spec_index); + RemoveRoadWaypointStop(cur_tile, flags, *specindex); } wp->road_waypoint_area.Add(cur_tile); @@ -444,7 +444,7 @@ CommandCost CmdBuildRoadWaypoint(DoCommandFlags flags, TileIndex start_tile, Axi UpdateCompanyRoadInfrastructure(tram_rt, tram_owner, ROAD_STOP_TRACKBIT_FACTOR); MakeDriveThroughRoadStop(cur_tile, wp->owner, road_owner, tram_owner, wp->index, StationType::RoadWaypoint, road_rt, tram_rt, axis); - SetCustomRoadStopSpecIndex(cur_tile, map_spec_index); + SetCustomRoadStopSpecIndex(cur_tile, *specindex); if (roadstopspec != nullptr) wp->SetRoadStopRandomBits(cur_tile, 0); Company::Get(wp->owner)->infrastructure.station++;