Improve trigger auto ordering and aura attachment (Lynde QOL fixes)

[alexander-novo]

Some QOL fixes for a card I've been playing a lot. The biggest thing is that ordering her triggers on the end step is a nightmare, because those triggers don't reference which card is coming back, and are considered "identical" for purposes of auto-trigger ordering.

I think the auto-trigger ordering system needs a little bit of an overhaul, because there are plenty of effects which differ by something other than their rules text... such as referenced objects. Right now my idea is only to look at targets, but it may make sense to look at keys/values?

Note to myself: Fix issue with clone effects like Estrid's Invocation coming back with Lynde's ability.

[alexander-novo]

This is more of a general change, but it has something that has been annoying me while playing Lynde. Now, if a triggered ability has a target on trigger (such as a related object, not a true target, since the player hasn't chosen targets yet), it considers this target when checking if two abilities are distinct. For example:

Even though these two abilities have the same text, it still asks me to order them since they have different related objects. Unfortunately, there isn't a great way to check this for keys, so I would encourage any triggered abilities to make use of target pointers rather than keys (this is better for players anyway, since they can see target pointers but not keys).

[alexander-novo]

Also added a fix for copy effects when used with Lynde. Previously, if a copy card like [[Estrid's Invocation]] came back as a copy of a curse with Lynde's ability, it would ask which player to attach to (even though it should be attached to the controller of Lynde).

[github-actions]

Estrid's Invocation - (Gatherer) (Scryfall) (EDHREC)

{2}{U}
Enchantment
You may have Estrid's Invocation enter the battlefield as a copy of an enchantment you control, except it has "At the beginning of your upkeep, you may exile this enchantment. If you do, return it to the battlefield under its owner's control."

[xenohedron]

[[Lynde]]

[github-actions]

Lynde, Cheerful Tormentor - (Gatherer) (Scryfall) (EDHREC)

{1}{U}{B}{R}
Legendary Creature — Human Warlock
2/4
Deathtouch
Whenever a Curse is put into your graveyard from the battlefield, return it to the battlefield attached to you at the beginning of the next end step.
At the beginning of your upkeep, you may attach a Curse attached to you to one of your opponents. If you do, draw two cards.

[xenohedron]

the fix to CopyPermanentEffect looks good to me

[xenohedron]

I'm not familiar with the existing trigger order logic at all, but your intent makes sense and I don't see any issues with the added code

[xenohedron]

@JayDi85 let me know if you see anything wrong here, otherwise I'll go ahead and merge this one (edit: decided to hold off due to the other things happening with trigger order)

[xenohedron]

Curious if the withData()/getData() on the TargetPointer would be at all useful here? Currently only used in Gruul Ragebeast, Amulet of Vigor, and #11310. Haven't looked in enough detail to determine if it's solving the same problem or a different type of problem.

stale

[JayDi85]

Aura attach choice looks fine.
Auto order of triggers looks fine but need some comments and null check.

[JayDi85]

BTW for the future: if auto order disabled too much due diff targets then it can be improved with same style settings as auto choices do (disable, auto-order on same ability, auto-order on same ability and targets).

[JayDi85]

decided to hold off due to the other things happening with trigger order

There are 3 steps:

• detects use case to use auto order without choose dialog (useAutoOrder, works for same triggers list only);
• detects previous user’s saved choices without choose dialog (first/last abilities);
• call of normal choose dialog.

[alexander-novo]

BTW for the future: if auto order disabled too much due diff targets then it can be improved with same style settings as auto choices do (disable, auto-order on same ability, auto-order on same ability and targets).

Or maybe it can be disabled on an ability-to-ability basis in the same menu as "always put this trigger first"?

[JayDi85]

Or maybe it can be disabled on an ability-to-ability basis in the same menu as "always put this trigger first"?

It's UX problem -- auto-order enabled for all abilities by default, so you can't catch choose dialog to disable it on first time (e.g. you will skip choose triggers all the time until another diff trigger pop up and allows to disable it by right click menu).

[alexander-novo]

Or maybe it can be disabled on an ability-to-ability basis in the same menu as "always put this trigger first"?

It's UX problem -- auto-order enabled for all abilities by default, so you can't catch choose dialog to disable it on first time (e.g. you will skip choose triggers all the time until another diff trigger pop up and allows to disable it by right click menu).

Maybe it can be opposite? Default to no auto-trigger, and let people choose what they want to auto-trigger.