Dubwise56 / Rimatomics

Nuclear power | Energy weapons | Nuclear weapons
41 stars 15 forks source link

Storage Pools causes the "Take to best stockpile" option in crafting menus to break 1.5 #44

Open Kronasamu opened 3 months ago

Kronasamu commented 3 months ago

Storage Pools causes the "Take to best stockpile" option in crafting menus to break, preventing you from choosing any option from the drop down menu. Removing the storage pools allows the menus to work properly.

How to Reproduce: Have RimAtomics installed on vanilla or with any mods. place storage pool anywhere on the map. Place crafting spot Add any recipe Select "details" Select "Take to best stockpile" that should throw this error:

Exception filling window for RimWorld.Dialog_BillConfig: System.ArgumentNullException: Value cannot be null. Parameter name: key at System.Collections.Generic.Dictionary2[TKey,TValue].FindEntry (TKey key) [0x00008] in <eae584ce26bc40229c1b1aa476bfa589>:0 at System.Collections.Generic.Dictionary2[TKey,TValue].ContainsKey (TKey key) [0x00000] in :0 at RimWorld.Dialog_BillConfig.FillOutputDropdownOptions (System.Collections.Generic.List1[Verse.FloatMenuOption]& opts, System.String prefix, System.Action1[T] selected) [0x000ed] in <957a20e0be784a65bc32cf449445b937>:0 at RimWorld.Dialog_BillConfig.DoWindowContents (UnityEngine.Rect inRect) [0x00875] in <957a20e0be784a65bc32cf449445b937>:0 at Verse.Window.InnerWindowOnGUI (System.Int32 x) [0x001a6] in <957a20e0be784a65bc32cf449445b937>:0 UnityEngine.StackTraceUtility:ExtractStackTrace () Verse.Log:Error (string) Verse.Window:InnerWindowOnGUI (int) UnityEngine.GUI:CallWindowDelegate (UnityEngine.GUI/WindowFunction,int,int,UnityEngine.GUISkin,int,single,single,UnityEngine.GUIStyle)

WJAtkin-2021 commented 3 months ago

Same issue here also able to replicate on my end

solidDoWant commented 3 months ago

Here's how the new(est) rimfridge fork fixed the same issue: https://github.com/just-harry/rimworld-rimfridge-now-with-shelves/commit/5b5ee14f998dac9c00064a2ab0d47ad88286f923

In short, the GroupingLabel property returns a null value.

NullSnapshot commented 3 months ago

Unfortunately, setting GroupingLabels doesn't fix it. No other storage mods like LWM use it, so it's not an issue with that property.

solidDoWant commented 3 months ago

The exception thrown shows that there is a null argument (dict key) being passed to ContainsKey when called in RimWorld.Dialog_BillConfig.FillOutputDropdownOptions. There are two calls to ContainsKey in that function, both of which provide a group label as a key:

private void FillOutputDropdownOptions(
      ref List<FloatMenuOption> opts,
      string prefix,
      Action<ISlotGroup> selected)
{
  List<SlotGroup> listInPriorityOrder = this.bill.billStack.billGiver.Map.haulDestinationManager.AllGroupsListInPriorityOrder;
  Dialog_BillConfig.tmpGroups.ClearValueLists<string, ISlotGroup>();
  for (int index = 0; index < listInPriorityOrder.Count; ++index)
  {
    SlotGroup slotGroup = listInPriorityOrder[index];
    if (slotGroup.StorageGroup != null)
    {
      StorageGroup storageGroup = slotGroup.StorageGroup;
      // First instance
      if (!Dialog_BillConfig.tmpGroups.ContainsKey(storageGroup.GroupingLabel))
        Dialog_BillConfig.tmpGroups.Add(storageGroup.GroupingLabel, new List<ISlotGroup>());
      if (!Dialog_BillConfig.tmpGroups[storageGroup.GroupingLabel].Contains((ISlotGroup) slotGroup.StorageGroup))
        Dialog_BillConfig.tmpGroups[storageGroup.GroupingLabel].Add((ISlotGroup) slotGroup.StorageGroup);
    }
    else if (!(slotGroup.parent is Building_Storage parent) || parent is IRenameable)
    {
      // Second instance
      if (!Dialog_BillConfig.tmpGroups.ContainsKey(slotGroup.GroupingLabel))
        Dialog_BillConfig.tmpGroups.Add(slotGroup.GroupingLabel, new List<ISlotGroup>());
      Dialog_BillConfig.tmpGroups[slotGroup.GroupingLabel].Add((ISlotGroup) slotGroup);
    }
  }
  foreach (KeyValuePair<string, List<ISlotGroup>> keyValuePair in (IEnumerable<KeyValuePair<string, List<ISlotGroup>>>) Dialog_BillConfig.tmpGroups.OrderBy<KeyValuePair<string, List<ISlotGroup>>, int>((Func<KeyValuePair<string, List<ISlotGroup>>, int>) (kvp => kvp.Value.Count <= 0 ? 0 : kvp.Value[0].GroupingOrder)))
  {
    KeyValuePair<string, List<ISlotGroup>> kvp = keyValuePair;
    if (this.ShouldCollapseGroup(kvp.Key))
      opts.Add(new FloatMenuOption(kvp.Key, (Action) (() =>
      {
        List<FloatMenuOption> floatMenuOptionList = new List<FloatMenuOption>();
        this.FillSlotGroupOptions(kvp.Value, floatMenuOptionList, prefix, selected);
        Find.WindowStack.Add((Window) new FloatMenu(floatMenuOptionList));
      })));
  }
  foreach (KeyValuePair<string, List<ISlotGroup>> keyValuePair in (IEnumerable<KeyValuePair<string, List<ISlotGroup>>>) Dialog_BillConfig.tmpGroups.OrderBy<KeyValuePair<string, List<ISlotGroup>>, int>((Func<KeyValuePair<string, List<ISlotGroup>>, int>) (kvp => kvp.Value.Count <= 0 ? 0 : kvp.Value[0].GroupingOrder)))
  {
    if (!this.ShouldCollapseGroup(keyValuePair.Key))
      this.FillSlotGroupOptions(keyValuePair.Value, opts, prefix, selected);
  }
}

What am I missing here?

NullSnapshot commented 3 months ago

To preface this, I have very little experience in rimworld modding, especially when it comes to assemblies.

Most other storage things in other rimworld mods are part of the Building_Storage thingClass, whereas the storage pool has the thingClass Rimatomics.Building_storagePool. I'm assuming the thingClass tag is probably (based on semantics) used to link the c# class instance that should be instantiated to an object when it's spawned.

I can't remember exactly what the behavior was in 1.4. Did the storage pool show up before as a haul destination?

Regardless, stepping through the current behavior, it appears to correctly build and register a storage group since it's getting through the null check to see if slotGroup.StorageGroup exists in the first place, but is almost assuredly failing on the first ContainsKey since we can validate this happens with only a single storage pool. If that's the case, it's failing (I assume) because the assembly is failing to pass along a GroupingLabel in its registered storage group, regardless of if one is defined in the defs. I feel somewhat safe in saying that since I copied the exact behavior that the just-Harry did with rimFridge, including translation inject defs, and it still didn't work. I could of course also be blowing smoke since I haven't done any sort of heavy-duty modding like this in Rimworld before.

NullSnapshot commented 3 months ago

Here's someone more qualified than me talking about the changes: https://github.com/lilwhitemouse/RimWorld-LWM.DeepStorage/pull/151#issue-2250758374

Specifically demonstrates through the code that it's likely an issue in the assemblies that needs to be fixed. Also supports my theory that the c# assembly isn't currently written to expose the GroupingLabel property, leading to the null key check.

tinygrox commented 5 days ago

Here's someone more qualified than me talking about the changes: lilwhitemouse/RimWorld-LWM.DeepStorage#151 (comment)

Specifically demonstrates through the code that it's likely an issue in the assemblies that needs to be fixed. Also supports my theory that the c# assembly isn't currently written to expose the GroupingLabel property, leading to the null key check.

Yeah, so we(player) can make a HarmonyPatch to fix that, like:

        [HarmonyPostfix]
        [HarmonyPatch(typeof(Building_storagePool), nameof(Building_storagePool.GroupingLabel), MethodType.Getter)]
        public static void GroupingLabelPatch(ref string __result)
        {
            __result = "GroupPlural".Translate();
        }