Creatures sacrificed don't trigger their own death triggers

[alexander-novo]

[[Industrial Advancement]]. I've noticed it several times. Here's an example of a log:

I sacrificed my [[Junji, the Midnight Sky]] and its death trigger did not trigger. Here's a pic of the board state:

[github-actions[bot]]

Industrial Advancement - (Gatherer) (Scryfall) (EDHREC)

{3}{R} Enchantment At the beginning of your end step, you may sacrifice a creature. If you do, look at the top X cards of your library, where X is that creature's mana value. You may put a creature card from among them onto the battlefield. Put the rest on the bottom of your library in a random order.

Junji, the Midnight Sky - (Gatherer) (Scryfall) (EDHREC)

{3}{B}{B} Legendary Creature — Dragon Spirit 5/5 Flying, menace When Junji, the Midnight Sky dies, choose one — • Each opponent discards two cards and loses 2 life. • Put target non-Dragon creature card from a graveyard onto the battlefield under your control. You lose 2 life.

[alexander-novo]

Another example:

Kogla had undying and no counters, but undying didn't trigger,

[alexander-novo]

Actually this might just be a general problem with sacrifice. Here's another log:

Here, I sacrifice Atsushi with [[Vivien on the Hunt]]'s +2 ability, and it doesn't activate Atsushi's death trigger.

[github-actions[bot]]

Vivien on the Hunt - (Gatherer) (Scryfall) (EDHREC)

{4}{G}{G} Legendary Planeswalker — Vivien 4 +2: You may sacrifice a creature. If you do, search your library for a creature card with mana value equal to 1 plus the sacrificed creature's mana value, put it onto the battlefield, then shuffle. +1: Mill five cards, then put any number of creature cards milled this way into your hand. −1: Create a 4/4 green Rhino Warrior creature token.

[alexander-novo]

Sacrificing with [[Evolutionary Leap]] seems to work fine, though...

[github-actions[bot]]

Evolutionary Leap - (Gatherer) (Scryfall) (EDHREC)

{1}{G} Enchantment {G}, Sacrifice a creature: Reveal cards from the top of your library until you reveal a creature card. Put that card into your hand and the rest on the bottom of your library in a random order.

[alexander-novo]

It seems it's only missing the triggers on the sacrificed permanent - in this example, The Skullspore Nexus triggers from the sacrifice, but not Protean Hulk itself.

[JayDi85]

1. Is it regression from last updates?
2. Is it affect cards with multiple modes only?

[alexander-novo]

1. Is it regression from last updates?

2. Is it affect cards with multiple modes only?

It must be a regression... there's no way this has been broken for very long. Also it's not multiple modes - cards like [[Protean Hulk]] don't work and also cards with undying. I'm planning on writing a unit test for it and doing a git bisect on it.

[github-actions[bot]]

Protean Hulk - (Gatherer) (Scryfall) (EDHREC)

{5}{G}{G} Creature — Beast 6/6 When Protean Hulk dies, search your library for any number of creature cards with total mana value 6 or less, put them onto the battlefield, then shuffle.

[alexander-novo]

Actually this may not be a regression...

I've tested on 229e8d3 and it also had the bug. Let me go back further

[sebastiandine]

I am also having this issue when sacrificing [[Su-Chi]] with [[Sage of Lat-Nam]]. I do not get the mana from Su-Chi in the pool.

[github-actions[bot]]

Su-Chi - (Gatherer) (Scryfall) (EDHREC)

{4} Artifact Creature — Construct 4/4 When Su-Chi dies, add {C}{C}{C}{C}.

Sage of Lat-Nam - (Gatherer) (Scryfall) (EDHREC)

{1}{U} Creature — Human Artificer 1/2 {T}, Sacrifice an artifact: Draw a card.

[Susucre]

Couldn't reproduce the bug with the test suite (added two tests in 907ac5c3, each one not failing on 10k attempts)

It is either:

• Related to a part of the code (GUI, AI), that the test suite doesn't use.
• Has some condition to happen. (a card/effect, or maybe enough triggers overall for a weird race condition bug)

[JayDi85]

@Susucre can be related to zcc too (same tests, but make blinks first);

[Susucre]

@Susucre can be related to zcc too (same tests, but make blinks first);

Was worth a try, but not that either: 02f19d9

[xenohedron]

Can also try in unit test, add to hand and cast, rather than add to battlefield

[Susucre]

Can also try in unit test, add to hand and cast, rather than add to battlefield

No replication either (4d076058bc7). I really think there is something outside of the pair DiesTriggeredAbility/SacrificeTargetCost that we are missing there.

[xenohedron]

I speculate it is related to isInUsableZone and short living lki

[JayDi85]

I speculate it is related to isInUsableZone and short living lki

Then put on stack some spells like bolt and exec same test commands after it (without full stack resolve).

[Susucre]

No luck either. I'm not around to continue investigating until next week at the earliest. One of you can try to find the cause from there. I don't have any clue, but feel like it is outside the Test Suite capabilities to replicate (unless we are missing a third card messing up the trigger somehow.).

[JayDi85]

Added reproducible test by e3ae00f69695fcafcf0e0ee132273072b6eb04a9. Real reason was in short LKI and move to battlefield code:

Nothing to do with it until move or dies logic rework. Maybe I'll look at it again later.

P.S. call processAction in sacrifice code is not a real fix :)

[JayDi85]

As idea to try:

1 - It's very hard to move ApplyEffects/ProcessActions code from move to battlefield to effects/cards like other multi steps effect does (move to battlefield is a most used command and require changes in 100500 files);

2 - It's require less effort but still hard to add process action to each sacrifice and other related effects (dies trigger depends on any moves to graveyard, not sacrifice only);

3 - Most perspective: replace short living LKI logic by zone change counter logic (ZCC) -- shortLKI used in dies triggers only, so it's can require much less changes and checks. Only one global usage can be a problem and need to research: Well, it's has a more weird usages:

[alexander-novo]

Great work finding the root cause

[xenohedron]

I agree that the best solution is to remove the short living LKI logic and replace with standard ZCC logic. "Short Living LKI" has always sounded like a hack/workaround to me.