Stanzilla / WoWUIBugs

World of Warcraft UI Bug Tracker
153 stars 7 forks source link

Add support for custom macro conditions #554

Open Meorawr opened 2 months ago

Meorawr commented 2 months ago

With the removal of the "macrotext" attribute (#552), one notable casualty are use cases where the macrotext attribute is used to execute macros that are configured out-of-combat based upon extended conditions not available to standard macros.

Addons may preprocess macros to transform custom conditions like [class:warrior] into boolean conditions that would be satisfied at execution time. These conditions may not necessarily be tied to game state, and can be entirely addon-defined.

There's a long list of candidate conditions that would be quite nice to have in the game baseline, however a more extensible approach that would be really useful is the ability to define custom conditions.

The benefits we get from custom conditions include:

Proposal

We propose adding a new API that allows addons to assign state values to custom conditions. This would be restricted to only ever working out-of-combat, potentially even if called from secure code.

The intent is that the values assigned to custom conditions remain entirely static while in combat, but that evaluation of a custom condition continues to work while in combat. This means it should be possible to condition a /cast on custom state, but that state will be fully immutable while the combat lockdown is in effect.

local value = C_Macro.GetCustomConditionState("_name")
C_Macro.SetCustomConditionState("_name"[, value])

Boolean conditions

Boolean conditions are similar to the [flying] style conditions we have today. The expectation is that evaluation of a [_custom] condition would check if the assigned state value is any truthy value. Given the following macro, we'd expect evaluation to result as follows:

/equip [_dancing] Lamia's Tiara; Ribbon
C_Macro.SetCustomConditionState("_dancing", true)  --> Results in Lamia's Tiara being equipped when the macro is used
C_Macro.SetCustomConditionState("_dancing", false) --> Results in Ribbon being equipped when the macro is used
C_Macro.SetCustomConditionState("_dancing", nil)   --> Results in the "Unknown macro option" message when the macro is used
-- Similarly, if "_dancing" is never set it should behave as-if it had been explicitly assigned nil.

Constrained conditions

Constrained conditions are similar to existing [known:spellid] conditions, with the expectation that evaluation should check that the specified value exactly matches the value that has been assigned via the API.

/equipset [_instancetype:battleground] Smash!; ★ Style! ★
C_Macro.SetCustomConditionState("_instancetype", "battleground")--> Results in the "Smash!" set being equipped when the macro is used.
C_Macro.SetCustomConditionState("_instancetype", "world") --> Results in the "★ Style! ★" set being equipped when the macro is used

As with boolean conditions, explicit assignment of a nil value or the case where a condition is never assigned a value to begin with should be expected to raise the "Unknown macro option" system message when the macro is evaluated. Additionally, the behavior of [_custom:value] if the API-assigned state value is a boolean is left undefined.

Future expansion

As a stretch goal, it'd also be nice to support [_custom:option1/option2/optionx] style conditions that have similar "matches any of" semantics that conditions like [button:1/2] provide today. This could take the form of accepting a table of multiple candidate values for matching. If this were added, it would also be desirable to implement [_custom:value] as also being considered satisfied if "value" is present in the supplied table.

This isn't necessarily part of the initial proposal as it adds complexity for a potentially niche case, but if support could be trivially added then it would be desirable.

mbattersby commented 2 months ago

I've been pondering this for a couple of days now. For what it's worth, I think this is heading in the wrong direction. Using the macro system as (another) workaround for allowing secure actions in combat lockdown is making a complicated setup even more complicated.

The way this is proposed, non-dynamically, either macros would behave differently in combat or addons would have to spend an infeasible amount of time mirroring the game state into the custom macro settings in case a macro will be run.

I'm not sure if macros are evaluated server-side or client-side, but if it is server side there's a whole can of worms with c-s state delay. I can't even think of any useful way of triggering an addon to do a dynamic update (out of combat) that would work in-line when a macro is evaluated.

What problem would this solve? I don't think there's a use case for user-written macros having addon-created conditions, and once addons are dynamically updating the fixed macros (assuming that's permitted in 11.0) they don't need this. This doesn't look like a solution for #552 to me.

Meorawr commented 2 months ago

The way this is proposed, non-dynamically, either macros would behave differently in combat or addons would have to spend an infeasible amount of time mirroring the game state into the custom macro settings in case a macro will be run.

I'm not too sure what you mean here. Custom conditions would evaluate to the same result both in and out of combat.

You'd only be restricted from changing the state of any conditions while in combat, which broadly matches the existing restrictions that we have today with respect to frame attributes being locked down in combat.

The only differences are that:

I'm not sure if macros are evaluated server-side or client-side, but if it is server side there's a whole can of worms with c-s state delay. I can't even think of any useful way of triggering an addon to do a dynamic update (out of combat) that would work in-line when a macro is evaluated.

Macro execution is client side - at a basic level it quite literally just feeds each line into the chat frame infrastructure.

What problem would this solve? I don't think there's a use case for user-written macros having addon-created conditions, and once addons are dynamically updating the fixed macros (assuming that's permitted in 11.0) they don't need this. This doesn't look like a solution for #552 to me.

This isn't intended to be a silver bullet fix to #552 that covers every use case. Any perceived limitations regarding in-combat flexibility are fully intentional.

Rather, this requesting a very specific subset of existing functionality be restored in a way that's more consistently available for users and can be more sanely reasoned about by Blizzard when it comes to the in-combat lockdown by removing the RE from the equation entirely. That we can also define conditions based off non-game state - such as the example in the original ticket - is also a plus, as such cases aren't feasible for Blizzard to ever support to begin with.

I don't think addons all competing to update static macros is particularly pleasant, and I fully expect that such APIs are going to be locked down if the collective answer to the removal of macrotext is to start using up a user's macro slots and dynamically replacing them.