Vanilla-Expanded / VanillaSkillsExpanded

2 stars 1 forks source link

Incompatibility with EdB Prepare Carefully #3

Open Flamez44 opened 3 weeks ago

Flamez44 commented 3 weeks ago

Linked here: https://steamcommunity.com/workshop/filedetails/discussion/2854967442/4356745682747827883/

Error below:

Error while instantiating a mod of type VSE.SkillsMod: System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.NullReferenceException: Null method for vanillaexpanded.skills [Ref AD12B732] at HarmonyLib.PatchProcessor.Patch () [0x0001d] in :0 at HarmonyLib.Harmony.Patch (System.Reflection.MethodBase original, HarmonyLib.HarmonyMethod prefix, HarmonyLib.HarmonyMethod postfix, HarmonyLib.HarmonyMethod transpiler, HarmonyLib.HarmonyMethod finalizer) [0x0002a] in :0 at VSE.PrepareCarefullyPatches.Do (HarmonyLib.Harmony harm) [0x0002d] in <01a0c5deb2f94ffeaefc22069c727ab1>:0 at VSE.ModCompat.Init () [0x000aa] in <01a0c5deb2f94ffeaefc22069c727ab1>:0 at <0x1e40fcfe6f0 + 0x00152> at (wrapper managed-to-native) System.Reflection.MonoCMethod.InternalInvoke(System.Reflection.MonoCMethod,object,object[],System.Exception&) at System.Reflection.MonoCMethod.InternalInvoke (System.Object obj, System.Object[] parameters) [0x00002] in :0 --- End of inner exception stack trace --- [Ref B296B70D] at System.Reflection.MonoCMethod.InternalInvoke (System.Object obj, System.Object[] parameters) [0x00014] in :0 at System.Reflection.MonoCMethod.DoInvoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x0007a] in :0 at System.Reflection.MonoCMethod.Invoke (System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00000] in :0 at System.RuntimeType.CreateInstanceImpl (System.Reflection.BindingFlags bindingAttr, System.Reflection.Binder binder, System.Object[] args, System.Globalization.CultureInfo culture, System.Object[] activationAttributes, System.Threading.StackCrawlMark& stackMark) [0x00213] in :0 at System.Activator.CreateInstance (System.Type type, System.Reflection.BindingFlags bindingAttr, System.Reflection.Binder binder, System.Object[] args, System.Globalization.CultureInfo culture, System.Object[] activationAttributes) [0x00095] in :0 at System.Activator.CreateInstance (System.Type type, System.Object[] args) [0x00000] in :0 at Verse.LoadedModManager.CreateModClasses () [0x00076] in <957a20e0be784a65bc32cf449445b937>:0 UnityEngine.StackTraceUtility:ExtractStackTrace () (wrapper dynamic-method) MonoMod.Utils.DynamicMethodDefinition:Verse.Log.Error_Patch1 (string) Verse.LoadedModManager:CreateModClasses () Verse.LoadedModManager:LoadAllActiveMods (bool) Verse.PlayDataLoader:DoPlayLoad () Verse.PlayDataLoader:LoadAllPlayData (bool) Verse.Root/<>c:b__6_1 () Verse.LongEventHandler:RunEventFromAnotherThread (System.Action) Verse.LongEventHandler/<>c:b__28_0 () System.Threading.ThreadHelper:ThreadStart_Context (object) System.Threading.ExecutionContext:RunInternal (System.Threading.ExecutionContext,System.Threading.ContextCallback,object,bool) System.Threading.ExecutionContext:Run (System.Threading.ExecutionContext,System.Threading.ContextCallback,object,bool) System.Threading.ExecutionContext:Run (System.Threading.ExecutionContext,System.Threading.ContextCallback,object) System.Threading.ThreadHelper:ThreadStart ()

frederik-hoeft commented 2 weeks ago

Specifically, these two methods seem to have been removed by the Prepare Carefully team in their latest 1.5 release:

https://github.com/Vanilla-Expanded/VanillaSkillsExpanded/blob/12dc4062f9a56e5dd093dbb31d671fa5575bca29/1.5/Source/VSE/ModCompat/PrepareCarefullyPatches.cs#L17-L20

Instead a new public event handler has been introduced:

public event PanelSkills.PassionButtonClickedHandler PassionButtonClicked;
...
public delegate void PassionButtonClickedHandler(SkillDef skill);

That is consumed by EdB.PrepareCarefully.ControllerTabViewPawns::UpdateSkillPassion(SkillDef skill) which looks like this:

public void UpdateSkillPassion(SkillDef skill)
{
    int button = Event.current.button;
    if (Event.current.button != 1)
        this.AdjustSkillPassion(skill, 1);
    else
        this.AdjustSkillPassion(skill, -1);
}

where you could hook into to fix your harmony patches instead. The only unfortunate thing is that this now seems to use SkillDef instances rather than SkillRecords, so it probably won't work with your caching implementation:

https://github.com/Vanilla-Expanded/VanillaSkillsExpanded/blob/12dc4062f9a56e5dd093dbb31d671fa5575bca29/1.5/Source/VSE/ModCompat/PrepareCarefullyPatches.cs#L38

So because that won't get you anywhere, I assume you would have to hook into the call site of that event instead, so in EdB.PrepareCarefully.PanelSkills::DrawPanelContent where you would have to install a harmony transpiler ~and hook specifically in here~:

// [142 19 - 142 56]
IL_0200: ldloc.s      V_8
IL_0202: ldfld        class ['Assembly-CSharp']RimWorld.SkillRecord EdB.PrepareCarefully.PanelSkills/'<>c__DisplayClass51_0'::skillRecord
// TODO: inject IL code to:
// 1. duplicate the value currently on the stack
// 2. call into a hook in your code (static method)
// ... and then continue with the default event handler invocation below.
// the original value that we duplicated should still be on the stack. Your hook should be a void
IL_0207: ldfld        class ['Assembly-CSharp']RimWorld.SkillDef ['Assembly-CSharp']RimWorld.SkillRecord::def
IL_020c: callvirt     instance void EdB.PrepareCarefully.PanelSkills/PassionButtonClickedHandler::Invoke(class ['Assembly-CSharp']RimWorld.SkillDef)

which would allow you to get the correct SkillRecord instance before the def field is dereferenced in below code:

...
if (Widgets.ButtonInvisible(rect, false))
{
    SoundDefOf.Tick_Tiny.PlayOneShotOnCamera(); // NOTE: this is the first call to that exact method on the `Tick_Tiny` property, so you could use this as an anchor point for the transpiler too!
    PanelSkills.PassionButtonClickedHandler passionButtonClicked = this.PassionButtonClicked;
    if (passionButtonClicked != null)
        passionButtonClicked(skillRecord/* INJECTION POINT HERE */.def);
}
...

EDIT: you should probably install your hook outside of the if statement lol 😅

I guess alternatively you could also search for the beginning of their foreach loop:

...
try
{
    while (enumerator.MoveNext())
    {
        PanelSkills.\u003C\u003Ec__DisplayClass51_0 cDisplayClass510 = new PanelSkills.\u003C\u003Ec__DisplayClass51_0();
        cDisplayClass510.skillRecord = enumerator.Current; // <-- could be used as an anchor point
        SkillDef def1 = cDisplayClass510.skillRecord.def;
        int num2 = cDisplayClass510.skillRecord.TotallyDisabled ? 1 : 0;
...

which could be easier to find because it's the only enumerator used in their implementation (get_Current()). You could then install a hook to keep track of the current values in a WeakReference field in your code, and just install a normal event handler on PanelSkills::PassionButtonClicked to get the notification and use your cached WeakReference'd SkillRecord value to clear your LearnRateFactorCache with.

IDK, just some thoughts. Seems like a bit of work, but then again there are quite a few people complaining in the Steam discussion 🤷

WaveshaperAT commented 3 days ago

Just some more random info related to this issue... There is a chance that this compatibility-patch against PrepCarefully failing is also the reason for the current incompatibility between VSkillsExp & Mad Skills - because that one only triggers if Prepare Carefully is present. So while we widely suspected this error in particular not having too much negative consequences, it seems to be a bit more complex than just breaking some functionality with PrepCarefully.