Predecessor overmap terrains should be migrated

[hexagonrecursion]

Describe the bug

#72431 removed some oter_id's. This caused a number of errors in my save file. #72594 solves most of the errors, but one remains:

ERROR : src/generic_factory.h:513 [int_id generic_factory::convert(const string_id&, const int_id&, bool) const [with T = oter_t]] invalid overmap terrain id "dirt_road_3way_east"

I have managed to narrow this down and create a way to reproduce this situation

Attach save file

Vaughns Mill-trimmed.tar.gz

Steps to reproduce

Steps 1 trough 5 can be skipped by loading the above save

1. Install this mod and create a world that uses it:

   [
     {
       "type": "MOD_INFO",
       "id": "predecessor_mapgen",
       "name": "predecessor_mapgen",
       "authors": [ "predecessor_mapgen" ],
       "description": "predecessor_mapgen",
       "category": "content",
       "dependencies": [ "dda" ]
     },
     {
       "type": "overmap_terrain",
       "id": "o_base",
       "name": "o_base",
       "sym": "B",
       "color": "white",
       "see_cost": 1
     },
     {
       "type": "mapgen",
       "method": "json",
       "om_terrain": "o_base",
       "object": { "fill_ter": "t_region_groundcover" }
     },
     {
       "type": "overmap_terrain",
       "id": "o_override",
       "name": "o_override",
       "sym": "O",
       "color": "red",
       "see_cost": 1,
       "flags": [ "REQUIRES_PREDECESSOR" ]
     },
     {
       "type": "mapgen",
       "method": "json",
       "om_terrain": "o_override",
       "object": { "fallback_predecessor_mapgen": "field" }
     }
   ]

2. Use debug menu -> [m]ap -> [O]vermap editor -> Place overmap [t]errain to place two copies of o_base. Let's call them test and control
3. Use the same menu to replace test with o_override
4. Now you can use the overmap editor to check that test has oter_type: o_override, predecessors: o_base_north:
   
5. Save and quit
6. Replace the mod with:

   [
     {
       "type": "MOD_INFO",
       "id": "predecessor_mapgen",
       "name": "predecessor_mapgen",
       "authors": [ "predecessor_mapgen" ],
       "description": "predecessor_mapgen",
       "category": "content",
       "dependencies": [ "dda" ]
     },
     {
       "type": "overmap_terrain",
       "id": "o_base",
       "name": "o_base",
       "sym": "B",
       "color": "white",
       "see_cost": 1
     },
     {
       "type": "mapgen",
       "method": "json",
       "om_terrain": "o_base",
       "object": { "fill_ter": "t_region_groundcover" }
     },
     {
       "type": "overmap_terrain",
       "id": "o_override",
       "name": "o_override",
       "sym": "O",
       "color": "red",
       "see_cost": 1,
       "flags": [ "REQUIRES_PREDECESSOR" ]
     },
     {
       "type": "mapgen",
       "method": "json",
       "om_terrain": "o_override",
       "object": { "fallback_predecessor_mapgen": "field" }
     },
     {
       "type": "overmap_terrain",
       "id": "o_base_new",
       "name": "o_base_new",
       "sym": "B",
       "color": "green",
       "see_cost": 1
     },
     {
       "type": "mapgen",
       "method": "json",
       "om_terrain": "o_base_new",
       "object": { "fill_ter": "t_region_groundcover" }
     },
     {
       "type": "oter_id_migration",
       "oter_ids": {
         "o_base_north": "o_base_new_north",
         "o_base_south": "o_base_new_south",
         "o_base_east": "o_base_new_east",
         "o_base_west": "o_base_new_west"
       }
     }
   ]

7. Load
8. Use the overmap editor to check that control was correctly migrated to o_base_new, but test still has predecessors: o_base_north
9. If the definition of o_base is removed you get an error when loading the save:

   DEBUG : invalid overmap terrain id "o_base_north"

   FUNCTION : int_id generic_factory::convert(const string_id&, const int_id&, bool) const [with T = oter_t]
   FILE : src/generic_factory.h
   LINE : 513
   VERSION : cdda-experimental-2024-03-25-0528 74771e1

Expected behavior

1. No error
2. Predecessor overmap_terrain should be correctly migrated

Screenshots

No response

Versions and configuration

• OS: Linux
  • OS Version:
• Game Version: cdda-experimental-2024-03-25-0528 74771e1 [64-bit]
• Graphics Version: Tiles
• Game Language: System language []
• Mods loaded: [
  Dark Days Ahead [dda],
  Disable NPC Needs [no_npc_food],
  Portal Storms Ignore NPCs [personal_portal_storms],
  Slowdown Fungal Growth [no_fungal_growth],
  predecessor_mapgen [predecessor_mapgen]
  ]

Additional context

When reading an overmap save file "layers" applies migrations:

Cataclysm-DDA/src/savegame.cpp

Lines 418 to 455 in 7511d69

	if( jsobj.has_member( "layers" ) ) {
	std::unordered_map<tripoint_om_omt, std::string> 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" );
	}
	}
	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;
	}
	}
	}
	migrate_oter_ids( oter_id_migrations );
	}

But "predecessors" expects valid oter_id's and does not apply migrations:

Cataclysm-DDA/src/savegame.cpp

Lines 662 to 717 in 7511d69

	} else if( name == "predecessors" ) {
	std::vector<std::pair<tripoint_om_omt, std::vector<oter_id>>> flattened_predecessors;
	om_member.read( flattened_predecessors, true );
	std::vector<oter_id> 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 &current_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, std::move( om_predecessors ) );
	// Reuse allocations because it's a good habit.
	om_predecessors = std::move( serialized_predecessors );
	om_predecessors.clear();
	}
	}

ref: #52208

[Procyonae]

Huh I'm amazed I haven't broke stuff previously because of this

[hexagonrecursion]

In my original save a "monster corpse" special was placed over dirt_road_3way_east. Your migration was correct and should have worked, but due to a bug described above the predecessor was not migrated.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not bump or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.