Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aura module needs a rewrite #925

Open
johnnylam88 opened this issue Jul 12, 2021 · 7 comments
Open

Aura module needs a rewrite #925

johnnylam88 opened this issue Jul 12, 2021 · 7 comments

Comments

@johnnylam88
Copy link
Contributor

The Aura module has a terrible API and is much more complex than it should be for maintaining aura state. There is no easy way to query the Aura module for the current buffs and debuffs, as all queries have to go through the state machine.

I will probably work on a refactor of this module next.

@Ophidon
Copy link

Ophidon commented Jul 14, 2021

I'm sure you wouldn't forget, but just in case, please don't forget to keep the choice to track only your own/pets (and maybe just current target instead of all enemies (as a choice) too?) auras instead of the whole party/raid's in the refactoring. This (now long ago) change made a hugely significant performance gain.

@johnnylam88
Copy link
Contributor Author

I'm thinking that for the redesign, we just watch for CLEU SPELL_AURA_* events and guess at the aura information using the information from the data module unless the GUID happens to have a valid unit ID, in which case we can scan it for the true aura information. We only need true aura information for unit IDs that we care about, so we can provide a way to register with the Aura module those particular unit IDs. The list of unit IDs to be registered can be derived from the script conditions used. For example, if we use target.DebuffPresent() and other similar script conditions, we register target with the Aura module. Similarly, using BuffPresent() and other similar conditions means we register player. In theory, using a condition like DebuffPresentOnAny() could mean we scan everything that is a hostile unit ID, but that's probably not necessary, and I'll think up some heuristics so that we're not actively scanning too many unit IDs.

@Hemario
Copy link
Contributor

Hemario commented Aug 14, 2021

But what happens if we swap target in the middle of the fight?

@Ophidon
Copy link

Ophidon commented Aug 14, 2021

Aura tracking will update to the new target at the very next state refresh then anyway right? There are a few instances for some classes where you might want to track auras on tertiary enemy targets, but not that many honestly, unless you want a box for each of them telling you redo debuffs, etc.

And honestly, just nameplate tracking is better for that than Ovale, anyway. In which case I think the checkbox option to track debuffs on all targets remains a good option instead of subjecting everyone to the rather substantial performance loss that for an edge case use.

@johnnylam88
Copy link
Contributor Author

But what happens if we swap target in the middle of the fight?

The same as now. The idea is that we use CLEU to passively keep track of auras on all GUIDs, but actively do UnitAura() scanning of registered unit IDs to get precise information about auras on those unit IDs. If a unit ID it's remapped to a different GUID, it just means that the precision of the information on the new GUID is updated while the old GUID gets the base CLEU treatment.

We currently do a lot of active scanning which can be time-consuming when UNIT_AURA fires for every unit in a raid group. If you could note somehow that you don't care about active scanning on raid units and only player and target, that would be more efficient.

@Ophidon
Copy link

Ophidon commented Aug 16, 2021

If you could note somehow that you don't care about active scanning on raid units and only player and target, that would be more efficient.

There is already a configuration option that is meant for exactly that. Unless it is being ignored by more recent code? Don't forget the player pet along with player and target as critical though, pets can get important buffs to be tracked.

@johnnylam88
Copy link
Contributor Author

You want this to happen automatically, not whether you have a manual config option checked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants