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

Easily sync config values between server & client #102

Merged
merged 3 commits into from
Jan 19, 2025
Merged

Conversation

Lyfts
Copy link
Member

@Lyfts Lyfts commented Jan 19, 2025

This adds a way to easily sync config values from server -> client by simple annotating the config field (or class) with @Config.Sync.
All values are restored to their original value once the player leaves the server.

Tested with this lovely config https://github.com/GTNewHorizons/GTNHLib/blob/7b65533a67926446c2e98f77c4503578dd049226/src/main/java/com/gtnewhorizon/gtnhlib/config/TestConfig.java and everything seems to sync correctly.

@Lyfts Lyfts requested a review from a team January 19, 2025 16:48
Copy link
Member

@serenibyss serenibyss left a comment

Choose a reason for hiding this comment

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

Great work, this should be a very useful feature

@serenibyss serenibyss merged commit 2752dc6 into master Jan 19, 2025
1 check passed
@serenibyss serenibyss deleted the config-sync branch January 19, 2025 18:06
@mitchej123
Copy link
Contributor

Leaving this comment here for any future crashes that come out of this:

I like the idea overall, but I think that syncing MC restart fields is a bad idea. A lot of configs use this for mixins, and also gate access to mixins introduced fields/changes later. Without an annotation processor to warn at compile time this leaves a lot of room open for immediate client side crashes if someone marks such a config as Sync without realizing the problems.

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

Successfully merging this pull request may close these issues.

3 participants