RandyKnapp / Auga

48 stars 16 forks source link

Fix Store_Setup.cs #173

Closed sbtoonz closed 1 year ago

sbtoonz commented 1 year ago

so that it doesn't kill StoreGui.Awake() for all other mods patching it

this resolves #172

sbtoonz commented 1 year ago

requesting review from current maintainer not sure if that is @RandyKnapp or @Vapok

Vapok commented 1 year ago

Can you help me with just understanding.. what does this actually fix? and so that I am aware.. how do you present the problem that this is solving?

sbtoonz commented 1 year ago

Explanation is as follows:

When you prefix patch with a boolean value it will in turn kill any subsequent patches for these objects ->Therefore when you return false on StoreGui.Awake() Prefix Bool()

It will KILL vanilla code/Patches from running after that prefix as the boolean value has returned true

In that aspect you also DestroyImmediate the vanilla gameobject in this patch. So ANY other mods that will patch StoreGUI.Awake() will throw an NRE and risk potentially crippling the current game session.

There is 2x StoreGui.Awake() methods called when the Auga prefab exists as the lifecycle of the monobehaviour is not dynamic to the owning game object.

It will have Awake() called on both objects therefore the patch will run on both objects.

I have a mod that uses a custom UIX that is instantiated on StoreGUI awake as it is a custom trader

In the current deployment of Auga

my patch

https://github.com/sbtoonz/Trader_2.0/blob/master/Trader2.0/Patches.cs#L194-L195

Fails to apply with the subsequent error as such:


[Error  : Unity Log] NullReferenceException
Stack trace:
Trader20.Patches+HudPatch.Postfix (StoreGui __instance) (at <2b046790a38c4d29a10b6612c8d75c13>:0)
(wrapper dynamic-method) StoreGui.DMD<StoreGui::Awake>(StoreGui)
UnityEngine.Object:Instantiate(GameObject, Transform)
InstantiatePrefab:Awake()

This is because it tries to apply to BOTH vanilla storeGUI.awake and AugaStore's mono behaviour StoreGui.Awake()

and my patch attempts to just instantiate my GUI object into the parent transform for that object

The object is gone in 1x trigger of patch (remember there is 2x StoreGui holding GameObjects so vanilla trigger == NRE, AugaPrefab == no NRE)

In looking at this again today in a second light

My patches will ALWAYS throw an NRE because they will try to apply to both AugaStore and the games Store prefab Auga destroys the games prefab therefore crippling that secondary patch and causing null refs

I suppose I could just add the GO name filtering to my patch but thought it would be more graceful to just leave it intact and redirect the StoreGui.Instance -> AugaStore.GetComponent<StoreGui>().m_instance;

I am sure there are more ways to approach this problem this just seemed the most straightforward when I looked at the code yesterday

The alternative way would be writing transpiler for the trader OnInteract method and just pop out the AugaStore if better trader is not installed

Since from what I can see this is the only place StoreGUI.Instance() is used

All other refs to StoreGUI vars happen within the StoreGUI class itself

Vapok commented 1 year ago

I've migrated these changes into my branch for the next release. Thanks for the help @sbtoonz!