From 71318dbcb35e716e99b10fcd666b1e28af5cec43 Mon Sep 17 00:00:00 2001 From: Hielke Morsink Date: Sun, 15 May 2022 00:00:47 +0200 Subject: [PATCH 1/4] Fix #17205: Crash in mapgen when base terrain is not selected --- distribution/changelog.txt | 1 + src/openrct2/world/MapGen.cpp | 42 +++++++++++++++++++++++------------ 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/distribution/changelog.txt b/distribution/changelog.txt index ad6ab81e3a..12780c1b82 100644 --- a/distribution/changelog.txt +++ b/distribution/changelog.txt @@ -19,6 +19,7 @@ - Fix: [#17099] Object selection thumbnail box is one pixel too tall. - Fix: [#17104] Changing map size does not invalidate park size. - Fix: [#17197] Segfault when extracting files from the GOG installer. +- Fix: [#17205] Map generator sometimes crashes when not all standard terrain objects are available. 0.4.0 (2022-04-25) ------------------------------------------------------------------------ diff --git a/src/openrct2/world/MapGen.cpp b/src/openrct2/world/MapGen.cpp index 3640534ce8..2f87e65520 100644 --- a/src/openrct2/world/MapGen.cpp +++ b/src/openrct2/world/MapGen.cpp @@ -144,12 +144,23 @@ void mapgen_generate(mapgen_settings* settings) auto mapSize = settings->mapSize; auto waterLevel = settings->water_level; const auto selectedFloor = TerrainSurfaceObject::GetById(settings->floor); - std::string floorTexture = selectedFloor != nullptr ? std::string(selectedFloor->GetIdentifier()) : ""; + std::string_view floorTexture = selectedFloor != nullptr ? selectedFloor->GetIdentifier() : ""; const auto selectedEdge = TerrainEdgeObject::GetById(settings->wall); - std::string edgeTexture = selectedFloor != nullptr ? std::string(selectedEdge->GetIdentifier()) : ""; + std::string_view edgeTexture = selectedFloor != nullptr ? selectedEdge->GetIdentifier() : ""; if (floorTexture.empty()) - floorTexture = BaseTerrain[util_rand() % std::size(BaseTerrain)]; + { + std::vector availableTerrains; + std::copy_if(std::begin(BaseTerrain), std::end(BaseTerrain), std::back_inserter(availableTerrains), [](auto terrain) { + return object_manager_get_loaded_object(ObjectEntryDescriptor(terrain)) != nullptr; + }); + + if (availableTerrains.empty()) + // Fall back to the first available surface texture that is available in the park + floorTexture = TerrainSurfaceObject::GetById(0)->GetIdentifier(); + else + floorTexture = availableTerrains[util_rand() % availableTerrains.size()]; + } if (edgeTexture.empty()) { @@ -160,6 +171,10 @@ void mapgen_generate(mapgen_settings* settings) edgeTexture = "rct2.terrain_edge.ice"; else edgeTexture = "rct2.terrain_edge.rock"; + + // Fall back to the first available edge texture that is available in the park + if (object_manager_get_loaded_object(ObjectEntryDescriptor(edgeTexture)) == nullptr) + edgeTexture = TerrainEdgeObject::GetById(0)->GetIdentifier(); } auto floorTextureId = object_manager_get_loaded_object_entry_index(ObjectEntryDescriptor(floorTexture)); @@ -205,18 +220,17 @@ void mapgen_generate(mapgen_settings* settings) mapgen_set_water_level(waterLevel); // Add sandy beaches - std::string beachTexture = std::string(floorTexture); - if (settings->floor == -1 && floorTexture == "rct2.terrain_surface.grass") + std::string_view beachTexture = floorTexture; + if (settings->floor == -1 && floorTexture == "rct2.terrain_surface.grass" && (util_rand() & 1)) { - switch (util_rand() % 4) - { - case 0: - beachTexture = "rct2.terrain_surface.sand"; - break; - case 1: - beachTexture = "rct2.terrain_surface.sand_brown"; - break; - } + std::vector availableBeachTextures; + if (object_manager_get_loaded_object(ObjectEntryDescriptor("rct2.terrain_surface.sand")) != nullptr) + availableBeachTextures.push_back("rct2.terrain_surface.sand"); + if (object_manager_get_loaded_object(ObjectEntryDescriptor("rct2.terrain_surface.sand_brown")) != nullptr) + availableBeachTextures.push_back("rct2.terrain_surface.sand_brown"); + + if (!availableBeachTextures.empty()) + beachTexture = availableBeachTextures[util_rand() % availableBeachTextures.size()]; } auto beachTextureId = object_manager_get_loaded_object_entry_index(ObjectEntryDescriptor(beachTexture)); From 52eeb702e0d9462aeec91e06b0bc26aecb477599 Mon Sep 17 00:00:00 2001 From: Hielke Morsink Date: Sun, 15 May 2022 00:01:55 +0200 Subject: [PATCH 2/4] Fix TerrainEdgeObject::GetById returning surface instead --- src/openrct2/object/TerrainEdgeObject.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openrct2/object/TerrainEdgeObject.cpp b/src/openrct2/object/TerrainEdgeObject.cpp index 2580ab83fc..2fb1ea3591 100644 --- a/src/openrct2/object/TerrainEdgeObject.cpp +++ b/src/openrct2/object/TerrainEdgeObject.cpp @@ -64,6 +64,6 @@ void TerrainEdgeObject::ReadJson(IReadObjectContext* context, json_t& root) TerrainEdgeObject* TerrainEdgeObject::GetById(ObjectEntryIndex entryIndex) { auto& objMgr = OpenRCT2::GetContext()->GetObjectManager(); - auto obj = objMgr.GetLoadedObject(ObjectType::TerrainSurface, entryIndex); + auto obj = objMgr.GetLoadedObject(ObjectType::TerrainEdge, entryIndex); return obj != nullptr ? static_cast(obj) : nullptr; } From f9676d1863f76b996f51870bfc7bc515c7df9cd8 Mon Sep 17 00:00:00 2001 From: Hielke Morsink Date: Sun, 15 May 2022 10:34:32 +0200 Subject: [PATCH 3/4] Use object manager interface --- src/openrct2/world/MapGen.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/openrct2/world/MapGen.cpp b/src/openrct2/world/MapGen.cpp index 2f87e65520..ae342e2126 100644 --- a/src/openrct2/world/MapGen.cpp +++ b/src/openrct2/world/MapGen.cpp @@ -147,12 +147,13 @@ void mapgen_generate(mapgen_settings* settings) std::string_view floorTexture = selectedFloor != nullptr ? selectedFloor->GetIdentifier() : ""; const auto selectedEdge = TerrainEdgeObject::GetById(settings->wall); std::string_view edgeTexture = selectedFloor != nullptr ? selectedEdge->GetIdentifier() : ""; + auto& objectManager = OpenRCT2::GetContext()->GetObjectManager(); if (floorTexture.empty()) { std::vector availableTerrains; - std::copy_if(std::begin(BaseTerrain), std::end(BaseTerrain), std::back_inserter(availableTerrains), [](auto terrain) { - return object_manager_get_loaded_object(ObjectEntryDescriptor(terrain)) != nullptr; + std::copy_if(std::begin(BaseTerrain), std::end(BaseTerrain), std::back_inserter(availableTerrains), [&](auto terrain) { + return objectManager.GetLoadedObject(ObjectEntryDescriptor(terrain)) != nullptr; }); if (availableTerrains.empty()) @@ -173,12 +174,12 @@ void mapgen_generate(mapgen_settings* settings) edgeTexture = "rct2.terrain_edge.rock"; // Fall back to the first available edge texture that is available in the park - if (object_manager_get_loaded_object(ObjectEntryDescriptor(edgeTexture)) == nullptr) + if (objectManager.GetLoadedObject(ObjectEntryDescriptor(edgeTexture)) == nullptr) edgeTexture = TerrainEdgeObject::GetById(0)->GetIdentifier(); } - auto floorTextureId = object_manager_get_loaded_object_entry_index(ObjectEntryDescriptor(floorTexture)); - auto edgeTextureId = object_manager_get_loaded_object_entry_index(ObjectEntryDescriptor(edgeTexture)); + auto floorTextureId = objectManager.GetLoadedObjectEntryIndex(ObjectEntryDescriptor(floorTexture)); + auto edgeTextureId = objectManager.GetLoadedObjectEntryIndex(ObjectEntryDescriptor(edgeTexture)); map_clear_all_elements(); @@ -224,15 +225,15 @@ void mapgen_generate(mapgen_settings* settings) if (settings->floor == -1 && floorTexture == "rct2.terrain_surface.grass" && (util_rand() & 1)) { std::vector availableBeachTextures; - if (object_manager_get_loaded_object(ObjectEntryDescriptor("rct2.terrain_surface.sand")) != nullptr) + if (objectManager.GetLoadedObject(ObjectEntryDescriptor("rct2.terrain_surface.sand")) != nullptr) availableBeachTextures.push_back("rct2.terrain_surface.sand"); - if (object_manager_get_loaded_object(ObjectEntryDescriptor("rct2.terrain_surface.sand_brown")) != nullptr) + if (objectManager.GetLoadedObject(ObjectEntryDescriptor("rct2.terrain_surface.sand_brown")) != nullptr) availableBeachTextures.push_back("rct2.terrain_surface.sand_brown"); if (!availableBeachTextures.empty()) beachTexture = availableBeachTextures[util_rand() % availableBeachTextures.size()]; } - auto beachTextureId = object_manager_get_loaded_object_entry_index(ObjectEntryDescriptor(beachTexture)); + auto beachTextureId = objectManager.GetLoadedObjectEntryIndex(ObjectEntryDescriptor(beachTexture)); for (auto y = 1; y < mapSize - 1; y++) { From 1a3dae6f568dfdbfe5596b8f1b6a054052c02257 Mon Sep 17 00:00:00 2001 From: Hielke Morsink Date: Sun, 15 May 2022 11:18:15 +0200 Subject: [PATCH 4/4] Remove unnecessary null checks static_cast on a nullptr is defined behaviour. --- src/openrct2/object/TerrainEdgeObject.cpp | 4 ++-- src/openrct2/object/TerrainSurfaceObject.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/openrct2/object/TerrainEdgeObject.cpp b/src/openrct2/object/TerrainEdgeObject.cpp index 2fb1ea3591..3bccf9c044 100644 --- a/src/openrct2/object/TerrainEdgeObject.cpp +++ b/src/openrct2/object/TerrainEdgeObject.cpp @@ -64,6 +64,6 @@ void TerrainEdgeObject::ReadJson(IReadObjectContext* context, json_t& root) TerrainEdgeObject* TerrainEdgeObject::GetById(ObjectEntryIndex entryIndex) { auto& objMgr = OpenRCT2::GetContext()->GetObjectManager(); - auto obj = objMgr.GetLoadedObject(ObjectType::TerrainEdge, entryIndex); - return obj != nullptr ? static_cast(obj) : nullptr; + auto* obj = objMgr.GetLoadedObject(ObjectType::TerrainEdge, entryIndex); + return static_cast(obj); } diff --git a/src/openrct2/object/TerrainSurfaceObject.cpp b/src/openrct2/object/TerrainSurfaceObject.cpp index 7adf3629d7..e90e8a72ba 100644 --- a/src/openrct2/object/TerrainSurfaceObject.cpp +++ b/src/openrct2/object/TerrainSurfaceObject.cpp @@ -160,6 +160,6 @@ uint32_t TerrainSurfaceObject::GetImageId( TerrainSurfaceObject* TerrainSurfaceObject::GetById(ObjectEntryIndex entryIndex) { auto& objMgr = OpenRCT2::GetContext()->GetObjectManager(); - auto obj = objMgr.GetLoadedObject(ObjectType::TerrainSurface, entryIndex); - return obj != nullptr ? static_cast(obj) : nullptr; + auto* obj = objMgr.GetLoadedObject(ObjectType::TerrainSurface, entryIndex); + return static_cast(obj); }