GladiusEx

GladiusEx

4M Downloads

Fix default values deep merge

vendethiel opened this issue ยท 9 comments

commented

There's a very very bad bug I just noticed.

Ace3/AceDB does a deep merge on the default values.
So if I added "Riptide" to my auras module, Ace3 will merge GetDefaultAuras() with it.

And thus all the default auras + my auras will show.
Even though the values aren't actually in SavedVariables.
Deleting them is no use, after the next refresh, it will deep merge the default auras again.

This basically means there's no way to set default auras, because people who don't want them in (they want whitelist instead of blacklist, ...) are just stuck with them being re-added every single time.


A big thing is -- they're not getting persisted! So if you remove all the default from classicons, the list will be empty... Because they are only the default auras and are not saved...

commented

I have absolutely no clue how I'm going to fix this...

A boolean flag to indicate that "stuff was changed"? But that doesn't fix the values not being persisted...

Set all the default values in GladiusEx:CheckFirstRun()? But that's improper and can be damaging...

Maybe I should detect that aurasFilterAuras / classIconAuras are nil instead (not empty), when setting up options, and setting them there? But that means everyone who currently has the addon will have to reset their classicon module to get the classicon-auras to show up...

Unless I change the variable's name (so the DB is empty)? Or make it impossible to have an empty classicon-auras list for a while..?

This is hairy.

commented

You could probably do something like setting the removed auras to something like false instead of nil.

commented

Do you think that would help with the deep merge? I'm not sure...

commented

If you just set an aura to nil, it is entirely removed from the list, and AceDB can't tell the difference from the default value. By setting to false, it would still exist on the list and AceDB wouldn't apply the default.

That's if I understand the problem correctly anyway, I may be missing something.

commented

I guess that solves the issue. It works a bit around it: all the default values are still not saved. They are only set to false when deleted, so I guess technically... It should work...

commented

aah, that makes everything more annoying though.. Since now everytime I use aurasFilterAuras / classIconAuras I'll have to check the value associated :(

commented

Okay, I switched code to use it. It doesn't feel pretty. But hey, it works!

Thanks for quick help as usual @slaren!

commented

to be checked:

  • alerts
  • announcements
  • auras
  • castbar
  • classicon
  • clicks
  • cooldowns
  • drtracker
  • healthbar
  • highlight
  • interrupts
  • petbar
  • powerbar
  • skill history
  • tags
  • targetbar
  • unitbar
commented

Gladius is a bit smarter with it: It only marks a value as false if it's in the default. It probably helps with memory usage if you keep adding and removing different auras...
https://github.com/Resike/Gladius/blob/master/Gladius/Modules/Classicon.lua#L1025-L1029