Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dies triggers fixes and improves (bugs like "no trigger on sacrifice cost") #13088

Merged
merged 12 commits into from
Nov 30, 2024

Conversation

JayDi85
Copy link
Member

@JayDi85 JayDi85 commented Nov 30, 2024

The main idea behind PR's refactors and fixes:

  • simplify and share different implementations of isInUseableZone for better support "look back in time" (603.6 and leaves-the-battlefield triggers);
  • improve leaves-the-battlefield triggers support in meta triggers like OrTriggeredAbility or ConditionalInterveningIfTriggeredAbility;
  • remove custom implementations and some usages of short LKI;
  • add verify and runtime checks for wrong usage of "dies" and "put to graveyard" triggers (bug example: no die trigger on itself sacrifice or mass removers);
  • many cards fixes (in some use cases fix is not required due trigger logic but it's better to keep same code base);

Additional fixes from PR:

How-to use/override isInUseableZone:

  • for normal abilities and triggers:
    • isInUseableZone - ignore;
    • setLeavesTheBattlefieldTrigger(true) - ignore;
  • for leave battlefield triggers:
    • isInUseableZone - ignore;
    • setLeavesTheBattlefieldTrigger(true) - set in constructor;
  • for dies triggers:
    • isInUseableZone - override by TriggeredAbilityImpl.isInUseableZoneDiesTrigger usage;
    • setLeavesTheBattlefieldTrigger(true) - set in constructor;
  • for multiple zones trigger (weird use case):
    • isInUseableZone - ignore;
    • setLeavesTheBattlefieldTrigger(true) - set in constructor;
  • for batch events
    • isInUseableZone - override and call for each real event;
    • setLeavesTheBattlefieldTrigger(true) - collect from any real event;

What can be done next:

  • refactor setLeavesTheBattlefieldTrigger -- add param like Zone lookingZone and use it in shared isInUseableZone (so no needs to override isInUseableZone and use of isInUseableZoneDiesTrigger);
  • refactor and combine code from three basic implementation of isInUseableZone in one method:
    • AbilityImpl.isInUseableZone
    • TriggeredAbilityImpl.isInUseableZone;
    • TriggeredAbilityImpl.isInUseableZoneDiesTrigger;
  • refactor HauntAbility to remove custom logic from isInUseableZone;
  • search and close bugs like "no die trigger on itself sacrifice or mass removers";
  • research that can be done with multiple event triggers (one event need actual info, another event need look back in time):
  • PeltCollectorTriggeredAbility
  • JerrenCorruptedBishop
  • Dreadhound
  • DaxosBlessedByTheSun
  • CarthTheLion
  • AthreosShroudVeiled
  • SlagstoneRefinery
  • ShardOfTheVoidDragon
  • Psychomancer
  • BridgeFromBelow
  • KayasGhostform
  • HauntAbility

@JayDi85 JayDi85 merged commit 52c4675 into master Nov 30, 2024
3 checks passed
@JayDi85 JayDi85 deleted the wrong-dies-triggers-fix-2 branch November 30, 2024 13:57
JayDi85 added a commit that referenced this pull request Dec 1, 2024
JayDi85 added a commit that referenced this pull request Dec 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant