From da2f1811044dcd928960ad6a91d7f65b9166f591 Mon Sep 17 00:00:00 2001 From: akrieger Date: Sun, 1 Oct 2023 17:37:53 -0700 Subject: [PATCH] Fix duplicate OMTs, take two (#68398) * Extract layers deserialize so information can be used for omt deduplication * Redo linear omt deduplication with extra comments and more consistency. --- src/overmap.cpp | 35 +++++++++---- src/savegame.cpp | 133 +++++++++++++++++++++++++++++------------------ 2 files changed, 107 insertions(+), 61 deletions(-) diff --git a/src/overmap.cpp b/src/overmap.cpp index 713346ffb6de1..9ed61aecfd160 100644 --- a/src/overmap.cpp +++ b/src/overmap.cpp @@ -2888,21 +2888,38 @@ void overmap::ter_set( const tripoint_om_omt &p, const oter_id &id ) return; } - oter_id &val = layer[p.z() + OVERMAP_DEPTH].terrain[p.xy()]; + oter_id ¤t_oter = layer[p.z() + OVERMAP_DEPTH].terrain[p.xy()]; + const oter_type_str_id ¤t_type_id = current_oter->get_type_id(); + const oter_type_str_id &incoming_type_id = id->get_type_id(); + const bool current_type_same = current_type_id == incoming_type_id; + + // Mapgen refinement can push multiple different roads over each other. + // Roads require a predecessor. A road pushed over a road might cause a + // road to be a predecessor to another road. That causes too many spawns + // to happen. So when pushing a predecessor, if the predecessor to-be-pushed + // is linear and the previous predecessor is linear, overwrite it. + // This way only the 'last' rotation/variation generated is kept. if( id->has_flag( oter_flags::requires_predecessor ) ) { - // Stops linear fallback_predecessor maps (roads etc) spawning over themselves std::vector &om_predecessors = predecessors_[p]; - if( om_predecessors.empty() || !val->is_linear() ) { - om_predecessors.push_back( val ); - } else { - // Collapse and keep the last linear oter of matching type + if( om_predecessors.empty() || ( !current_oter->is_linear() && !current_type_same ) ) { + // If we need a predecessor, we must have a predecessor no matter what. + // Or, if the oter to-be-pushed is not linear, push it only if the incoming oter is different. + om_predecessors.push_back( current_oter ); + } else if( !current_type_same ) { + // Current oter is linear, incoming oter is different from current. + // If the last predecessor is the same type as the current type, overwrite. + // Else push the current type. oter_id &last_predecessor = om_predecessors.back(); - if( last_predecessor->get_type_id() == val->get_type_id() ) { - last_predecessor = val; + if( last_predecessor->get_type_id() == current_type_id ) { + last_predecessor = current_oter; + } else { + om_predecessors.push_back( current_oter ); } } + // We had a predecessor, and it was the same type as the incoming one + // Don't push another copy. } - val = id; + current_oter = id; } const oter_id &overmap::ter( const tripoint_om_omt &p ) const diff --git a/src/savegame.cpp b/src/savegame.cpp index eb3518b8e310b..1cfb053179184 100644 --- a/src/savegame.cpp +++ b/src/savegame.cpp @@ -411,46 +411,48 @@ void overmap::unserialize( const JsonObject &jsobj ) mapgen_args_index.emplace( p.first, &*it ); } } - for( JsonMember om_member : jsobj ) { - const std::string name = om_member.name(); - if( name == "layers" ) { - std::unordered_map oter_id_migrations; - JsonArray layers_json = om_member; + // Extract layers first so predecessor deduplication can happen. + if( jsobj.has_member( "layers" ) ) { + std::unordered_map oter_id_migrations; + JsonArray layers_json = jsobj.get_array( "layers" ); - for( int z = 0; z < OVERMAP_LAYERS; ++z ) { - JsonArray layer_json = layers_json.next_array(); - int count = 0; - std::string tmp_ter; - oter_id tmp_otid( 0 ); - for( int j = 0; j < OMAPY; j++ ) { - for( int i = 0; i < OMAPX; i++ ) { - if( count == 0 ) { - { - JsonArray rle_terrain = layer_json.next_array(); - tmp_ter = rle_terrain.next_string(); - count = rle_terrain.next_int(); - if( rle_terrain.has_more() ) { - rle_terrain.throw_error( 2, "Unexpected value in RLE encoding" ); - } + for( int z = 0; z < OVERMAP_LAYERS; ++z ) { + JsonArray layer_json = layers_json.next_array(); + int count = 0; + std::string tmp_ter; + oter_id tmp_otid( 0 ); + for( int j = 0; j < OMAPY; j++ ) { + for( int i = 0; i < OMAPX; i++ ) { + if( count == 0 ) { + { + JsonArray rle_terrain = layer_json.next_array(); + tmp_ter = rle_terrain.next_string(); + count = rle_terrain.next_int(); + if( rle_terrain.has_more() ) { + rle_terrain.throw_error( 2, "Unexpected value in RLE encoding" ); } - if( is_oter_id_obsolete( tmp_ter ) ) { - for( int p = i; p < i + count; p++ ) { - oter_id_migrations.emplace( tripoint_om_omt( p, j, z - OVERMAP_DEPTH ), tmp_ter ); - } - } else if( oter_str_id( tmp_ter ).is_valid() ) { - tmp_otid = oter_id( tmp_ter ); - } else { - debugmsg( "Loaded invalid oter_id '%s'", tmp_ter.c_str() ); - tmp_otid = oter_omt_obsolete; + } + if( is_oter_id_obsolete( tmp_ter ) ) { + for( int p = i; p < i + count; p++ ) { + oter_id_migrations.emplace( tripoint_om_omt( p, j, z - OVERMAP_DEPTH ), tmp_ter ); } + } else if( oter_str_id( tmp_ter ).is_valid() ) { + tmp_otid = oter_id( tmp_ter ); + } else { + debugmsg( "Loaded invalid oter_id '%s'", tmp_ter.c_str() ); + tmp_otid = oter_omt_obsolete; } - count--; - layer[z].terrain[i][j] = tmp_otid; } + count--; + layer[z].terrain[i][j] = tmp_otid; } } - migrate_oter_ids( oter_id_migrations ); - } else if( name == "region_id" ) { + } + migrate_oter_ids( oter_id_migrations ); + } + for( JsonMember om_member : jsobj ) { + const std::string name = om_member.name(); + if( name == "region_id" ) { std::string new_region_id; om_member.read( new_region_id ); if( settings->id != new_region_id ) { @@ -657,30 +659,57 @@ void overmap::unserialize( const JsonObject &jsobj ) } else if( name == "predecessors" ) { std::vector>> flattened_predecessors; om_member.read( flattened_predecessors, true ); - std::vector deduped_predecessors; - for( std::pair> &p : flattened_predecessors ) { - // TODO remove after 0.H release. - // JSONizing roads caused some bad mapgen data to get saved to disk. Repeated redundant linear - // type omts were all saved. The new logic only pushes a linear predecessor if the predecessors - // list is empty or the incoming omt is not linear. Fixup bad saves to conform. - deduped_predecessors.reserve( p.second.size() ); - for( const oter_id &id : p.second ) { - if( deduped_predecessors.empty() || !id->is_linear() ) { - deduped_predecessors.push_back( id ); - } else { - oter_id &last_predecessor = deduped_predecessors.back(); - if( last_predecessor->get_type_id() == id->get_type_id() ) { - last_predecessor = id; - } else { - deduped_predecessors.push_back( id ); + std::vector om_predecessors; + + for( auto& [p, serialized_predecessors] : flattened_predecessors ) { + if( !serialized_predecessors.empty() ) { + // TODO remove after 0.H release. + // JSONizing roads caused some bad mapgen data to get saved to disk. Fixup bad saves to conform. + // The logic to do this is to emulate setting overmap::set_ter repeatedly. The difference is the + // 'original' terrain is lost, all we have is a chain of predecessors. + // This doesn't matter for the sake of deduplicating predecessors. + // + // Mapgen refinement can push multiple different roads over each other. + // Roads require a predecessor. A road pushed over a road might cause a + // road to be a predecessor to another road. That causes too many spawns + // to happen. So when pushing a predecessor, if the predecessor to-be-pushed + // is linear and the previous predecessor is linear, overwrite it. + // This way only the 'last' rotation/variation generated is kept. + om_predecessors.reserve( serialized_predecessors.size() ); + oter_id current_oter; + auto local_set_ter = [&]( oter_id & id ) { + const oter_type_str_id ¤t_type_id = current_oter->get_type_id(); + const oter_type_str_id &incoming_type_id = id->get_type_id(); + const bool current_type_same = current_type_id == incoming_type_id; + if( om_predecessors.empty() || ( !current_oter->is_linear() && !current_type_same ) ) { + // If we need a predecessor, we must have a predecessor no matter what. + // Or, if the oter to-be-pushed is not linear, push it only if the incoming oter is different. + om_predecessors.push_back( current_oter ); + } else if( !current_type_same ) { + // Current oter is linear, incoming oter is different from current. + // If the last predecessor is the same type as the current type, overwrite. + // Else push the current type. + oter_id &last_predecessor = om_predecessors.back(); + if( last_predecessor->get_type_id() == current_type_id ) { + last_predecessor = current_oter; + } else { + om_predecessors.push_back( current_oter ); + } } + current_oter = id; + }; + + current_oter = serialized_predecessors.front(); + for( size_t i = 1; i < serialized_predecessors.size(); ++i ) { + local_set_ter( serialized_predecessors[i] ); } + local_set_ter( layer[p.z() + OVERMAP_DEPTH].terrain[p.xy()] ); } - predecessors_.insert_or_assign( p.first, std::move( deduped_predecessors ) ); + predecessors_.insert_or_assign( p, std::move( om_predecessors ) ); // Reuse allocations because it's a good habit. - deduped_predecessors = std::move( p.second ); - deduped_predecessors.clear(); + om_predecessors = std::move( serialized_predecessors ); + om_predecessors.clear(); } } }