ModifiedPowerAurasTBC icon indicating copy to clipboard operation
ModifiedPowerAurasTBC copied to clipboard

Thoughts on December Update

Open Arcitec opened this issue 5 years ago • 0 comments

Hi Tom, I've been reviewing the code and having some thoughts and findings... hehe.

  1. PROBLEM Tooltip warning about "Mine" needs to be added to the german translation too: https://github.com/Geigerkind/ModifiedPowerAurasTBC/blob/master/localization/deDE.lua#L155

  2. NEW BUG? The changes to "alpha" transparency progress calculation are good, since they prevent division by zero. But after the update, self.progress is never updated if duration is 0. That may cause problems? Most likely the code should be changed to if (self.duration > 0) then ... else self.progress = "fully complete"; end?

  3. COMMENT The IsMounted() API is weird in TBC. I just checked the function. It returns nil when not mounted and 1 when mounted, (as also described here in Vanilla's API). So yeah, I guess the fix is appropriate! I always get nervous when I see api() ~= nil. I may have written that as api() and true or false so that any "truthy" value will set it to true and anything like false/nil will set it to false. But it's fine... the ~= nil check works too...!

  4. PROBLEM The MPOWA.mounted flag does not update itself often enough. It's often still false for a loooong time while IsMounted() is true, which means "don't show when mounted" auras can stay for a loooong time on screen. Sadly there doesn't seem to be any ingame event you can register to react quickly to mounting. Except possibly the "buff aura gained/lost" event, since I think the mount is an aura. You can perhaps register the aura event and if target is "player", update IsMounted(). Hopefully IsMounted() reacts fast enough and provides the correct value during the aura gain/loss events. See here: http://wowwiki.wikia.com/wiki/Events/Buff, for a list of buff-related events. I'm going to guess UNIT_AURA with "if target == player" is a good way to update IsMounted() without CPU impact.

  5. COMMENT What was the cooldown bug? All those "+1 second" were removed? Just curious. Looks weird to suddenly remove them.

  6. NEW BUG? I see that the settings saving bug was fixed! And it was just a simple problem with the "if self.loaded elseif (other events that never run)" if-statement order... Doh! But now I am curious! All of a sudden, all of the code for event == "UNIT_MANA" or event == "UNIT_RAGE" or event == "UNIT_ENERGY" is now running. It wasn't running before (in older versions of MPOWA for TBC), so it caused 0% CPU before. Now it'll take up CPU. But from what I can see, that code is useless. It tracks "unit power/energy/mana" per-unit for yourself and everybody in your raid/party. But there's no feature (that I can find) in MPOWA that actually has "self/unit/target power level" as a condition. I can only make buff trackers. I can't make health/power trackers... So what is this code for? Perhaps those UNIT_MANA UNIT_RAGE UNIT_ENERGY events should just be commented out? Or a "unit mana/rage/energy" aura condition feature should be added? But most likely, just comment out that whole UNIT-block, so that none of that code runs!

<3

Arcitec avatar May 20 '19 15:05 Arcitec