Skip to content

Conversation

@muz
Copy link
Contributor

@muz muz commented Jan 25, 2026

Testing the waters a bit...

In other efforts to address some TODOs around Token implementations I saw there's a comment encouraging that we move towards using CreatureToken;

// CHECK: private class for inner tokens (no needs at all -- all private tokens must be replaced by CreatureToken)

That coupled with @theelk801 (rightly!) calling me out on PR comments to align with this, suggests to be that there's a long-standing desire to just get away from custom Private Tokens and it's just a matter of time/effort to do that...

So, I've had a stab at adjusting two instances below to remove their dependency on custom classes. If this PR is approved, or more guidance can be given, I can align with that and have a go at cleaning up other instances retroactively that exist in the codebase 🙏 Happy to break the PRs out into a handful at a time to make reviewing easier to digest too 😄

@github-actions github-actions bot added the cards label Jan 25, 2026
}
game.addEffect(new BecomesCreatureSourceEffect(
new AnsweredPrayersToken(), CardType.ENCHANTMENT, Duration.EndOfTurn
new Angel33Token(), CardType.ENCHANTMENT, Duration.EndOfTurn
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case, there is an existing token class that represents what this permanent becomes. If we're happy to piggy back onto those classes where the effect isn't wholly custom, I can keep this in mind.

Copy link
Contributor

Choose a reason for hiding this comment

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

@theelk801 any opinion on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did get curious at how this works in game, and with a locally built client it seems fine? I had the Answered Prayers with the correct attributes, and the art was still visually Answered Prayers.

It's very possible I'm overlooking some nuance here to why we wouldn't want to pass a fully fledged Token in as the blueprint to BecomesCreatureSourceEffect, but from my cursory look - maybe it's okay? 🤷

@JayDi85
Copy link
Member

JayDi85 commented Jan 26, 2026

For info:

  • public tokens are tokens with official images, it’s share same images between same tokens and uses static stats in most use cases (*/* are exceptions);
  • private tokens are tokens without images, e.g. special creatures with dynamic stats, so there are impossible to print static image for it and wizards don’t do it;
  • creature token is inner/builder class to simplify private tokens setup;

@JayDi85
Copy link
Member

JayDi85 commented Jan 26, 2026

Wizards can print new token with new sets, so old private token can be replaced by new public token as well. I don’t have any details on wizards logic for token printing. So the main point for this — look at official token image.

@muz
Copy link
Contributor Author

muz commented Jan 26, 2026

100%. It was rattling about the back of my head, but I had wondered about something to audit the use of custom tokens (either private ones currently, or ones declared via CreatureToken) where unnecessary - but that comes much later after all this other clean up first.

Public tokens represent items where Wizards have printed actual token items. They have certainly "gone back" and printed missing tokens with the release of new sets and commander decks in the past and gotten better about printing cards with no supporting token - but it's not wholly consistent either.

In the case of cards becoming a token, be it public token or CreatureToken as the blueprint, it'd be good to know if it matters or if we have a preference for implementation convention. So far, it looks like the client correctly uses the original permanent art and attributes where applicable and part of me leans towards using the public token. It'll potentially reduce future work if there are other refactoring to be made, and semantically it makes it easier to find equivalent code paths.

Mirroring this off of tabletop play, it's valid I guess to use newly printed tokens to represent an item made by an older card where a token didn't originally exist.


But yeah, as a guiding set of principles:

  • There are currently private tokens that Wizards never have produced an explicit functional printing for. These should use CreatureTokens
  • For passing in a blueprint to BecomesCreatureSourceEffect style calls, do we want to say "it MUST be a CreatureToken" or using a public token as blueprint is fine?

@JayDi85
Copy link
Member

JayDi85 commented Jan 26, 2026

BecomesCreatureSourceEffect's token param used for blueprint e.g. for characteristics copy. It do not support image. So it can be used for any token source -- public token, private token, creature token -- it's ok here. And it's ok to replace custom blueprint by public blueprint/token.

Moreover, BecomesCreatureSourceEffect's and any blueprint token effects can try to use image info, so it will be updated too (if effect uses public token).

Example:

permanent.setExpansionSetCode(token.getExpansionSetCode());
permanent.setUsesVariousArt(token.getUsesVariousArt());
permanent.setCardNumber(token.getCardNumber());
permanent.setImageFileName(token.getImageFileName());
permanent.setImageNumber(token.getImageNumber());

Maybe it will work fine but I'm not sure -- need research and test.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants