Fix default values deep merge
vendethiel opened this issue ยท 9 comments
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...
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.
You could probably do something like setting the removed auras to something like false instead of nil.
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.
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...
aah, that makes everything more annoying though.. Since now everytime I use aurasFilterAuras / classIconAuras I'll have to check the value associated :(
Okay, I switched code to use it. It doesn't feel pretty. But hey, it works!
Thanks for quick help as usual @slaren!
to be checked:
- alerts
- announcements
- auras
- castbar
- classicon
- clicks
- cooldowns
- drtracker
- healthbar
- highlight
- interrupts
- petbar
- powerbar
- skill history
- tags
- targetbar
- unitbar
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