oUF

99.9k Downloads

Pet frame isn't always properly updated when entering a vehicle.

Adirelle opened this issue ยท 8 comments

commented

There is still a small issue with the removal of the vehicle element:

In the old version, the code of "updateActiveUnit" would be called "OnShow" by UpdateAllElements so the frame units would always be valid. Right now, this isn't the case anymore since it isn't an element. Moreover, it seems the "pet" frame is hidden when the UNIT_*_VEHICLE events are fired so it isn't properly updated to display the player instead of the pet (as oUF doesn't dispatch events to hidden frames).

One solution is to dispatch the UNIT_*_VEHICLE events to hidden frames anyway ; another one would be to manage to call updateActiveUnits when the frame is shown.

commented

Should be solved with 2cc9341.

I've also made some improvements to the aura element since we had this discussion.

commented

I'm planning on including some minor guid tracking in oUF. I wanted to delay this until API-6 however, since I have to use OnHide (which I currently don't).

The main reason for this is to remove quite a lot of the extra updating required for party and raid units. Right now we do a full update when we could survive just fine by updating a selected few.

I could push this into 1.5.x and flag the pet frame for updating when it gets hidden. It would be a pretty clean fix for this issue imo.

commented

About the pet frame issue, you may want to take a look at this : Adirelle@3de3477 (beware: you might not be able to chery-pick this commit as my oUF version is a bit different from yours ; I could make a "clean" branch if you wanted too.)

About the GUID tracking, I'm not sure about what you mean. If you wanted to track all frames associated with a given GUID, I'm not sure it would very useful because the events are fired for every units anyway. When UnitGUID("player") == UnitGUID("target"), most events are fired for both unitids. If you were willing to dispatch them by GUID instead of unitids, how would you avoid to dispatch both events to both frames ?

I've tried to add unitid-to-frame tracking to dispatch some events only to the relevant frames (so every 25 UNIT_AURA aren't dispatched to every 25 frames). If you interested in this, you could take a look there : https://github.com/Adirelle/oUF/blob/801c92c1b92cf4700b6c53f8f1157dc7a4d17034/events.lua

commented

guid tracking would only be used in addition to what we have now, and the main idea behind it is to track when we need to execute a full update and not.

I'm not sure if unitid-to-frame tracking would be very beneficial. Are you seeing a major gain by using a central dispatcher compared to one per frame? It seems odd that handling dispatching on the Lua-side should be faster than the C-side.

And as you said, I can't cherry-pick from your branch. It would be nice if you used feature branches if you want to contribute changes back to oUF, especially as we both seem to be focusing on slightly overlapping issues atm. I'll manually pull in some of the changes made in 3de3477cbcaa006153aa for now tho'.

commented

I found a pretty annoying corner case. It's possible to exit a vehicle in such a manner that modUnit == realUnit == self:GetAttribute'unit' ~= unit.

I'm able to reproduce it if I:

  1. Have my pet out.
  2. Enter a vehicle
  3. Leave the vehicle just as the pet despawns

It takes me around 2-10 tries hit this scenario tho'. The thing that happens is that the pet unit doesn't disappear like it usually do when you enter a vehicle.

I'm starting to believe that we should start using UnitHasVehicleUI() instead, and just simplify the entire unit swap even more, and use realUnit to track if we actually are in a vehicle or not.

commented

Okey, I've had enough for now. I reverted oUF to 8c07046 and we still had this issue back then.

The only pure[1] way to solve this is actually using guid tracking to control full updates. The idea is that: Run a full update if the guid has changed from the frames previous value.

[1] We could separate vehicle handling for each type. One way to handle player/pet and another to handle party/raid. It would just work and several headaches would go away. The downside is that i won't solve the extensive update issue, which guid tracking really was intended to solve.

Moving oUF over to guid tracking shouldn't be too much work either. I'll see what I'll get done tomorrow.

commented

About unitid-to-frame tracking: it is pretty experimental. I did some profiling on my addons and ouf_adirelle was eating a good bunch of CPU time so I looked into ways to reduce the CPU usage. And this seems to help a bit (along with some other optimizations).

Imagine an encounter where the boss periodically applies a debuff to the whole raid (as they often do). Letting pets alone, this is like 25 UNIT_AURA events to dispatch to 25 frames. If only one UNIT_AURA handler is registered, with the basic event dispatching, this is 625 handler calls, out of which 600 are discarded. If there are several handlers on each frame, you have to factor in the scanning overhead to this.

commented

By the way, I remembed that Tuller found out that for OmniCC using a centralized, throttling OnUpdate script for all frames gives better results than a separate handler for each frame.

I've tried to bucket some events, like UNIT_AURA, but Actually they are rarely fired twice in a row for the same unitid, so this wasn't as effective as I first thought and I removed the piece of code.