kemayo / wow-silverdragon

World of Warcraft addon to find rare mobs
24 stars 11 forks source link

Profiles do not save status of expansion modules or enabled/disabled rares #155

Open KyrosKrane opened 2 years ago

KyrosKrane commented 2 years ago

Summary: The selection of enabled modules (by expansion) and enabled rares inside each expansion is not saved with the profiles from the Profiles tab.

Steps to reproduce:

1) Exit WoW if running and remove your SilverDragon settings file from retail\WTF\Account\ACCOUNTNAME\SavedVariables 2) Load into WoW. 3) Right click the SD icon on the minimap to open settings. 4) Click on Profiles in the left menu 5) In the New box, type in "Alts" and click OK. Note that above, it updates to say "Current Profile: Alts" 6) Click on Mobs in the left menu 7) Click the BattleForAzeroth tab 8) Uncheck Enable 9) Click the BurningCrusade tab 10) Uncheck Enable 11) Click the Legion tab 12) Click Adventurer of Aszuna 13) Uncheck the first two items in the list (Ravyn-Drath and Valiyaka the Stormbri...) 14) Click Close to save the settings 15) Right click the SD icon on the minimap to open settings. 16) Click on Profiles in the left menu 17) From the Existing Profiles dropdown, choose Default. 18) Click on Mobs in the left menu 19) Click the BattleForAzeroth tab 20) Note that the Enable checkbox is not checked <-- Error 21) Click the BurningCrusade tab 22) Note that the Enable checkbox is not checked <-- Error 23) Click the Legion tab 24) Click Adventurer of Aszuna 25) Note that the first two items in the list (Ravyn-Drath and Valiyaka the Stormbri...) are not checked <-- Error

KyrosKrane commented 2 years ago

I just retested this with v90200.10. Right after step 17, I got the error from #156. The errors in step 20, 22, and 25 are still there as well.

kemayo commented 2 years ago

Ah, that's historically-deliberate but without any real degree of intentionality. As in, I put (the predecessors to) that stuff in global back in 2009 in 2a570ce0b0e2ffc4393557887366c8f3b72a71e4 and can't really remember my reasoning. I think it'd be fair to move the module enable/disable and the ignore/add lists into profiles, if nothing else. I'll need to go read the acedb docs and see what a migration would entail...

KyrosKrane commented 2 years ago

Heh, I was able to retrace my steps on how this came about. A few months ago, I created a toon specifically for farming Timeless Isle to get my last few things there. I created a new SD profile for that toon, and disabled all expansions except MOP and all MOP rares except the few I still needed on Timeless Isle. I didn't realize this affected all my toons, even ones on other profiles. I'd been wondering why I got such inconsistent alerting on my other toons, in stuff like Legion or Shadowlands. I had chalked it up to buggy Blizzard data. :D

kemayo commented 2 years ago

Yeah, I think the bare minimum necessary-change would be flagging in the settings UI what stuff is global, because I certainly gave you no way of knowing that would happen. 😄

KyrosKrane commented 2 years ago

From a user's perspective (and putting aside code complexity considerations), I'm not sure it makes sense to have any settings be global, not stored in profiles. In most addons I've used that support profiles, the profile was synonymous with "this addon's configuration." For example, in Bartender, switching profiles changes not only bar positions and visibility, but also general settings like bar lock, button lock, and minimap icon.

kemayo commented 2 years ago

It grew into it as the addon's structure changed over the years -- the stuff originally stored in global was data-collection, as you encountered rares in the world it'd collect their data and remember it there, so it made sense to keep it somewhere global. Then later I started actually including a database of rares in the addon, and slotted in the bookkeeping around that into the same place as the old data-collection... even though it's now config and not just data.

The main thing that should remain there is the last-seen time for rares, since that's mostly useful for respawn-tracking.

Amethice commented 8 months ago

Reminder to please work on this if feasible. Would be super nice for things like AFK-camping the druid rares being able to select which rares to track per profile.