magefree / mage

Magic Another Game Engine
http://xmage.today
MIT License
1.88k stars 770 forks source link

Denry Klin puts too many counters on himself when entering the battlefield. #9779

Open Grath opened 1 year ago

Grath commented 1 year ago

It's consistent no matter which counter I choose or if I have other causes of him to enter with counters/gain them before his ability applies, but Denry's ability is somehow consistently resulting in getting 8 times as many counters as he should, and only when it applies to him.

[[Denry Klin, Editor in Chief]]

github-actions[bot] commented 1 year ago

Denry Klin, Editor in Chief - (Gatherer) (Scryfall) (EDHREC)

{2}{W}{U} Legendary Creature — Cat Advisor 2/2 Denry Klin, Editor in Chief enters the battlefield with your choice of a +1/+1, first strike, or vigilance counter on it. Whenever a nontoken creature enters the battlefield under your control, if Denry has counters on it, put the same number of each kind of counter on that creature.

Grath commented 1 year ago

Looks like something gets fucked up if the number of counters changes somewhere in the process. I played Denry first (got 8 counters incorrectly), I played a [[Wingspan Mentor]]; I stacked Denry's trigger for Wingspan Mentor first, then Wingspan Mentor. Denry gained the flying counter, then when Denry's trigger resolved Wingspan Mentor received 3 flying counters and 24 +1/+1 counters (when it should've given 1 flying and 8 +1/+1 counters based on what Denry actually had at the time. The following turn I played [[Grateful Apparition]] which correctly received 1 flying and 8 +1/+1 counters.

github-actions[bot] commented 1 year ago

Wingspan Mentor - (Gatherer) (Scryfall) (EDHREC)

{2}{U} Creature — Human Wizard 1/3 When Wingspan Mentor enters the battlefield, put a flying counter on target non-Human creature you control. {2}{U}, {T}: Put a +1/+1 counter on each creature you control with flying.

Grateful Apparition - (Gatherer) (Scryfall) (EDHREC)

{1}{W} Creature — Spirit 1/1 Flying Whenever Grateful Apparition deals combat damage to a player or planeswalker, proliferate. (Choose any number of permanents and/or players, then give each another counter of each kind already there.)

Tharkon commented 1 year ago

Line 84 in Denry's code should probbaly not include a direct reference to denryPermanent.getCounters(game).values(). Copying it to a seperate variable first and then referencing that in the for loop might fix it. My experience is mostly Javascript and not Java so I am not sure what the correct syntax is here. Maybe something like this:

84 Counters countersToAdd = denryPermanent.getCounters(game).values(); 85 for (Counter counter : countersToAdd) { 86 enteringCreature.addCounters(counter, source, game); 87 }

Grath commented 1 year ago

It's somehow being applied multiple times from a single trigger. Doing more digging but this is bizarre.

Grath commented 1 year ago

Okay, so in my current build (current master branch + some debugging outputs): While Denry Klin is in play, he appears to be causing all ETB abilities of all nontoken creatures to apply his effect when they resolve, including his own ETB ability but also ETB abilities that have nothing to do with counters.

Tharkon commented 1 year ago

Some new information, though mostly causing extra confusion.

While Denry's ability is on the stack other ETB abilities also apply Denry's ability 3 times. If you re-order the abilities so Denry resolves first, the other ETB abilities don't apply Denry's ability. This only happens during a main phase, either yours or your opponents, but it varies based on the active player. During my own turn (I was player 3), the effect applied 3 additional times to every ability, including the original (so 4 in total on that one). During player 4's turn, it applied 3 additional time right after it entered but before anyone got priority, but did not apply additional times to Denry's ability During player 1's turn, it applied twice as it entered and once to Denry's ability. And during player 2's turn, it applied once as it entered and twice to Denry's ability. In all cases it applied 3 times to any other ETB abilities as long as they resolved before Denry's ability. There was no difference between pre-combat and post-combat main phases.

In a three-player game where I was player 1: It applied 2 additional times to Denry's ability in my turn. It applied 2 additional times as it entered during player 2's turn. It applied once automatically after entering and once to Denry's ability during player 3's turn. In all cases it applied 2 additional times to each other ability as long as they resolved before Denry's ability. And again still only during main phases.

During an eight-player game it went completely haywire during my own turn causing [[Soul Warden]]'s ability to add various amounts of +1/+1 and Shield counters (I saw numbers as high as 25 +1/+1 and 5 shield counters) to creatures entering the battlefield. No permanents had or referenced shield counters in the game, neither did any commanders or cards in players hands. During other players turns everything went as expected and 7 additional counters were added for each triggered ability.

In summary: The bug happens only during a main phase. The bug applies Denry's effect to other ETB triggers but only if they resolve before Denry's ability. The bug applies Denry's effect additional times to itself or automatically after entering but before priority. The way the previous effect is divided depends on whose turn it is, if the controller is the active player, it applies entirely to Denry's own ability. The amount of times the bug happens is equal to the amount of other players.

The code looks fine, I have no suggestions that could fix any of the above. Perhaps a complete rewrite based on [[The Ozolith]], assuming that card works fine.

github-actions[bot] commented 1 year ago

Soul Warden - (Gatherer) (Scryfall) (EDHREC)

{W} Creature — Human Cleric 1/1 Whenever another creature enters the battlefield, you gain 1 life.

The Ozolith - (Gatherer) (Scryfall) (EDHREC)

{1} Legendary Artifact Whenever a creature you control leaves the battlefield, if it had counters on it, put those counters on The Ozolith. At the beginning of combat on your turn, if The Ozolith has counters on it, you may move all counters from The Ozolith onto target creature.

Grath commented 1 year ago

Worth noting: JayDi commented somewhere that this might be specifically a bug with AI players? IE the AI simulates game events, and Denry may be applying during the simulated game events (and having that actually impact the real game)

xenohedron commented 1 year ago

That sounds wild... and even worse is that seems like the most plausible explanation right now...

JayDi85 commented 1 year ago

@Grath it was about GameEvent and tokens (more details in #10445). BUT it doesn't related to current bug with counters

xenohedron commented 1 year ago

I suggest it's worth trying to add !game.isSimulation() to the code somewhere and see if that addresses the problem. (If it does, probably indicative of a deeper issue.)