Rebuild Ability tables
SabreValkyrn opened this issue ยท 8 comments
Rebuild ability tables scraping TBC databases.
Similar to Gogo1951/GogoWatch#4
Pull directly from class lists to avoid mob spell IDs.
Might also yield pet abilities Gogo1951/GogoWatch#13
Pet abilities I think had been included, but there was an issue where it didn't pair pets with their owners, so it didn't send messages to the owners.
Building in some sort of "Hey, your growl is on and we're in an instance..." for locks and hunters would be nice too. (=
Shame this never made it over to Classic. https://www.curseforge.com/wow/addons/badpet
Part of the reason is perhaps that this particular niche is covered by some tank-oriented addons, like for example Simple Taunt Announce (Does warlock/hunter pets and shaman totem)
STA doesn't tell the hunter though. It just lets you, as the tank, know when someone else growls. I have it play a murlock sound. Ha. And when the hunter leaves growl on I just get spammed with "Mmmrrrggglll" over and over. Then I have to tell the hunter. But it'd be great to have something that automatically reminded the hunter.
I never want to do announcements like that in party / raid, much better as just a private tell. Last I used STA I don't think it had that output option like Bad Pet had.
From Discord DM with Gogo, https://docs.google.com/spreadsheets/d/1jtx1WyfChzACzh0WBWANtrqkRtS3D-zPWqs3eOnyVvY/edit#gid=0
Working on skimming that data, massaging, and scripting conversion logic.
Got the data itself in a good place, at least seems like a good place until I start trying to use it. PR draft at #13
Still need to rework combat parse logic.
Should I be concerned about the memory footprint of such a large table?
Not that this is a perfect comparison or representation of RAM footprint, but DataTables is 185,500 characters whereas the the current AbilityData https://github.com/valkyrnstudios/SpellSentinel/pull/13/files#diff-8ae75864741e465c0f802a487ecef578c9c8a5dee4f3f577e6f71bc1d5560609 is 189,400
"Foo (Rank N)" feels redundant and not useful, I'll remove it once I update combat log parsing to make sure it's not useful.
Converted combat parsing to new data! Rudimentary testing seems good.
I'm not sold on my use of self.db.profile.isMaxRank
or if that should be an in-session variable. Evaluating rank is a lot more processing intensive compared to GogoWatch's MaxLevel property, as it's a few dictionary lookups and comparisons so it felt like caching the results would save a non-trivial amount of processing time in a raid with the same people casting their abilities every 3 seconds.
I haven't looked at the code but static memory is not a problem (other than longer loading screens depending when that data is put in memory)
The problem is cpu usage spikes either because of doing heavy operations or - and this is where memory usage can be an issue - memory churn, lots of garbage created (for example through frequent creation of tables that are then left hanging).
This can cause the Lua garbage collector to fire more often which also causes a cpu spike and can make the game "stutter" or show FPS drops.
TL;DR: High but static memory usage is not an issue, Heavy computation (especially in combat) or memory increase spikes are.
Any time the choice is between for example compression to save memory at the cost of unpacking or packing data before they can be used, larger (but static) memory footprint is always the correct answer.
(Over)Optimized a bit more, removed unused name entirely since there's no human curation on AbilityData.lua, doesn't need to be very discoverable. All data changes should happen in the GSheet and table be regenerated from that csv.
Also changed AbilityGroup from a "Class - Ability Name" string to a number for more efficient lookups during runtime. A number takes less space than a string and an indices lookup must be faster than a dictionary key lookup. 9494a03
Not that this is a perfect comparison or representation of RAM footprint, but DataTables is 185,500 characters whereas the the [string lookup] AbilityData is 189,400
AbilityData.lua is now 99,900 characters using numerical lookup and seems as optimized as I can realistically get.