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

Refactor needed for "second main phase" #12788

Open
theelk801 opened this issue Sep 2, 2024 · 5 comments · May be fixed by #12970
Open

Refactor needed for "second main phase" #12788

theelk801 opened this issue Sep 2, 2024 · 5 comments · May be fixed by #12970
Assignees
Labels
refactoring Developers topics about code and programming

Comments

@theelk801
Copy link
Contributor

The phrases "precombat main phase" and "postcombat main phase" have been replaced with "first main phase" and "second main phase" respectively. This is a functional change in the latter case, as additional main phases won't trigger those abilities. However, this is further wrinkled by the fact that there is a partial reverting of this change that hasn't applied yet according to Matt Tabak. So some older cards will still say "postcombat main phase" but their oracle text hasn't been reverted yet and won't until Duskmourn releases.

@xenohedron xenohedron added the refactoring Developers topics about code and programming label Sep 2, 2024
@JayDi85
Copy link
Member

JayDi85 commented Sep 17, 2024

DSK release notes published, so main phase changes can be fully researched now:
https://magic.wizards.com/en/news/feature/duskmourn-house-of-horror-release-notes

@JayDi85
Copy link
Member

JayDi85 commented Sep 17, 2024

Related article about phase changes and card examples: https://mtgrocks.com/postcombat-main-phase-change/

  • Cards like [[Aggravated Assault]] must add additional main phases, not first/second (already supported). There are 8 cards with additional main phase effects.

  • Make sure cards like [[Chancellor of the Tangle]] trigger one time per game. Old rules text “first main phase” replaced with “first main phase of the game”. There is only one card with such texts and looks like it must be fixed.

  • Refactor all triggers with EventType.POSTCOMBAT_MAIN_PHASE* and EventType.PRECOMBAT_MAIN_PHASE* to check “not extra”.

I don’t think it requires any code renaming from precombat to first and post combat to second. Official rules still use it, see “505. Main Phase” (from blb update). Maybe will be changed in later rules release (with dsk).

Copy link

Aggravated Assault - (Gatherer) (Scryfall) (EDHREC)

{2}{R}
Enchantment
{3}{R}{R}: Untap all creatures you control. After this main phase, there is an additional combat phase followed by an additional main phase. Activate only as a sorcery.

Chancellor of the Tangle - (Gatherer) (Scryfall) (EDHREC)

{4}{G}{G}{G}
Creature — Phyrexian Beast
6/7
You may reveal this card from your opening hand. If you do, at the beginning of your first main phase of the game, add {G}.
Vigilance, reach

@Grath
Copy link
Contributor

Grath commented Sep 17, 2024

  • Refactor all triggers with EventType.POSTCOMBAT_MAIN_PHASE* and EventType.PRECOMBAT_MAIN_PHASE* to check “not extra”.

Worth noting: Not everything is being changed away from "postcombat main phase", but they still haven't reverted the errata yet on Neheb to make it still function during extra main phases:
https://x.com/WotC_Matt/status/1816524550433329385

So we still need to wait for them to finish up the Duskmourn updates to find out exactly which 11 cards, besides Neheb the Eternal, Brazen Cannonade, and Megatron, Tyrant, are still getting to work during extra postcombat main phases.

@theelk801
Copy link
Contributor Author

Update: we now have a list of all the cards that will still say "postcombat main phase"

@theelk801 theelk801 linked a pull request Oct 3, 2024 that will close this issue
@theelk801 theelk801 self-assigned this Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Developers topics about code and programming
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants