Refactor: PermanentToken should not have CardImpl as super
- Dominant language
- Java
- Stars
- 2.4k
- Forks
- 940
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 160
Description
This is something I mentioned in the discord that I wanted to put here as an issue.
# Problem
Currently, the inheritance of these classes look like so:
`MageObjectImpl` <- `CardImpl` <- `PermanentImpl` <- (`PermanentCard`, `PermanentToken`, etc..)
This creates an incompatibility in regards to being compliant with the Comprehensive Rules.
## Not all Permanents are Cards
> 110.1. A [permanent](https://yawgatog.com/resources/magic-rules/#permanent) is a [card](https://yawgatog.com/resources/magic-rules/#card) or [token](https://yawgatog.com/resources/magic-rules/#token) on the [battlefield](https://yawgatog.com/resources/magic-rules/#battlefield). A [permanent](https://yawgatog.com/resources/magic-rules/#permanent) remains on the [battlefield](https://yawgatog.com/resources/magic-rules/#battlefield) indefinitely. A [card](https://yawgatog.com/resources/magic-rules/#card) or [token](https://yawgatog.com/resources/magic-rules/#token) [becomes](https://yawgatog.com/resources/magic-rules/#becomes) a [permanent](https://yawgatog.com/resources/magic-rules/#permanent) as it [enters the battlefield](https://yawgatog.com/resources/magic-rules/#enters_the_battlefield) and it stops being a [permanent](https://yawgatog.com/resources/magic-rules/#permanent) as it's moved to another [zone](https://yawgatog.com/resources/magic-rules/#zone) by an [effect](https://yawgatog.com/resources/magic-rules/#effect) or rule.
## Only permanents should be on the battlefield
> 403.3. [Permanents](https://yawgatog.com/resources/magic-rules/#permanent) exist only on the [battlefield](https://yawgatog.com/resources/magic-rules/#battlefield). Every [object](https://yawgatog.com/resources/magic-rules/#object) on the [battlefield](https://yawgatog.com/resources/magic-rules/#battlefield) is a [permanent](https://yawgatog.com/resources/magic-rules/#permanent).
# Proposed Solution
* Make `PermanentImpl` extend `MageObjectImpl` instead.
* Get rid of `PermanentCard` and `PermanentToken` and instead create a new enum `PermanentType`.
* I think this is already mostly there, but casting `CardImpl` should properly create new `PermanentImpl` that is separate from the `CardImpl` that created it. In other words, `CardImpl` should only exist where `Card`s should exist, and `Spell`s should only exist on the stack, and `PermanentImpl` should only exist on the battlefield.
* Zone change code can possibly be massively simplified by not trying to have cards where they shouldn't.
# Problems that become easier to solve
## Mutate
Mutate is one of those mechanics that just isn't implemented yet.
Take this into consideration [725. Merging with Permanents](https://yawgatog.com/resources/magic-rules/#R725).
> 725.2d. If a [merged permanent](https://yawgatog.com/resources/magic-rules/#merged_permanent) contains a [token](https://yawgatog.com/resources/magic-rules/#token), the resulting [permanent](https://yawgatog.com/resources/magic-rules/#permanent) is a [token](https://yawgatog.com/resources/magic-rules/#token) only if the topmost component is a [token](https://yawgatog.com/resources/magic-rules/#token).
This was one of the main issues with the Mutate PR #8733.
Mutating over a token would create some issues with trying to turn a `PermanentToken` into a `PermanentCard`.
> 725.2. To merge an [object](https://yawgatog.com/resources/magic-rules/#object) with a [permanent](https://yawgatog.com/resources/magic-rules/#permanent), place that [object](https://yawgatog.com/resources/magic-rules/#object) on top of or under that [permanent](https://yawgatog.com/resources/magic-rules/#permanent). That [permanent](https://yawgatog.com/resources/magic-rules/#permanent) [becomes](https://yawgatog.com/resources/magic-rules/#becomes) a [merged permanent](https://yawgatog.com/resources/magic-rules/#merged_permanent) represented by the [card](https://yawgatog.com/resources/magic-rules/#card) or [copy](https://yawgatog.com/resources/magic-rules/#copy) that represented that [object](https://yawgatog.com/resources/magic-rules/#object) in addition to any other components that were representing it.
Thanks to a `Permanent` not being a 1-to-1 with a `Card`, you can have the `Permanent` have a list (probably a deque?) of cards that define it. Mutating a `Permanent` now becomes a simple case of modifying the `MageObjectImpl` characteristics and adding the card from the [Mutating Creature Spell](https://yawgatog.com/resources/magic-rules/#R403:~:text=702.140.%20Mutate-,702.140a,-.%20Mutate%20appears) to the list instead of having to worry about transforming a token into a card.
## Meld
> 701.37a. [Meld](https://yawgatog.com/resources/magic-rules/#meld) is a [keyword action](https://yawgatog.com/resources/magic-rules/#keyword_action) that appears in an [ability](https://yawgatog.com/resources/magic-rules/#ability) on one [card](https://yawgatog.com/resources/magic-rules/#card) in a [meld](https://yawgatog.com/resources/magic-rules/#meld) pair. To [meld](https://yawgatog.com/resources/magic-rules/#meld) the two [cards](https://yawgatog.com/resources/magic-rules/#card) in a [meld](https://yawgatog.com/resources/magic-rules/#meld) pair, put them onto the [battlefield](https://yawgatog.com/resources/magic-rules/#battlefield) with their back faces up and combined. The resulting [permanent](https://yawgatog.com/resources/magic-rules/#permanent) is a single [object](https://yawgatog.com/resources/magic-rules/#object) represented by two [cards](https://yawgatog.com/resources/magic-rules/#card).
In essence, Meld is doing something that is extremely similar to Mutate. The only difference here, is that you can't meld tokens and the melding permanents leave the battlefield (to become cards again I suppose) before the new melded (singular) permanent returns to the battlefield.
Hopefully, the separation of cards from the permanent they become should help solve #9642 and similar issues. Though I haven't looked into this too extensively so it could be a naïve assumption on my part.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by tracing MageObjectImpl, CardImpl, PermanentImpl, PermanentCard, and PermanentToken, along with the zone-change paths that construct them. Compare the current inheritance and casting behavior with the proposed PermanentType separation, including the mutate and meld scenarios described in the issue. Done means cards, spells, and permanents are represented separately while preserving the stated battlefield rules.
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
- 20/100