magefree / magefree/mage

LKI of copied spells are not fully implemented

Open
#12,911 0 comments 0 reactions 1 assignee Claimed by @JayDi85 View on GitHub
bug refactoring
Dominant language
Java
Stars
2.4k
Forks
940
Avg merge
2d 12h
Merged PRs (30d)
160

Description

After Swan Song fix f85a70780f958112014fa01fb4885a3477a5e230 from #12883 found many problems:

1. Looks like original spell LKI implementation is not fully implemented (also related to Unbound Flourishing's PR #12597) -- there are many places with stack removing without LKI save, search by `getStack().remove`. All that places require same fix as Swan Song above.
![shot_240925_095711](https://github.com/user-attachments/assets/07320d39-cdc0-42be-8775-ee4a099d6666)

2. All code with spell's copy check is outdated, cause current engine uses really new cards for copies (old code used original card, so that check required). So spell copy check must be removed as outdated. Also manual remove from stack must be removed too -- card moving code already processing it.
![shot_240925_100348](https://github.com/user-attachments/assets/112d7141-a385-46e4-aa51-d2694bcb6fa5)
![shot_240925_100959](https://github.com/user-attachments/assets/23418758-96eb-4de2-bee1-5f50f382fb41)

3. Many use cases with counter of spell's copy is not covered by tests (no breakpoints at all on debug). So outdate code remove must help with it. If some check code will keep exists then it must be covered by tests like 701bd68d97120576b34a3a8a0259c6999b9db7fe.

TODO:
* [ ] write covered tests for copy code with stack.remove calls - search by `// TODO: need copy tests, see #12911`
* [ ] remove all calls of `getStack().remove` from outside of inner game engine - must be replaced by card moving code
* [x] make sure counter of copied spells covered by tests (it must counter and save LKI for next effects);
* [ ] review all use of `isCopy()` and remove outdate code (with spells, with another objects, etc);

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.