-
-
Notifications
You must be signed in to change notification settings - Fork 177
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
Synchronize Registry-Affecting Configs to Client before Registry Sync #1564
base: 1.21.x
Are you sure you want to change the base?
Conversation
|
The problem with configs to remove items is not limited to just syncing to clients for the record. It also notably causes issues with missing mappings as you can change the config option to remove an item that previously existed. With removal of a mod its more clear that it might damage old worlds, but with simply changing a config option that is expected to be more stable. What is the usecase for conditionally registered items? Every usecase I have seen so far works just as well if you just hide the item when "disabled" |
Wouldn’t the feature flag pr technically be usable for hiding items safely? |
This is not limited to just builtin toml configs. Modders should warn players about the potential issues of changing those configs. On example use case: I have a mod that have items of a kind that modpack devs would like to add more while they provide textures themselves. Exact logic of those items are provided by datamaps, but it would be cleaner to have those statically defined to have custom texture. |
I have already implemented this feature in my mods and tested this PR with my mods. I just want to have a way to synchronize the data to client and prompt client users if they would like to implement. |
I have to agree with KnightMiner here -- I can only imagine how often the following would play out with this sort of feature:
More broadly -- I really dislike the idea of a player playing on a server changing settings locally that affect play outside the server. That seems like a huge source of confusion waiting to happen, all things told. Especially since as far as I can tell there's no mechanism in place here to recover the original client settings later? |
It is to be expected that applying the config from a server would probably corrupt the client world, and player needs to be informed about it. An instance that player would join a server is usually dedicated for that server, where single player worlds are mostly used to test the server. Mods that use external resources to affect static registry is already widely used (thingspack, KubeJS, etc). Currently when things doesn’t match it just says registry mismatch and requires player to download newest resources manually. This PR is a feature to allow players to update local resource in one click. Script-like mods should not use this feature for safety reasons. |
Is it better for me to create a warning screen to stress that replacing those configs could seriously damage single player saves, and add an option to backup those configs? |
I think the feature is inherently problematic if this is a risk at all. It's way too easy to do accidentally, and "doing dangerous stuff that will potentially break other worlds" should never be a prerequisite to joining a server, warning screen or no. |
Though this is an actual requirement regardless of this PR. The only difference is that without it players need to manually download files from the server website / group chat, like how they update mods, but with this feature it’s much easier. |
It is not a requirement without this PR -- generally speaking this sort of stuff is handled by just making an instance for that server, with whatever files in question. Folks don't generally play on their local worlds and a server from the same instance, if the two require different config files. Only with this PR is that something people are at risk of doing. |
This PR makes updates to those servers easier, and when players accidentally delete all their configs it allows them to download from server again with ease. |
Yes; it also adds a very easy way for players to permanently mess up a local world, something players generally expect to not be possible if they're not actually changing any configs themselves or anything. |
The above highlighted another concern which I had not stated before: why are we requiring a client restart to apply server config in the first place? Why not instead encourage config options that are syncable to not require that restart on config mismatch? So registry affecting configs are not syncable without a restart, well then don't conditionally register. Condition other behaviors to hide/disable the registry entries so a restart is not required. |
Eh, that's not always possible -- my understanding of the use case here was in cases where you have some sort of configuration able to specify details of any number of different blocks to register or the like. That said, I'm still unconvinced that this is a good idea |
This PR addresses the following problem:
Sometimes modders would like to have a config option to disable certain items in game, or allow users to add more contents that needs to be statically registered. It was not recommended to do it in start up config as it will cause registry desync when server and client has different settings, causing misleading error message. This PR solves the issue by allowing mods to synchronize server-side registry-affecting configs to client, and ask client-side players if they want to apply those configs (and restart) or not.
This PR introduce the following concepts:
RegistryConfigHandler
: It is a handler that is responsible for encoding RACs intoJsonElement
for synchronization, decoding json, check if client and server have different configs that will cause registry desync or any other problem, and overwrite client config with the server one.StartUpRegistryConfigHandler
: A template implementation ofRegistryConfigHandler
for those who useModConfigSpec
start up config.