From e9b13f89a3a4c78bfe846a44c2420b9f18d885fd Mon Sep 17 00:00:00 2001 From: rubidium Date: Sat, 2 Oct 2010 14:41:25 +0000 Subject: [PATCH] (svn r20871) [1.0] -Backport from trunk: - Fix: [NewGRF] Clamp/convert some vehicle variables so NewGRFs get their specified range (r20800, r20799, r20795) - Fix: [NoAI] Document that AITile::HasTransportType does not work for TRANSPORT_AIR [FS#4117] (r20798) - Fix: [NewGRF] Disable houses without a size that are available according to their building flags (r20797) - Fix: [NewGRF] Make sure all houses in the house spec array are valid. It was possible that part of a multitile house was not copied because the array was full (r20796) --- src/ai/api/ai_tile.hpp | 4 ++ src/house.h | 3 +- src/newgrf.cpp | 92 +++++++++++++++++++++++++++++------------- src/newgrf_engine.cpp | 26 +++++++----- 4 files changed, 86 insertions(+), 39 deletions(-) diff --git a/src/ai/api/ai_tile.hpp b/src/ai/api/ai_tile.hpp index be5f8ff7dc..09db9f96a1 100644 --- a/src/ai/api/ai_tile.hpp +++ b/src/ai/api/ai_tile.hpp @@ -297,10 +297,14 @@ public: * @param tile The tile to check. * @param transport_type The TransportType to check against. * @pre AIMap::IsValidTile(tile). + * @pre transport_type != TRANSPORT_AIR. * @note Returns false on tiles with roadworks and on road tiles with only * a single piece of road as these tiles cannot be used to transport * anything on. It furthermore returns true on some coast tile for * TRANSPORT_WATER because ships can navigate over them. + * @note Use AIAirport.IsAirportTile to check for airport tiles. Aircraft + * can fly over every tile on the map so using HasTransportType + * doesn't make sense for TRANSPORT_AIR. * @return True if and only if the tile has the given TransportType. */ static bool HasTransportType(TileIndex tile, TransportType transport_type); diff --git a/src/house.h b/src/house.h index 15b0a38bc7..c1772e60aa 100644 --- a/src/house.h +++ b/src/house.h @@ -76,7 +76,8 @@ enum HouseZones { ///< Bit Value Meaning HZ_TEMP = 0x1000, ///< 12 1000 can appear in temperate climate HZ_SUBARTC_BELOW = 0x2000, ///< 13 2000 can appear in sub-arctic climate below the snow line HZ_SUBTROPIC = 0x4000, ///< 14 4000 can appear in subtropical climate - HZ_TOYLND = 0x8000 ///< 15 8000 can appear in toyland climate + HZ_TOYLND = 0x8000, ///< 15 8000 can appear in toyland climate + HZ_CLIMALL = 0xF800, ///< Bitmask of all climate bits }; DECLARE_ENUM_AS_BIT_SET(HouseZones) diff --git a/src/newgrf.cpp b/src/newgrf.cpp index a68f6babc2..f324b5e256 100644 --- a/src/newgrf.cpp +++ b/src/newgrf.cpp @@ -6193,7 +6193,59 @@ static void FinaliseCargoArray() } } -/** Add all new houses to the house array. House properties can be set at any +/** + * Check if a given housespec is valid and disable it if it's not. + * The housespecs that follow it are used to check the validity of + * multitile houses. + * @param hs The housespec to check. + * @param next1 The housespec that follows \c hs. + * @param next2 The housespec that follows \c next1. + * @param next3 The housespec that follows \c next2. + * @param filename The filename of the newgrf this house was defined in. + * @return Whether the given housespec is valid. + */ +static bool IsHouseSpecValid(HouseSpec *hs, const HouseSpec *next1, const HouseSpec *next2, const HouseSpec *next3, const char *filename) +{ + if (((hs->building_flags & BUILDING_HAS_2_TILES) != 0 && + (next1 == NULL || !next1->enabled || (next1->building_flags & BUILDING_HAS_1_TILE) != 0)) || + ((hs->building_flags & BUILDING_HAS_4_TILES) != 0 && + (next2 == NULL || !next2->enabled || (next2->building_flags & BUILDING_HAS_1_TILE) != 0 || + next3 == NULL || !next3->enabled || (next3->building_flags & BUILDING_HAS_1_TILE) != 0))) { + hs->enabled = false; + if (filename != NULL) DEBUG(grf, 1, "FinaliseHouseArray: %s defines house %d as multitile, but no suitable tiles follow. Disabling house.", filename, hs->local_id); + return false; + } + + /* Some places sum population by only counting north tiles. Other places use all tiles causing desyncs. + * As the newgrf specs define population to be zero for non-north tiles, we just disable the offending house. + * If you want to allow non-zero populations somewhen, make sure to sum the population of all tiles in all places. */ + if (((hs->building_flags & BUILDING_HAS_2_TILES) != 0 && next1->population != 0) || + ((hs->building_flags & BUILDING_HAS_4_TILES) != 0 && (next2->population != 0 || next3->population != 0))) { + hs->enabled = false; + if (filename != NULL) DEBUG(grf, 1, "FinaliseHouseArray: %s defines multitile house %d with non-zero population on additional tiles. Disabling house.", filename, hs->local_id); + return false; + } + + /* Substitute type is also used for override, and having an override with a different size causes crashes. + * This check should only be done for NewGRF houses because substitute_id is not set for original houses.*/ + if (filename != NULL && (hs->building_flags & BUILDING_HAS_1_TILE) != (HouseSpec::Get(hs->substitute_id)->building_flags & BUILDING_HAS_1_TILE)) { + hs->enabled = false; + DEBUG(grf, 1, "FinaliseHouseArray: %s defines house %d with different house size then it's substitute type. Disabling house.", filename, hs->local_id); + return false; + } + + /* Make sure that additional parts of multitile houses are not available. */ + if ((hs->building_flags & BUILDING_HAS_1_TILE) == 0 && (hs->building_availability & HZ_ZONALL) != 0 && (hs->building_availability & HZ_CLIMALL) != 0) { + hs->enabled = false; + if (filename != NULL) DEBUG(grf, 1, "FinaliseHouseArray: %s defines house %d without a size but marked it as available. Disabling house.", filename, hs->local_id); + return false; + } + + return true; +} + +/** + * Add all new houses to the house array. House properties can be set at any * time in the GRF file, so we can only add a house spec to the house array * after the file has finished loading. We also need to check the dates, due to * the TTDPatch behaviour described below that we need to emulate. */ @@ -6224,38 +6276,24 @@ static void FinaliseHouseArray() const HouseSpec *next2 = (i + 2 < HOUSE_MAX ? housespec[i + 2] : NULL); const HouseSpec *next3 = (i + 3 < HOUSE_MAX ? housespec[i + 3] : NULL); - if (((hs->building_flags & BUILDING_HAS_2_TILES) != 0 && - (next1 == NULL || !next1->enabled || (next1->building_flags & BUILDING_HAS_1_TILE) != 0)) || - ((hs->building_flags & BUILDING_HAS_4_TILES) != 0 && - (next2 == NULL || !next2->enabled || (next2->building_flags & BUILDING_HAS_1_TILE) != 0 || - next3 == NULL || !next3->enabled || (next3->building_flags & BUILDING_HAS_1_TILE) != 0))) { - hs->enabled = false; - DEBUG(grf, 1, "FinaliseHouseArray: %s defines house %d as multitile, but no suitable tiles follow. Disabling house.", (*file)->filename, hs->local_id); - continue; - } - - /* Some places sum population by only counting north tiles. Other places use all tiles causing desyncs. - * As the newgrf specs define population to be zero for non-north tiles, we just disable the offending house. - * If you want to allow non-zero populations somewhen, make sure to sum the population of all tiles in all places. */ - if (((hs->building_flags & BUILDING_HAS_2_TILES) != 0 && next1->population != 0) || - ((hs->building_flags & BUILDING_HAS_4_TILES) != 0 && (next2->population != 0 || next3->population != 0))) { - hs->enabled = false; - DEBUG(grf, 1, "FinaliseHouseArray: %s defines multitile house %d with non-zero population on additional tiles. Disabling house.", (*file)->filename, hs->local_id); - continue; - } - - /* Substitute type is also used for override, and having an override with a different size causes crashes. */ - if ((hs->building_flags & BUILDING_HAS_1_TILE) != (HouseSpec::Get(hs->substitute_id)->building_flags & BUILDING_HAS_1_TILE)) { - hs->enabled = false; - DEBUG(grf, 1, "FinaliseHouseArray: %s defines house %d with different house size then it's substitute type. Disabling house.", (*file)->filename, hs->local_id); - continue; - } + if (!IsHouseSpecValid(hs, next1, next2, next3, (*file)->filename)) continue; _house_mngr.SetEntitySpec(hs); if (hs->min_year < min_year) min_year = hs->min_year; } } + for (int i = 0; i < HOUSE_MAX; i++) { + HouseSpec *hs = HouseSpec::Get(i); + const HouseSpec *next1 = (i + 1 < HOUSE_MAX ? HouseSpec::Get(i + 1) : NULL); + const HouseSpec *next2 = (i + 2 < HOUSE_MAX ? HouseSpec::Get(i + 2) : NULL); + const HouseSpec *next3 = (i + 3 < HOUSE_MAX ? HouseSpec::Get(i + 3) : NULL); + + /* We need to check all houses again to we are sure that multitile houses + * did get consecutive IDs and none of the parts are missing. */ + IsHouseSpecValid(hs, next1, next2, next3, NULL); + } + if (min_year != 0) { for (int i = 0; i < HOUSE_MAX; i++) { HouseSpec *hs = HouseSpec::Get(i); diff --git a/src/newgrf_engine.cpp b/src/newgrf_engine.cpp index cc34ad088d..a3f49ab50f 100644 --- a/src/newgrf_engine.cpp +++ b/src/newgrf_engine.cpp @@ -731,14 +731,18 @@ static uint32 VehicleGetVariable(const ResolverObject *object, byte variable, by } return (variable - 0x80) == 0x10 ? ticks : GB(ticks, 8, 8); } - case 0x12: return max(v->date_of_last_service - DAYS_TILL_ORIGINAL_BASE_YEAR, 0); - case 0x13: return GB(max(v->date_of_last_service - DAYS_TILL_ORIGINAL_BASE_YEAR, 0), 8, 8); + case 0x12: return Clamp(v->date_of_last_service - DAYS_TILL_ORIGINAL_BASE_YEAR, 0, 0xFFFF); + case 0x13: return GB(Clamp(v->date_of_last_service - DAYS_TILL_ORIGINAL_BASE_YEAR, 0, 0xFFFF), 8, 8); case 0x14: return v->service_interval; case 0x15: return GB(v->service_interval, 8, 8); case 0x16: return v->last_station_visited; case 0x17: return v->tick_counter; - case 0x18: return v->max_speed; - case 0x19: return GB(v->max_speed, 8, 8); + case 0x18: + case 0x19: { + uint max_speed = v->type == VEH_TRAIN ? Train::From(v)->tcache.cached_max_speed : v->max_speed; + if (v->type == VEH_AIRCRAFT) max_speed = max_speed * 10 / 128; + return (variable - 0x80) == 0x18 ? max_speed : GB(max_speed, 8, 8); + } case 0x1A: return v->x_pos; case 0x1B: return GB(v->x_pos, 8, 8); case 0x1C: return v->y_pos; @@ -749,21 +753,21 @@ static uint32 VehicleGetVariable(const ResolverObject *object, byte variable, by case 0x29: return GB(v->cur_image, 8, 8); case 0x32: return v->vehstatus; case 0x33: return 0; // non-existent high byte of vehstatus - case 0x34: return v->cur_speed; - case 0x35: return GB(v->cur_speed, 8, 8); + case 0x34: return v->type == VEH_AIRCRAFT ? (v->cur_speed * 10) / 128 : v->cur_speed; + case 0x35: return GB(v->type == VEH_AIRCRAFT ? (v->cur_speed * 10) / 128 : v->cur_speed, 8, 8); case 0x36: return v->subspeed; case 0x37: return v->acceleration; case 0x39: return v->cargo_type; case 0x3A: return v->cargo_cap; case 0x3B: return GB(v->cargo_cap, 8, 8); - case 0x3C: return v->cargo.Count(); - case 0x3D: return GB(v->cargo.Count(), 8, 8); + case 0x3C: return ClampToU16(v->cargo.Count()); + case 0x3D: return GB(ClampToU16(v->cargo.Count()), 8, 8); case 0x3E: return v->cargo.Source(); - case 0x3F: return v->cargo.DaysInTransit(); + case 0x3F: return ClampU(v->cargo.DaysInTransit(), 0, 0xFF); case 0x40: return v->age; case 0x41: return GB(v->age, 8, 8); - case 0x42: return v->max_age; - case 0x43: return GB(v->max_age, 8, 8); + case 0x42: return ClampToU16(v->max_age); + case 0x43: return GB(ClampToU16(v->max_age), 8, 8); case 0x44: return Clamp(v->build_year, ORIGINAL_BASE_YEAR, ORIGINAL_MAX_YEAR) - ORIGINAL_BASE_YEAR; case 0x45: return v->unitnumber; case 0x46: return Engine::Get(v->engine_type)->internal_id;