Skip to content

Conversation

@Susucre
Copy link
Contributor

@Susucre Susucre commented Aug 14, 2023

So the starting point was #8363.
Investigating that, I found out that the ReplacementEffect was changing UUID right after being paid.
Once I figured out how to stabilize that (more on that later), I started different tests, and multiple of the same activation of the same Entangler could be paid with a single cost paid. That one was due to the applies of the ReplacementEffect not identifying the parent ability differently from the others ones from the same source.

I did introduce a LinkedSimpleStaticAbility to solve both the issues:
-> It does share an handshake UUID with its Effect, and the Effect can use the handshake to identify its parent ability. That handshake changes on copy of the Ability, but both copy of parent and child do share the new handshake after copy.
-> For the ReplacementEffect having non-stable UUID, I did reinforce the ability field in GainAbilityTargetEffect, the main idea being that the ability should make a copy that has new ids for both the parent Ability and the child Effect. And there should be no Id change after that.

So the LinkedSimpleStaticAbility takes full responsibility of manually calling its Effect's newId method with a weird setup: the Effect's newId is overwritten to do nothing, and the parent Ability can call manualNewId instead, which call the inherited newId of the Effect.
That way, GainAbilityTargetEffect makes UUID-fresh-but-stable LinkedSimpleStaticAbility + ReplacementEffect in its constructors, which are later used in the GainAbilityTargetEffect::apply to add the ReplacementEffect to Objects (here the targetted Permanent).

The changes made do not alter any other card (there is that one copy + newId for the ability in the main public constructor of GainAbilityTargetEffect), so it is safe to merge, but any other use of LinkedSimpleStaticAbility will probably need careful consideration on if and when the Effect's id must be reset to a fresh one.

fix #8363

Made a class for "Ability linked with an Effect", that also takes responsability of manually calling its effect's newId method.
@github-actions
Copy link

Whipgrass Entangler - (Gatherer) (Scryfall) (EDHREC)

{2}{W}
Creature — Human Cleric
1/3
{1}{W}: Until end of turn, target creature gains "This creature can't attack or block unless its controller pays {1} for each Cleric on the battlefield."

Copy link
Contributor

@xenohedron xenohedron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite interesting. Is Whipgrass Entangler really the only card affected here? Or could this root cause manifest as other bugs? If it's a broader problem, then I'd be wary of a card-specific workaround.

@Susucre
Copy link
Contributor Author

Susucre commented Aug 15, 2023

This is quite interesting. Is Whipgrass Entangler really the only card affected here? Or could this root cause manifest as other bugs? If it's a broader problem, then I'd be wary of a card-specific workaround.

I do not know for sure.
I would think any kind of ContinuousEffect gained temporarily can have similar issue. I'll attempt to find potential cards using reflection like my work on VerifyTest.

@Susucre
Copy link
Contributor Author

Susucre commented Aug 15, 2023

Alright limiting to RemplacementEffect (as those would cause similar issue than Entangler on replacing events and being attributed fresh id inbetween each replacement)
image

scryfall for matchs

I do not see anything here that would cause similar issue as the Whipgrass Entangler.

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

Successfully merging this pull request may close these issues.

Whipgrass Entangler doesn't react to cost being paid

2 participants