Refactor batched triggered abilities to expose which part of batch event made them trigger.

[Susucre]

So with [[Felix Five Boots]] in #12074, we have an issue of Felix incrementing the trigger count for batch events, where the part of the batch Felix cares about has no common part with the part the batch trigger cares about.

I suspect this might occur in a variety of situations, but for now the one I can illustrate the problem with is the following:

• PlayerA has Felix, an Elite Vanguard equipped with [[Umezawa's Jitte]].
• PlayerA attacks with its 2 creatures.
• PlayerB blocks the creature with Jitte, but the other is not blocked.
• A DAMAGED_BATCH_FOR_ALL is triggered with all creatures dealing damage. There is two events in it: Felix dealing damage to playerB, and Elite Vanguard dealing damage to the blocker (there is also the playerB's blocker damage, but irrelevant here)
• Jitte's trigger is checking the trigger -> there is the creature dealing damage in it. Jitte's trigger does trigger on the batched event.
• Felix sees the Jitte's triggering with the DAMAGED_BATCH_FOR_ALL, there is the event for Elite Vanguard's damage in it, so it is increased by Felix. This is where the problem is.

So to solve the problem, I think we need to improve TriggeredEvents to expose which part of events are relevant to them when they do trigger. For non-batched events, this is trivial, if it did trigger, then the event is what made the trigger triggers.

For Batch events, it is a on a case-by-case basis. For instance Jitte is using DealsCombatDamageEquippedTriggeredAbility:

Here the part of the batch that are relevant are quite explicit:

Permanent sourcePermanent = getSourcePermanentOrLKI(game);
if (sourcePermanent == null || sourcePermanent.getAttachedTo() == null) {
    return false;
}
((DamagedBatchAllEvent) event)
                .getEvents()
                .stream()
                .filter(DamagedEvent::isCombatDamage)
                .filter(e -> e.getAttackerId().equals(sourcePermanent.getAttachedTo()))

So I would be in favor of separating checkTrigger in two:

@Override
public boolean checkEventType(GameEvent event, Game game) {
    return event.getType() == GameEvent.EventType.DAMAGED_BATCH_FOR_ALL;
}

@Override
public Stream<GameEvent> filterEvent(GameEvent event, Game game) {
    Permanent sourcePermanent = getSourcePermanentOrLKI(game);
    if (sourcePermanent == null || sourcePermanent.getAttachedTo() == null) {
        return Stream.empty();
    }
    return ((DamagedBatchAllEvent) event)
            .getEvents()
            .stream()
            .filter(DamagedEvent::isCombatDamage)
            .filter(e -> e.getAttackerId().equals(sourcePermanent.getAttachedTo()))
}

@Override
public boolean checkTrigger(GameEvent event, Game game) {
    int amount = filterEvent(event, game)
            .mapToInt(GameEvent::getAmount)
            .sum();
    if (amount < 1) {
        return false;
    }
    this.getEffects().setValue("damage", amount);
    return true;
}

I think that would help us going forward, as we introduce more batch events and new cards with various one or more triggers are more and more frequent.

To go back on Felix, the filtering for Felix would call filterEvent of the original trigger to have the relevant part of the batch, then do its own filtering to decide if it triggers.

[github-actions]

Felix Five-Boots - (Gatherer) (Scryfall) (EDHREC)

{2}{B}{G}{U}
Legendary Creature — Ooze Rogue
5/4
Menace, ward {2}
If a creature you control dealing combat damage to a player causes a triggered ability of a permanent you control to trigger, that ability triggers an additional time.

Umezawa's Jitte - (Gatherer) (Scryfall) (EDHREC)

{2}
Legendary Artifact — Equipment
Whenever equipped creature deals combat damage, put two charge counters on Umezawa's Jitte.
Remove a charge counter from Umezawa's Jitte: Choose one —
• Equipped creature gets +2/+2 until end of turn.
• Target creature gets -1/-1 until end of turn.
• You gain 2 life.
Equip {2}

[JayDi85]

Can you make reproducible test for master with wrong triggers?

[Susucre]

I don't think the current trigger-caring cards can cause issue at the moment.
Need the Felix text, of "If a creature you control dealing combat damage to a player causes a triggered ability of a permanent you control to trigger, ", which is looking at specifically batchs.

[Susucre]

There might be, but it needs to be a combination of one of those:
https://scryfall.com/search?q=o%3A%22causes+a+triggered+ability%22
And a batch triggers that filters a batch event in an disjoint way.

I didn't find the Jitte+Felix example quickly, so maybe there are actual exemple with current cards.

[JayDi85]

Maybe it’s possible to use test with custom card. See good usage example from commit crime mechanics: https://github.com/magefree/mage/pull/11859/files#diff-ea86893179679bbafef2d79b9257e24555b8e297789eb3be67a05eadaf6bf5f0

[Susucre]

I think this should be an affected case (blinking wizard non-artifact + non-wizard artifact with [[Naban, Dean]] + [[Ingenious artillerist]]):

But Naban is bugged and not looking at batchs:

I'll try to find another with non-bugged card.

[github-actions]

Naban, Dean of Iteration - (Gatherer) (Scryfall) (EDHREC)

{1}{U}
Legendary Creature — Human Wizard
2/1
If a Wizard entering the battlefield under your control causes a triggered ability of a permanent you control to trigger, that ability triggers an additional time.

Unable to retrieve information for "Ingenious artificer"

[github-actions]

Ingenious Artillerist - (Gatherer) (Scryfall) (EDHREC)

{2}{R}
Creature — Human Artificer
3/1
Whenever one or more artifacts enter the battlefield under your control, Ingenious Artillerist deals that much damage to each opponent.

[Susucre]

I have not looked at all of them, but the move zones "If [...] causes a triggered ability of [...] to trigger," all seem to not work on batch move events.
So the refactor I propose is even more needed to rework those cards.

[xenohedron]

Interesting find. Is the bug only applicable to NUMBER_OF_TRIGGERS?

[Susucre]

Interesting find. Is the bug only applicable to NUMBER_OF_TRIGGERS?

I do think so. This should be the only meta-trigger?