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

Refactored AttackConfirmed and blockers list initialization + admin messages QoL improvements #315

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

DragosIonita23
Copy link

@DragosIonita23 DragosIonita23 commented Feb 21, 2025

Refactored fx.AttackConfirmed usages inside fx.Creature handle of AttackPlayer and AttackCreature, so fx.WheneverThisAttacks can be removed (and all of the other helper functions that were introduced in order for WheneverThisAttacks to actually work).

Essentially, both AttackConfirmed and WheneverThisAttacks served the same purpose, to handle attacking effects that happened right after the attack was confirmed (i.e. player chooses what shield / what creature to attack and commits the attack, i.e. action is NOT cancellable anymore) as per DM rules.

Also, introduced a new "step" event, BlockerSelectionStep, that serves as a helper step right before the opponent is prompted to select a blocker before the battle / shield brake to happen. This new "step" event is used in order to modify "in-place" the blockers list from the parent event, i.e. AttackPlayer or AttackCreature. Also, this new event is also used to handle conditional CantBeBlocked effects more easily.

Finally, I improved the admin messages handler with some QoL additions for faster debugging and initialization of the cards in the playzones. They reduce the number of commands one has to write in order to actually create a "playable" match context, i.e. populate different zones (hand, mana, shields, deck) very easily with a lot of different cards.

P.S. It is my first PR here, so please feel free to comment anything you think, I am eager to learn and to accept every kind of comment.

P.S. 2: Here is a non-exhaustive list of cards that I thought would be affected by the changes. I tested them (not very much though, please feel free to test yourself by cloning this branch from my public repo) and for me they seemed fine.

TOEL VIZIER OF HOPE
AMBER PIERCER
SILVER AXE
DARK TITAN MAGINN
PLASMA CHASER
HORRID WORM
PSYSHROOM
GAMIL
SNIPER MOSQUITO
EARTHSTOMP GIANT
THREE-EYED DRAGONFLY
BLOODWIG MANTIS

METALWING SKYTERROR
STAINED GLASS
WYN THE ORACLE
CURIOUS EYE
Armored Warrior Quelos
MURAMASA, DUKE OF BLADES
KING NEPTAS
LE QUIST THE ORACLE
DAIDALOS GENERAL OF FURY
Geoshine Spectral Knight
Necrodragon Galbazeek

HEADLONG GIANT
Tower Shell
Stampeding Longhorn
Xeno Mantis
Masked Pomegranate
Ultra Mantis, Scourge of Fate
Clobber Totem

Dragos Ionita added 8 commits February 14, 2025 18:40
Silver Axe
Dark Titan Maginn
Plasma Chaser
Horrid Worm
Psyshroom
Gamil Knight of Hatred
Sniper Mosquito
Earthstomp Giant
Photocide Lord of the Wastes
Three-Eyed Dragonfly
Bloodwig Mantis

QoL fx: MayDraw1ToMana
…ffects+qol-refactors

Merge upstream 21.02.2025
Removed wheneverThisAttacks fx,

Refactored blockers list initialization

Refactored conditional CantBeBlocked cards

Improved admin commands for faster initialization of cards in various play zones.
@sindreslungaard
Copy link
Owner

Looks good but I think the main reason for adding the new BlockerSelection event should be to have one place for cards to hook into and modify the blockers list. So instead of having a blockers list on both AttackCreature and AttackPlayer it would only be on this new blocker event, and instead of cards using conditions with functions for value for conditional blockers they should instead hook into this event and remove/add itself where necessary, similar to what some cards already to with the AttackCreature and AttackPlayer event, for example "Ancient Giant".

And maybe a better name for the new event would be SelectBlockers as the existing "Step" events are direct references to the predefined steps in the official rules.

Also the blockers slice on the new event should be []*Card not *[]*Card and then you pass the event struct around as a reference which will allow other functions to modify it without storing the actual slice as a pointer on the event.

Please also open individual pull requests for individual features/fixes like admin commands and CanPlayCard fix etc. in the future instead of appending to existing PR's.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants