Skip to content

Conversation

@muz
Copy link
Contributor

@muz muz commented Jan 25, 2026

The (currently ignored) test_checkMissingTokenData test case has a schwack of warnings like below:

warning, token class must ends with Token: AvatarToken2 from mage.game.permanent.token.AvatarToken2

There was an intention somewhere that tokens following a naming convention. This is likely one of those things that unless someone actually sits down to deal with it, it'll be a code smell that lurks for longer than it needs to.

In the interests of cleaning this up and making the test output cleaner as well as any other future token work, I can have a stab at cleaning these up as proposed below. If this change is welcome, I'm willing to go and clean up others to, to get this done hopefully once-and-for-all.

Just anecdotally, getting involved in contributing to this codebase is complex as is due to the age and size of it. Something that actively hindered me was inconsistencies where it wasn't immediately clear what was desired. So this hopefully leaves things in a better state than I found them, and makes it easier for the next person - albeit marginally.

Opting to just do two to illustrate the point and garner feedback, but also to make reviews manageable instead of dumping it all in one massive PR 😅

Copy link
Contributor

@xenohedron xenohedron left a comment

Choose a reason for hiding this comment

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

Looks like a fine cleanup to me

@muz muz force-pushed the token_naming_convention_a branch from 2106f72 to 6d1cbd5 Compare January 26, 2026 01:47
@muz
Copy link
Contributor Author

muz commented Jan 26, 2026

Rebased given the other much larger tokens-database refactor just landed in master first.

@JayDi85 JayDi85 merged commit 4da61af into magefree:master Jan 26, 2026
1 check passed
@muz muz deleted the token_naming_convention_a branch January 26, 2026 13:31
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.

3 participants