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

Proposal to be more explicit in naming of ModConfig.Type #1786

Open
ThatGravyBoat opened this issue Dec 18, 2024 · 1 comment
Open

Proposal to be more explicit in naming of ModConfig.Type #1786

ThatGravyBoat opened this issue Dec 18, 2024 · 1 comment
Labels
1.21.4 Targeted at Minecraft 1.21.4 enhancement New (or improvement to existing) feature or request request for comments For gathering opinions on some topic or subject

Comments

@ThatGravyBoat
Copy link
Contributor

Description of issue:
Currently as of writing this ModConfig.Type has the following values:

  • CLIENT
  • COMMON
  • SERVER
  • STARTUP

They all are different but similar in how they work but one is extra special and that is SERVER it on connection of a client the server will read all server configs and send them to the client. While this is helpful this can also lead to issues where if a developer does not know that the SERVER config type is automatically synced they could unintentionally leak information such as passwords (this has been seen in production) as such I propose the following.

Proposal:
With the issue above described I think that changing the name of the type to better represent what it does explicitly would be good.
Changing the name to something along the lines of SERVER_SYNCED would be best and possibly making 2 separate types one that is synced and one that isnt would probably be even better as SERVER type still has the property that its per world/server and not global.

@ThatGravyBoat ThatGravyBoat added the triage Needs triaging and confirmation label Dec 18, 2024
@TelepathicGrunt
Copy link
Contributor

An idea floating around in chat is to split off the syncing as a separate option and then print a dev warning if you set sync to true for a config type that does not support syncing.

Some pros and cons with that but it is a thought

@TelepathicGrunt TelepathicGrunt added enhancement New (or improvement to existing) feature or request request for comments For gathering opinions on some topic or subject 1.21.4 Targeted at Minecraft 1.21.4 and removed triage Needs triaging and confirmation labels Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.21.4 Targeted at Minecraft 1.21.4 enhancement New (or improvement to existing) feature or request request for comments For gathering opinions on some topic or subject
Projects
None yet
Development

No branches or pull requests

2 participants