Fix Brutal Cathar trigger timing and add regression canaries(#9484)#9669
Fix Brutal Cathar trigger timing and add regression canaries(#9484)#9669Kiousu wants to merge 5 commits intoCard-Forge:masterfrom
Conversation
| game.getPhaseHandler().devModeSet(PhaseType.MAIN1, catharController); | ||
| SpellAbility catharSa = brutalCathar.getFirstSpellAbility(); | ||
| AssertJUnit.assertNotNull(catharSa); | ||
| catharSa.setTargetCard(specialist); |
There was a problem hiding this comment.
hmn thanks, but you picked a pretty difficult problem for your first PR...
anyway I thought I already fixed the trigger handling (and only left the issue open for the replacement effect part):
while I see this test fails without the engine change I suspect that's only because you're not providing the LastStateBattlefield correctly when destroying 🤷♂️
also here a target is set for the permanent spell which makes no sense (it's possible the AI targets the trigger by itself later though)
this kind of leads me to believe this is generated by an AI once again fooling itself? 👎
There was a problem hiding this comment.
Thanks for the feedback, absolutely my mistake.
I think that the tests should be valid now. At the very least, I think I've fixed the two issues that you've pointed out. Dropping the guard I added causes the following tests to fail:
testBanisherPriestReturnDoesNotSeePastOpponentDeath
testBrutalCatharDoesNotTriggerReturnedSlaughterSpecialistFromItsDeath
testBrutalCatharReturnDoesNotSeeSimultaneousOtherCreatureDeaths
If I’m still missing something and the tests are still wrong, that’s on me.
There was a problem hiding this comment.
I had the most terrible thought. What if I misread/misremembered the card?
In a case of tragic serendipity, the test was right for the wrong reason.
There was a problem hiding this comment.
I'm trying to recreate testBrutalCatharReturnDoesNotSeeSimultaneousOtherCreatureDeaths ingame and it seems to work fine:
so unfortunately I still don't trust those tests 🤔
There was a problem hiding this comment.
Started with the test, got the test wrong. The issue was always solved. Sorry for wasting time.
3 participants
Fixes ChangesZone trigger timing for battlefield-origin events so newly-entered objects don’t retroactively observe older leaves/dies events (Brutal Cathar / Slaughter Specialist case).
Also adds focused regression canaries in GameSimulationTest for:
No functional changes outside trigger timing guard + tests.