Overhaul of copy effects
- Dominant language
- Java
- Stars
- 2.4k
- Forks
- 940
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 160
Description
This issue was noticed and an attempt to fix started in #10287, but it's connected to a bunch of things so I think a clear definition of scope is needed. Some other previous discussion of the topic in: https://github.com/magefree/mage/issues/9444#issuecomment-1233576579
Currently XMage has a few different ways of implementing copy effects.
* The main continuous effect is `CopyEffect` which is called by various ad-hoc custom one-shot effects, and by `Game::copyPermanent`.
* There's the standard one-shot `CopyPermanentEffect` which uses `Game::copyPermanent`.
* There's `CopyTokenEffect` which is used for flip cards, using tokens to hold their characteristics, and not for actually copying tokens.
* There's `CreateTokenCopyTargetEffect` which does indeed create a token copy of a target. The constructors in that class are a total mess and there are fields for all the possible characteristics that could be adjusted. It uses `CopyTokenFunction` (not to be confused with `CopyTokenEffect`) as the utility class (which has a confusingly named `apply()` method but isn't an `Effect`).
* A `CopyApplier` is used to adjust characteristics, but it is not at all intuitive to use correctly.
* There's `StackObject::createCopyOnStack` which is mostly separate, with its own `StackObjectCopyApplier`, and called directly by effects that copy an object on the stack. (This has good reason to be separate and is not impacted by most of the other weirdness.)
* There's `Game::copyCard` for effects that cast a copy of a card. Thankfully this one doesn't need a CopyApplier yet.
There are a lot of associated bugs because XMage currently doesn't have a good way of getting copiable values. There are workarounds for the special cases of transformed permanents (which have their own issues with a major rework pending in #10385), face-down permanents (also a bunch of messy code), prototype (per #11249, same workaround method as transformed cards) and copies of copies (which are detected by searching through continuous effects e.g. https://github.com/magefree/mage/pull/10287#issuecomment-1526818072). There are other circumstances that are not handled correctly such as Primal Clay (#10661).
The relevant section of the Comprehensive Rules is [707. Copying Objects](https://yawgatog.com/resources/magic-rules/#R707). There's a lot of info there but the most important paragraph is probably this:
> 707.2. When copying an [object](https://yawgatog.com/resources/magic-rules/#object), the [copy](https://yawgatog.com/resources/magic-rules/#copy) acquires the [copiable values](https://yawgatog.com/resources/magic-rules/#copiable_values) of the original [object](https://yawgatog.com/resources/magic-rules/#object)'s [characteristics](https://yawgatog.com/resources/magic-rules/#characteristics) and, for an [object](https://yawgatog.com/resources/magic-rules/#object) on the [stack](https://yawgatog.com/resources/magic-rules/#stack), choices made when casting or activating it ([mode](https://yawgatog.com/resources/magic-rules/#modal_mode), [targets](https://yawgatog.com/resources/magic-rules/#target), the value of [X](https://yawgatog.com/resources/magic-rules/#x), whether it was [kicked](https://yawgatog.com/resources/magic-rules/#kicker_kicked), how it will affect multiple [targets](https://yawgatog.com/resources/magic-rules/#target), and so on). The [copiable values](https://yawgatog.com/resources/magic-rules/#copiable_values) are the values derived from the text printed on the [object](https://yawgatog.com/resources/magic-rules/#object) (that text being [name](https://yawgatog.com/resources/magic-rules/#name), [mana cost](https://yawgatog.com/resources/magic-rules/#mana_cost), [color indicator](https://yawgatog.com/resources/magic-rules/#color_indicator), [card type](https://yawgatog.com/resources/magic-rules/#card_type), [subtype](https://yawgatog.com/resources/magic-rules/#subtype), [supertype](https://yawgatog.com/resources/magic-rules/#supertype), [rules text](https://yawgatog.com/resources/magic-rules/#rules_text), [power](https://yawgatog.com/resources/magic-rules/#power), [toughness](https://yawgatog.com/resources/magic-rules/#toughness), and/or [loyalty](https://yawgatog.com/resources/magic-rules/#loyalty)), as [modified](https://yawgatog.com/resources/magic-rules/#modified) by other [copy](https://yawgatog.com/resources/magic-rules/#copy) [effects](https://yawgatog.com/resources/magic-rules/#effect), by its face-down [status](https://yawgatog.com/resources/magic-rules/#status), and by "as . . . [enters the battlefield](https://yawgatog.com/resources/magic-rules/#enters_the_battlefield)" and "as . . . is turned [face up](https://yawgatog.com/resources/magic-rules/#face_up)" [abilities](https://yawgatog.com/resources/magic-rules/#ability) that set [power](https://yawgatog.com/resources/magic-rules/#power) and [toughness](https://yawgatog.com/resources/magic-rules/#toughness) (and may also set additional [characteristics](https://yawgatog.com/resources/magic-rules/#characteristics)). Other [effects](https://yawgatog.com/resources/magic-rules/#effect) (including [type](https://yawgatog.com/resources/magic-rules/#type)-changing and [text-changing effects](https://yawgatog.com/resources/magic-rules/#text-changing_effect)), [status](https://yawgatog.com/resources/magic-rules/#status), [counters](https://yawgatog.com/resources/magic-rules/#counter), and [stickers](https://yawgatog.com/resources/magic-rules/#sticker) are not copied.
There are various different sorts of copy effects:
* A permanent enters the battlefield as a copy of another permanent (e.g. [[Clone]], [[Clever Impersonator]])
* A permanent on the battlefield becomes a copy of another permanent (e.g. [[Cryptoplasm]], [[Metamorphic Alteration]])
* A token is created that's a copy of a permanent or a card (e.g. [[Quasiduplicate]], [[Back from the Brink]])
* A copy of a spell or ability is created on the stack (e.g. [[Reverberate]], [[Lithoform Engine]])
* A copy of a card is created and cast (e.g. [[Elite Arcanist]], [[Demilich]])
* Any of these may have modifications or exceptions to the copied characteristics per 707.9.
Proposed out of scope:
* Creating a copy of an object on the stack. The existing `createCopyOnStack` method is adequate. There are additional copiable values in this scenario. The Layer 1 effects that need to be accounted for with permanents aren't an issue here.
* Creating a copy of a card in order to cast that copy. The existing `copyCard` method is sufficiently different. The Layer 1 effects aren't an issue here either. So far, none of these effects modify the characteristics of the card.
Remaining scope for rework:
* `CopyPermanentEffect` / `game.copyPermanent` / `CopyEffect`
* CreateTokenCopyTargetEffect
I think the way to handle each of these is to have a `CopiableValues` object, that is initialized with each card, and updated for each permanent as part of the layer process after Layer 1 effects are applied. It should store only the characteristics that are copiable values, and it should have easy methods to adjust those characteristics (either directly or using a CopyApplier). These copy effects should take the CopiableValues object from the card/permanent it's copying, and store a copy of that to initialize and apply its effect.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by reading CopyPermanentEffect, Game.copyPermanent, CopyEffect, CreateTokenCopyTargetEffect, CopyApplier, and the layer-processing code described in the issue. Review Comprehensive Rules 707.2 and the linked discussions before defining the design. Done means the remaining permanent and token-copy paths share a clear CopiableValues approach, while stack copies and cast card copies remain out of scope.
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
- Needs clarification
- Newbie friendliness
- 25/100