A18 bugged save state for DefMaps

[Razuhl]

Add a new RecordDef, load a save where the definition did not exist yet, open the records page for a pawn. The page is not rendered since the defmap storing the records for a pawn runs into an index out of bounds error.
The same happens for WorkTypeDef and the "work" window.
Its untested but modding JoyKindDef and TrainableDef should also cause errors since DefMaps for those exist in IExposable objects.

Those out of bounds are however just one of the bugs that DefMaps are prone to. Changing the order of definitions inside an xml file moves the saved values to the wrong definition. So basically removing, adding or reordering cause the save state to fail.

The cause of the problem is that DefMaps only save values and not the corresponding keys. At the same time the assumption is made that the corresponding database(Defs) never change in order or quantity.

I fixed the issue with HarmonyPatches for my mod but I would be much easier to fix it in the main branch. Here is the code for reference.

Code Select Expand


static public class DefMapSaveStateFix
{
static public void Postfix<T,K>(DefMap<T, K> __instance) where T : Def, new() where K : new() {
if (Scribe.mode == LoadSaveMode.Saving) {
List<string> keys = new List<string>();
foreach ( T def in DefDatabase<T>.AllDefsListForReading.OrderBy(e=>e.index) ) {
keys.Add(def.defName);
}
Scribe_Collections.Look<string>(ref keys, "keys", LookMode.Undefined, new object[0]);
} else {
//grabbing the values via reflection. The llokup can be cached per load for performance.
System.Reflection.FieldInfo fi = __instance.GetType().GetField("values", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
List<K> values = (List<K>)fi.GetValue(__instance);
List<string> keys = new List<string>();
Scribe_Collections.Look<string>(ref keys, "keys", LookMode.Undefined, new object[0]);
//we only need to save and calculate the keys once per save. To keep the patch simple we save it multiple times
List<int> valueMapping = new List<int>(values.Count);
if ( keys.NullOrEmpty() ) { //old save without keys, we must assume the order is matching the database ordering
for ( int index = 0; index < values.Count; index++ ) {
valueMapping.Add(index);
}
} else {
foreach ( string defName in keys ) {
T def = DefDatabase<T>.GetNamed(defName, false);
if ( def == null ) {
//we no longer use the value specified at this index.
valueMapping.Add(-1);
} else {
valueMapping.Add(def.index);
}
}
}

List<K> oldValues = new List<K>(values.Count);
oldValues.AddRange(values);
values.Clear();
//fill the list until it has enough entries
foreach ( T def in DefDatabase<T>.AllDefsListForReading.OrderBy(e=>e.index) ) {
values.Add(default(K));
}
//assign the saved values to the right spot
for ( int index = 0; index < valueMapping.Count; index++ ) {
if ( valueMapping[index] >= 0 ) {
values[valueMapping[index]] = oldValues[index];
}
}
}
}
}

[HarmonyPatch(typeof(DefMap<RecordDef, float>))]
[HarmonyPatch("ExposeData")]
static public class DefMapSaveStateFixRecordDef
{
static void Postfix(DefMap<RecordDef, float> __instance) {
DefMapSaveStateFix.Postfix<RecordDef,float>(__instance);
}
}

[HarmonyPatch(typeof(DefMap<WorkTypeDef, int>))]
[HarmonyPatch("ExposeData")]
static public class DefMapSaveStateFixWorkTypeDef
{
static void Postfix(DefMap<WorkTypeDef, int> __instance) {
DefMapSaveStateFix.Postfix<WorkTypeDef,int>(__instance);
}
}


[ison]

Okay, thanks for reporting.

I've changed DefMap so it won't cause errors if you add more defs and then load an old savefile.

However the problem with reordering defs remains, and I'm not sure if it's worth fixing. We don't support changing mods on the fly anyway. There are many more things which could break if you load a savefile saved with different mods.