magefree / magefree/mage

REFACTOR: AsThoughEffect need a refactor

Open
#9,521 2 comments 0 reactions 0 assignees View on GitHub
bug refactoring
Dominant language
Java
Stars
2.4k
Forks
940
Avg merge
2d 12h
Merged PRs (30d)
160

Description

Currently AsThoughEffect use their `applies` method to *both* identify if the asThoughEffect can be used AND to make changes to the state of the game.

E.g. [[Bolas's Citadel]] has the following code at the end of its `.applies`:
https://github.com/magefree/mage/blob/572104b8fcdb50b509c34b10e113cf39602374bb/Mage.Sets/src/mage/cards/b/BolassCitadel.java#L116-L122

If this method is called, it will cause the controller to pay life instead of mana to cast the spell *even if they did not want to use Bolas's citadel*.

This in turns requires each of these effects to be checked while it `game.inCheckPlayableState() == true`, and list of them to be stored in the state so that one the second pass (when `game.inCheckPlayableState() == false`) that list can be read back and one of the entries in it be picked.

This needs to change. I propose that the functionality be split across two methods:
1. The first one which checks if the asThoughEffect applies in this instance
2. The second actually makes and changes required to the state

This is the same idea as how TriggeredAbility has `checkEventType`, `checkTrigger` and only after `resolve`.

_Out of the discussion in https://github.com/magefree/mage/issues/9268#issuecomment-1243287853_

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the AsThoughEffect `applies` method and the BolassCitadel.java example linked in the issue, then trace `game.inCheckPlayableState()` and the stored effect list. Compare this flow with TriggeredAbility’s `checkEventType`, `checkTrigger`, and `resolve` methods. Done means applicability checks no longer change game state, and the affected effects and state handling follow the proposed separation.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
game-dev
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.