Mod Destructors "modTable" is Nil
Arcitec opened this issue ยท 14 comments
Game and Plater version
- Game: TBC
- Plater: 336-BCC
Describe the bug
Plater doesn't pass modTable
to mod destructors. It passes nil instead. All other values work properly (envTable, unitFrame, etc).
The Plater template for Destructors literally includes modTable in the template function definition, so I assume this is a bug.
I've tested all of this with a totally blank, "New Mod", to verify that it indeed always sends a nil
value.
Question:
Unfortunately the lack of modTable in the destructor means that the state which I set up in the Initialization section's modTable cannot be torn down in the Destructor. I basically need to clean up certain things whenever the mod is manually (or automatically based on load conditions) disabled, because the plate mod I'm working on has to hook into and overwrite code of another addon, and then restore it when the mod is disabled. That state is all stored in the modTable.
For now, I guess I'll use a _G global table as a workaround for persistent storage. That workaround actually also solves the other issue I was having, which is that the modTable
is wiped every time the mod is disabled and re-enabled.
Is there any official way to have a "persistent" state between mod enable/disables in Plater, or any official "teardown" function (the opposite of Initialization)?
I will look into this.
It is certainly not intended to have the modTable
not available in this case.
Please test against the next alpha version.
Thank you so much for fixing this bug. I have tested it now, and can confirm that the modTable with its contents is now available in the destructor, so that I can do stuff like "call existing modtable functions to destruct the nameplates" instead of having to duplicate code manually.
I noticed 1 small major change that may be unintentional though. All the new "hot reloading" flags have changed the behavior of the Initialization scripts.
Before: Every time you change ANY script in a mod and press Save, the Initializer runs and the modtable is re-created. It also used to run on ANY option change in the Options panel for a mod, and also ran incessantly (way too much in my opinion) when someone was sliding settings-sliders in the Options.
After the Destructor modTable commit: Initializer only runs when you enable/disable the mod. The re-save/option change scenarios no longer trigger the Initialization script.
This may have been unintentional? I assume the reason the initializer used to re-run every time you saved the script, would be so that the modTable would be constantly re-created with new code while someone develops a mod.
The full docs of how Initializer used to behave in the past, was documented by me in my mod, as follows:
Runs once when the mod is first loaded, and every time when it's automatically
(or manually) activated again after an unload. It also runs every time the user
saves any changes to the mod's scripts. And finally, it runs every time the user
changes any setting in the mod's options (including running at a very high, spammy
rate whenever the user moves any slider in the options).
This is where we'll set up the global functions used by our mod.
Due to the frequent calls to this function, it's important that we do very little work
here, and defer most processing until later.
IMPORTANT NOTE: The "modTable" is wiped every time the mod is disabled and re-enabled,
so we are forced to use a global variable since we want to store some persistent values.
Also note that the "Constructors" run again, but inconsistently, whenever the mod is
merely disabled and re-enabled. See the "Constructors" code comments for more on that.
IMPORTANT NOTE: Every time the user hits "Save" in Plater to save any changes to the mod's
code, it always wipes the "modTable", re-runs the "Init" and then re-runs the "Constructors"
for ALL visible plates. So that's something we have to keep in mind when hooking/unhooking,
which is why it's very important for us to keep a global, persistent variable with important
data for restoring the originally hooked Questie state whenever our mod is unloaded.
What I can see, now Initializer only runs when the user first runs/unloads/re-loads the mods.
When saving changes to code, the CONSTRUCTORS still run for all visible plates. It's just the Initializer that no longer runs.
So yeah it does seem like the refactoring introduced an initializer bug?
modTable
will be cleared on unload/disable. I currently don't intend to change this. Not clearing and init not running was unintentional.
_G
is in a secured environment and can be used across mods.
The behavior change seems to be related to the "hot reload" idea, which apparently recompiles ALL scripts but doesn't wipe the modTable and doesn't re-run the initializer. Is that how it behaves now?
I would suggest a different approach in that case:
- Always run initializer anyway.
- But present initializer with
function (modTable, isHotReload)
. Or alternatively the user could just do it themselves via stuff likeif not modTable.MyFunction then modTable.MyFunction = abc...
, meaning the always runs but it's up to the mod author to decide if/what data they want to overwrite in the modTable. They can just check the existence of certain table keys and determine if they need to be preserved or overwritten. This is the one I'd prefer as a mod developer. And the "isHotReload" flag could be provided too, as a bonus way to truly tell the mod author that initializer is running during a hot reload.
I do in general like the idea of a persistent modTable between script reloads. It removes the need to store _G[]
global variables for persistent state.
Edit: I've verified this "hot reload" behavior now by making "nameplate added" script print "haha" and then the modTable.HideIcon
value. Then I rewrote the script to print "haha2" and saved it. And looked around.
It printed the exact same HideIcon function reference, but Haha2 now instead. Which means that scripts were recompiled but modTable was kept.
So yeah that is a major change, and I like it, it's just very confusing right now. I'd probably think that Initializer should still run, but the mod author can then take the decision to overwrite certain data or not. Otherwise it makes three things very difficult:
- Mod development becomes hard because you'd need to /reloadui every time you change the initializer code, to update the modTable with your latest code.
- Wiping certain data on re-initializtion becomes a nightmare if Initializer doesn't run.
- Deinitializer runs all the time, but Initializer only runs sometimes. So if you teardown stuff in deinitializer, you don't set it up again since the initializer doesn't run anymore.
@cont1nuity Alright, perfect, glad to hear it :) And glad I helped testing! I've edited my comments here quite a bit, so please check if you saw the latest info/ideas for how to solve this.
Oh, hmm, nah, I write to
_G
and can retrieve it via/dump <var here>
so it doesn't seem like it's a self-contained environment.
This is intended, it works with the actual _G
and is access-limited.
Thank you for testing and the reports.
@cont1nuity Alright, thanks a lot for these great improvements! Really love how much these new changes will clean up my mod.
One last question. What is the access limit of _G
?
modTable
will be cleared on unload/disable. I currently don't intend to change this. Not clearing and init not running was unintentional.
Ah, okay! Yeah the old behavior was fine.
I am downloading your latest commit now to test the change. Is that the entire fix or are more commits needed?
_G
is in a secured environment and can be used across mods.
Oh, hmm, nah, I write to _G
and can retrieve it via /dump <var here>
so it doesn't seem like it's a self-contained environment. Which is kinda surprising since I saw that you compile scripts with a custom global environment. I guess it has a metatable that retrieves data from the real globals, and retrieves the real _G
, which is what lets mods break out. I assume that if a mod merely does globalstuff = 1
it will be in the secure environment, but _G['globalstuff'] = 1
breaks free from the prison via secure environment metatable lookup of the real _G from globals.
The way to fix that, if I remember correctly, is to create a fake _G
inside your secure environment. But this late in development, such a change may break mods if they relied on reading real third-party globals via _G
.
Anyway, glad to hear that setting globals to keep persistent values between reloads is a reasonable approach. I'll be keeping 1 global variable where I store the hook state (since I can't be sure modTable
will preserve state across reloads), so that I never double-hook or multi-unhook. :D
Testing the latest commit now, c6c88ee
- Saving any of the scripts: Forces init to run.
modTable
is nil. - Scrolling an options slider or changing an options value: Forces init to run.
modTable
is nil. - Enabling mod after it was disabled: Forces init to run.
modTable
is nil.
Summing it up:
- Old init behavior is back again! :)
- Deinit works perfectly.
- Destructor modtable works perfectly.
I cannot see any issues, after quite thorough testing.