AdiButtonAuras

AdiButtonAuras

404k Downloads

Some item-based user rules are creating duplicate handlers

Derangement opened this issue · 4 comments

commented

Now that User Rules are working, a minor bug that mostly just affects performance has shown up:

If we create a User Rule, and give it an item-id for what spell it affects, like "item:86569", a new duplicate instance of that rule's handler is created any time the SPELLS_CHANGED event fires, if that item appears in LibItemBuffs.
All of the superfluous duplicates go away on UI reload.

I've submitted a pull request with a suggested fix. 😸

commented

I think I know what is happening there. However I am not sure about the right way to fix it.

LibItemBuffs rules are accessible from the addon.rules table using a metatable __index, so the wipe on SPELLS_CHANGED does not remove them. However, when you create an user rule about an item, instead of creating a new rule table in addon.rules, it grabs the existing, persistent one from the item table and insert the handlers there.

Half of the solution is to remove the items metatable from addon.rules so this doesn't happen and check both "spells" and "items" rule sets. The issue would then be to merge the user-defined item rules with the LibItemBuffs ones.

commented

Yeah, I noticed the metatable shenanigans, and coded a working solution that manually removes the custom handlers that the wipe fails to catch when updating.

I'm a complete newbie to github, though, so I have no idea if you can see the branch I made with that fix, since your updates to the master repo mean there's merge conflicts (that are probably trivial to solve).
The fix to the item-handlers is several lines of code in multiple files, so trying to write the solution here in comments is probably silly. :P

Any suggestions on what the best way for me to share that code is?

commented

I have seen your pull request, and the code. I am just not sure if it is the best way to fix it.

commented

Oh, gotcha!

It's definitely not as elegant as keeping library item rules in a separate table, and consulting both where we currently iterate through just one, since it requires additional time for clearing junk handlers.
My quick-and-dirty fix is operating on the assumption that there aren't a ton of handlers to iterate through, for each individual item-ID, and that there won't be a lot of user rules with item-IDs either.

I assume the reason items are currently storing themselves in the metadata is to avoid wasting time in recalculating their rules, since as items they're not really affected by spellbook changes.