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

sync part of cards zcc properly #12186

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

Conversation

Susucre
Copy link
Contributor

@Susucre Susucre commented Apr 25, 2024

For MDFC / Split / Others. Started from argumenting in #12184

It would make a LOT of sense to keep part of cards zcc sync.
All tests are working here. MDFC are some of the cards with a lot of coverage. There is no issue ensuring the main card and the parts are in zcc sync.

Please @JayDi85, give me an example of behavior this would break. I can add unit tests if you're not convinced.

Fix #10478

@@ -438,7 +438,7 @@ private static boolean maybeRemoveFromSourceZone(ZoneChangeInfo info, Game game,
if (event.getToZone() == Zone.BATTLEFIELD && event.getTarget() != null) {
event.getTarget().updateZoneChangeCounter(game, event);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw for that TODO: wtf comment in PermanentCard, this is where that would be called.

Copy link
Member

@JayDi85 JayDi85 Apr 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Permanent’s zcc is static and can’t be changed (only synced on first usage/init/put). If it changed somehow then it can be an error or wrong code usage.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but when the permanent is put onto the battlefield, its card's zcc gets updated, and the permanent stores that updated zcc.

I don't think a permanent's zcc should move after that.

@JayDi85
Copy link
Member

JayDi85 commented Apr 25, 2024

  • 1 - Need research. updateZoneChangeCounter - there are many places with card overrides, unsupported errors, etc (~20 places). Only few places are legit to use like new permanent/token code. Other places needs to research for safety or potential problems.
  • 2 - Need improve. Must add comments to the game engine code with every new workarounds. Example: “must change zcc from main card so it will change all parts too”;
  • 3 - For info. That’s code is not sync a zcc, it’s “move” all parts (increment zcc of all parts). It will fail in really independent parts like Mutate and Meld’s parts (they linked with diff zcc). So don’t use same zcc for different objects in effects;
  • 4 - Need research. Card can be moved by part or by main card (if it main then all parts moved too), depends on target usage. Make sure it compatible with zcc updates (no duplicated calls with +2 zcc instead +1).

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

Successfully merging this pull request may close these issues.

ZCC of MDFCs aren't being incremented correctly
2 participants