OrnitheMC / ornithe-standard-libraries

Apache License 2.0
6 stars 5 forks source link

Config API #2

Open SpaceWalkerRS opened 1 year ago

SpaceWalkerRS commented 1 year ago

Discussion about a Config API, features it should have and how they should be implemented. Keep in mind

SpaceWalkerRS commented 1 year ago

While I'm personally against a "carpet-style" implementation (i.e. keeping all config options as raw values in a settings class, and using reflection to automatically parse all the settings in it), I think some form of hybrid approach could work.

What I mean by this is, the Config API in its most basic form consists only of (de)serializers. This allows mod developers to keep all their configs as raw values in a settings class (making them convenient to use in code, which is the main benefit of the "carpet-style" implementation IMO) and register (de)serializers for the API to handle saving/loading/etc. and which can be passed to a separate API for config menus.

We could also provide a basic extension to this that wraps the config values in objects to make the configs easier to create, though slightly less convenient to use in code.

SpaceWalkerRS commented 1 year ago

A few more ideas:

Implement ConfigEnvironments that define where configs apply: CLIENT for global client-side configs, SERVER for global (dedicated!) server-side configs, WORLD for world-specific configs, and perhaps DIMENSION for dimension-specific configs.

Implement different 'phases' during configs can be loaded, depending on whether mods need their config before or after the client/server/world has been fully initialized.

SRAZKVT commented 1 year ago

A few more ideas:

Implement ConfigEnvironments that define where configs apply: CLIENT for global client-side configs, SERVER for global (dedicated!) server-side configs, WORLD for world-specific configs, and perhaps DIMENSION for dimension-specific configs.

Implement different 'phases' during configs can be loaded, depending on whether mods need their config before or after the client/server/world has been fully initialized.

What you describe as ConfigEnvironment really sounds just like a scope, i propose it be named ConfigScope from now on.

I would say it would make sense to have as scopes:

Basically merging the global client and server scopes into one, as i personally really don't see a reason to have them seperated

The idea of phases for config is an interesting idea, but i am unsure of how useful it would actually be. Maybe in world generation ? But then you probably want to use world types anyway.

SpaceWalkerRS commented 1 year ago

A few more ideas: Implement ConfigEnvironments that define where configs apply: CLIENT for global client-side configs, SERVER for global (dedicated!) server-side configs, WORLD for world-specific configs, and perhaps DIMENSION for dimension-specific configs. Implement different 'phases' during configs can be loaded, depending on whether mods need their config before or after the client/server/world has been fully initialized.

What you describe as ConfigEnvironment really sounds just like a scope, i propose it be named ConfigScope from now on.

I would say it would make sense to have as scopes:

  • GLOBAL : Resolves to client's config and dedicated server's world. Basically, the most scope available for the current environment.
  • WORLD : Resolves to only the world folder, on both client and dedicated server.
  • Any other we think would be useful

Basically merging the global client and server scopes into one, as i personally really don't see a reason to have them seperated

This sounds good, I like it.

The idea of phases for config is an interesting idea, but i am unsure of how useful it would actually be. Maybe in world generation ? But then you probably want to use world types anyway.

Some configs might impact world behavior (like many of Carpet's 'creative mode' options do) and thus should be loaded before the world is initialized, while other configs might rely on the world to be initialized in order to validate their values.

SRAZKVT commented 1 year ago

The idea of phases for config is an interesting idea, but i am unsure of how useful it would actually be. Maybe in world generation ? But then you probably want to use world types anyway.

Some configs might impact world behavior (like many of Carpet's 'creative mode' options do) and thus should be loaded before the world is initialized, while other configs might rely on the world to be initialized in order to validate their values.

That is fair. Sounds like a good idea then. What should happen then if the world isn't fully doesn't have set values ? Should we just throw an exception, and a mod could hook up and write up a gui that would allow people to set variables to (for world), and for server just print out that config needs to be written manually before the world is allowed to be created ?

SpaceWalkerRS commented 1 year ago

What should happen then if the world isn't fully doesn't have set values ? Should we just throw an exception

Hmm that's a good point. My initial reaction would be, configs would by default just be set to, well, their default values, and once loaded in from disk the values get populated proper. But that could lead to unwanted behavior, so perhaps throwing an exception if the config hasn't been loaded from disk yet is a good idea.

a mod could hook up and write up a gui that would allow people to set variables to (for world), and for server just print out that config needs to be written manually before the world is allowed to be created ?

That's a neat idea. I think for most configs just populating them with default values if no config file exists yet is fine, but this would be a nice option to give mod devs.

SRAZKVT commented 1 year ago

What should happen then if the world isn't fully doesn't have set values ? Should we just throw an exception

Hmm that's a good point. My initial reaction would be, configs would by default just be set to, well, their default values, and once loaded in from disk the values get populated proper. But that could lead to unwanted behavior, so perhaps throwing an exception if the config hasn't been loaded from disk yet is a good idea.

a mod could hook up and write up a gui that would allow people to set variables to (for world), and for server just print out that config needs to be written manually before the world is allowed to be created ?

That's a neat idea. I think for most configs just populating them with default values if no config file exists yet is fine, but this would be a nice option to give mod devs.

Yeah, populating with default values for non required make sense. Now, what should required values be populated with ? we could go with a custom keyword, but that would probably need to write a custom parser for whatever text format will be used, as well as a custom writer. We could go with a custom object that signifies it isn't set, but that could conflict with developer values, though is less of an issue than writing and maintaining a custom parser imo

SpaceWalkerRS commented 1 year ago

Yeah, populating with default values for non required make sense. Now, what should required values be populated with ? we could go with a custom keyword, but that would probably need to write a custom parser for whatever text format will be used, as well as a custom writer. We could go with a custom object that signifies it isn't set, but that could conflict with developer values, though is less of an issue than writing and maintaining a custom parser imo

if the option is not required to have a value then we could just not write it to the file at all?

SpaceWalkerRS commented 1 year ago

also should we just not allow options to have null values? that would simplify some things

SRAZKVT commented 1 year ago

Yeah, populating with default values for non required make sense. Now, what should required values be populated with ? we could go with a custom keyword, but that would probably need to write a custom parser for whatever text format will be used, as well as a custom writer. We could go with a custom object that signifies it isn't set, but that could conflict with developer values, though is less of an issue than writing and maintaining a custom parser imo

if the option is not required to have a value then we could just not write it to the file at all?

would make sense imo to write it to the file to let users know the option exists, sure technically it wouldn't be needed, but it is clearer and simpler, and allows to not need to refer to documentation of a specific mod to get information, which is useful.

SRAZKVT commented 1 year ago

also should we just not allow options to have null values? that would simplify some things

I guess we could have null disallowed for any non required value, and null as a value that says it is required to set. Essentially that would still give it a utility to a problem i raised a bit earlier, but other than that one edge case, it should indeed just not be allowed.

SpaceWalkerRS commented 1 year ago

if the option is not required to have a value then we could just not write it to the file at all?

would make sense imo to write it to the file to let users know the option exists, sure technically it wouldn't be needed, but it is clearer and simpler, and allows to not need to refer to documentation of a specific mod to get information, which is useful.

That's a good point. I'm not sure how best to handle it then. A few ways I see:

SRAZKVT commented 1 year ago

if the option is not required to have a value then we could just not write it to the file at all?

would make sense imo to write it to the file to let users know the option exists, sure technically it wouldn't be needed, but it is clearer and simpler, and allows to not need to refer to documentation of a specific mod to get information, which is useful.

That's a good point. I'm not sure how best to handle it then. A few ways I see:

* make all options required

and "required" options would just have a null as value and a recommended default, but no actual default ?

* write the config files in two sections, a section for all options that have a value, and a section for all options that do not have a value (either just a list of the options, or otherwise give the options their default value, this value should then be discarded by the parser)

honestly this one seems unideal, for a user it would seem weird as to why a config file looks that way, would also make it a bit more complex for devs to go through big config files if there's multiple sections

SpaceWalkerRS commented 1 year ago

and "required" options would just have a null as value and a recommended default, but no actual default ?

That could work. I think I'd want configs to have proper default values though, so as not to put the responsibility on the end-user to set every single config value the first time they boot up the game with a particular mod.

SRAZKVT commented 1 year ago

and "required" options would just have a null as value and a recommended default, but no actual default ?

That could work. I think I'd want configs to have proper default values though, so as not to put the responsibility on the end-user to set every single config value the first time they boot up the game with a particular mod.

that is fair, the end user should only have to set values that are required. I guess in code, a value would only be set required, and when default config gets dumped, it is set to null, and anything that isn't null when read is either required and set or not required ?

SpaceWalkerRS commented 1 year ago

Just for clarification, if an option is required a) it must have a value b) that value must have been set by the end-user is that right? 'cause I've been using it to mean just the first condition.

SRAZKVT commented 1 year ago

Just for clarification, if an option is required a) it must have a value b) that value must have been set by the end-user is that right? 'cause I've been using it to mean just the first condition.

yup, and it being set as null in a file would mean it isn't set, and need to be set, and will raise that as an error.

question : what will happen in singleplayer if someone has a mod that has a required field (that is per world) and the exception is thrown ? the world folder is just not made right ? what happens when creating a world but a folder already exists, and there is just only the config in it ? the idea is to also make it possible for people to create a world without needing a mod, so the previously discussed gui in singleplayer cases might need to be a part of this library if the game simply refuses to create the world in the folder we asked it to

SpaceWalkerRS commented 1 year ago

Just for clarification, if an option is required a) it must have a value b) that value must have been set by the end-user is that right? 'cause I've been using it to mean just the first condition.

yup, and it being set as null in a file would mean it isn't set, and need to be set, and will raise that as an error.

Hmm, is it worth it to have such property? I don't think I've come across a mod that has it, most seem to do fine with just giving options default values.

question : what will happen in singleplayer if someone has a mod that has a required field (that is per world) and the exception is thrown ? the world folder is just not made right ? what happens when creating a world but a folder already exists, and there is just only the config in it ? the idea is to also make it possible for people to create a world without needing a mod, so the previously discussed gui in singleplayer cases might need to be a part of this library if the game simply refuses to create the world in the folder we asked it to

Yeah this is part of why I'm not sure if having any required options is worth it. But if we do want it, something akin to what modern MC versions have for setting game rules could work?

SRAZKVT commented 1 year ago

Just for clarification, if an option is required a) it must have a value b) that value must have been set by the end-user is that right? 'cause I've been using it to mean just the first condition.

yup, and it being set as null in a file would mean it isn't set, and need to be set, and will raise that as an error.

Hmm, is it worth it to have such property? I don't think I've come across a mod that has it, most seem to do fine with just giving options default values.

That is fair, i don't think i have ever seen one either honestly

question : what will happen in singleplayer if someone has a mod that has a required field (that is per world) and the exception is thrown ? the world folder is just not made right ? what happens when creating a world but a folder already exists, and there is just only the config in it ? the idea is to also make it possible for people to create a world without needing a mod, so the previously discussed gui in singleplayer cases might need to be a part of this library if the game simply refuses to create the world in the folder we asked it to

Yeah this is part of why I'm not sure if having any required options is worth it. But if we do want it, something akin to what modern MC versions have for setting game rules could work?

Sounds good, so we need to hack in the world creation menu something like that, or have an intermediate menu ? though that will be a change compared to vanilla, but i believe it could be considered minimal enough we can ignore it.

SpaceWalkerRS commented 1 year ago

Yeah this is part of why I'm not sure if having any required options is worth it. But if we do want it, something akin to what modern MC versions have for setting game rules could work?

Sounds good, so we need to hack in the world creation menu something like that, or have an intermediate menu ? though that will be a change compared to vanilla, but i believe it could be considered minimal enough we can ignore it.

I suppose it depends. If we ditch required options, it can be an extra option in the world creation screen. If we do want required options it will have to be an intermediate screen IMO, 'cause the end-user will have to set these options.

SRAZKVT commented 1 year ago

Yeah this is part of why I'm not sure if having any required options is worth it. But if we do want it, something akin to what modern MC versions have for setting game rules could work?

Sounds good, so we need to hack in the world creation menu something like that, or have an intermediate menu ? though that will be a change compared to vanilla, but i believe it could be considered minimal enough we can ignore it.

I suppose it depends. If we ditch required options, it can be an extra option in the world creation screen.

but there would be one extra option for each mod wouldn't there ? that would be not ideal.

If we do want required options it will have to be an intermediate screen IMO, 'cause the end-user will have to set these options.

Yeah.

Honestly one issue i have with dealing with it within OSL means that #1 will need to be finished before this part can be implemented, or we just mark it as non supported yet but planned in the future, for when #1 is done ?

SpaceWalkerRS commented 1 year ago

Yeah this is part of why I'm not sure if having any required options is worth it. But if we do want it, something akin to what modern MC versions have for setting game rules could work?

Sounds good, so we need to hack in the world creation menu something like that, or have an intermediate menu ? though that will be a change compared to vanilla, but i believe it could be considered minimal enough we can ignore it.

I suppose it depends. If we ditch required options, it can be an extra option in the world creation screen.

but there would be one extra option for each mod wouldn't there ? that would be not ideal.

I think it can be a single button for "mod configs" which leads to a screen that lists all mod configs, where you can click on them to go to another screen to change the values.

If we do want required options it will have to be an intermediate screen IMO, 'cause the end-user will have to set these options.

Yeah.

Honestly one issue i have with dealing with it within OSL means that https://github.com/OrnitheMC/ornithe-standard-libraries/issues/1 will need to be finished before this part can be implemented, or we just mark it as non supported yet but planned in the future, for when https://github.com/OrnitheMC/ornithe-standard-libraries/issues/1 is done ?

It's probably easiest to just not implement required options until the GUI module is done, and dealing with it then? On the other hand, if we drop required options altogether, the world-creation-mod-config-screen-thing can be moved into a Mod Menu mod/module.

SRAZKVT commented 1 year ago

Yeah this is part of why I'm not sure if having any required options is worth it. But if we do want it, something akin to what modern MC versions have for setting game rules could work?

Sounds good, so we need to hack in the world creation menu something like that, or have an intermediate menu ? though that will be a change compared to vanilla, but i believe it could be considered minimal enough we can ignore it.

I suppose it depends. If we ditch required options, it can be an extra option in the world creation screen.

but there would be one extra option for each mod wouldn't there ? that would be not ideal.

I think it can be a single button for "mod configs" which leads to a screen that lists all mod configs, where you can click on them to go to another screen to change the values.

hmmm, not a bad idea, it can be a seperate mod then

If we do want required options it will have to be an intermediate screen IMO, 'cause the end-user will have to set these options.

Yeah. Honestly one issue i have with dealing with it within OSL means that #1 will need to be finished before this part can be implemented, or we just mark it as non supported yet but planned in the future, for when #1 is done ?

It's probably easiest to just not implement required options until the GUI module is done, and dealing with it then? On the other hand, if we drop required options altogether, the world-creation-mod-config-screen-thing can be moved into a Mod Menu mod/module.

i guess ? i'm not sure how good of an idea that is, but it could work

SpaceWalkerRS commented 1 year ago

I think it'll be much easier to see what we need once we get started, so I just quickly threw some shit together in a new branch. It's all WIP so anything could still change. Feel free to pick it apart, change stuff, etc.

SRAZKVT commented 1 year ago

I think it'll be much easier to see what we need once we get started, so I just quickly threw some shit together in a new branch. It's all WIP so anything could still change. Feel free to pick it apart, change stuff, etc.

in libraries/config/src/main/java/net/ornithemc/osl/config/api/ConfigScope.java, i am not sure why GLOBAL has its path under .config, but WORLD has it under config. Is there any reason why the two are different ?

other than that, looks good to me, would be nice to have someone else's opinion though

SpaceWalkerRS commented 1 year ago

in libraries/config/src/main/java/net/ornithemc/osl/config/api/ConfigScope.java, i am not sure why GLOBAL has its path under .config, but WORLD has it under config. Is there any reason why the two are different ?

Just going off of memory, I thought the default config directory for Fabric was .config, so I adopted that for the global scope, but I felt like I didn't want to put the world configs in a hidden directory, so I went with config for the world scope. Should we adopt config for both scopes then?

SRAZKVT commented 1 year ago

in libraries/config/src/main/java/net/ornithemc/osl/config/api/ConfigScope.java, i am not sure why GLOBAL has its path under .config, but WORLD has it under config. Is there any reason why the two are different ?

Just going off of memory, I thought the default config directory for Fabric was .config, so I adopted that for the global scope, but I felt like I didn't want to put the world configs in a hidden directory, so I went with config for the world scope. Should we adopt config for both scopes then?

going with config for both would seem the best, as on Linux (used for servers quite a lot) it would allow people to easily see it through ls, which is always good

SpaceWalkerRS commented 1 year ago

going with config for both would seem the best, as on Linux (used for servers quite a lot) it would allow people to easily see it through ls, which is always good

alrighty, we'll go with config then

atm I'm thinking about serialization. I'm thinking there'll be at least two serializers, one JSON serializer (for saving to disk) and a serializer used for networking (will be handy for client-side GUI's to change server settings, for example). I'm hoping we can abstract it to the point where you can take any random config and any random serializer and it'll just work.

SRAZKVT commented 1 year ago

going with config for both would seem the best, as on Linux (used for servers quite a lot) it would allow people to easily see it through ls, which is always good

alrighty, we'll go with config then

atm I'm thinking about serialization. I'm thinking there'll be at least two serializers, one JSON serializer (for saving to disk) and a serializer used for networking (will be handy for client-side GUI's to change server settings, for example). I'm hoping we can abstract it to the point where you can take any random config and any random serializer and it'll just work.

sounds good

AdelinaM17n commented 1 year ago

I think it can be a single button for "mod configs" which leads to a screen that lists all mod configs, where you can click on them to go to another screen to change the values.

Would this be sort of like the interface mod menu provides? where aside from allowing access to the mod configs, information/description of the mod would also be shown? Oh I've read above, So that mod configs sections will be provided only on the world creation screen

SRAZKVT commented 1 year ago

I think it can be a single button for "mod configs" which leads to a screen that lists all mod configs, where you can click on them to go to another screen to change the values.

~Would this be sort of like the interface mod menu provides? where aside from allowing access to the mod configs, information/description of the mod would also be shown?~ Oh I've read above, So that mod configs sections will be provided only on the world creation screen

yeah, global configs should be just editable from mod menu, the question here was about world specific configs

SpaceWalkerRS commented 1 year ago

another neat idea (probably not high priority though):

Implement a versioning system for configs, as well as a way to update configs from old versions. This way old config files can still be read and don't corrupt the configs or cause the parser to poop the bed. Think of something like data fixers in Minecraft (though that system is probably way overkill for what we need).

SRAZKVT commented 1 year ago

Sounds overkill, would be better to allow devs to allow multiple configs for their mod, and just have multiple versions this way, deprecate the old version, and in later versions remove it. Unless you know of a case where that kind of solution wouldn't work ?

SpaceWalkerRS commented 1 year ago

No, that solution probably works for pretty much anything. But it's not very pretty to potentially end up with several almost-the-same config files.

It could be a fairly simple system. The simplest way I can think of is to pass the config version along to the option serializers:

if (version < 3) {
    // parse the super old way
} else if (version < 5) {
    // parse the old way
} else {
    // parse the new way
} 

Dealing with removed options would still be more complex though.

SRAZKVT commented 1 year ago

honestly i still think not bothering ourselves with it is basically the way to go. each mod will have their own way to deal with configuration versions, it's not either going to be such a huge need for any developer, as changing the way the config is agenced is quite rare, at most it gets updated to add new fields, and that's about it.

oh actually that's a good question, what about a field that isn't found in the config ? do we just assume it is of the option's default ?

SpaceWalkerRS commented 1 year ago

oh actually that's a good question, what about a field that isn't found in the config ? do we just assume it is of the option's default ?

probably - an idea I had was that all options are null by default, and after loading the config from disk any options that are still null are jus set to default, then when 'unloading' the config the options are set to null again.

SRAZKVT commented 1 year ago

oh actually that's a good question, what about a field that isn't found in the config ? do we just assume it is of the option's default ?

probably - an idea I had was that all options are null by default, and after loading the config from disk any options that are still null are jus set to default, then when 'unloading' the config the options are set to null again.

tbh i don't see why we would set a variable to non set if a default can be inferred, just write the default value then

SpaceWalkerRS commented 1 year ago

oh actually that's a good question, what about a field that isn't found in the config ? do we just assume it is of the option's default ?

probably - an idea I had was that all options are null by default, and after loading the config from disk any options that are still null are jus set to default, then when 'unloading' the config the options are set to null again.

tbh i don't see why we would set a variable to non set if a default can be inferred, just write the default value then

It would allow us to distinguish between unloaded options and default options. But yeah maybe that's not very useful xD

SRAZKVT commented 1 year ago

oh actually that's a good question, what about a field that isn't found in the config ? do we just assume it is of the option's default ?

probably - an idea I had was that all options are null by default, and after loading the config from disk any options that are still null are jus set to default, then when 'unloading' the config the options are set to null again.

tbh i don't see why we would set a variable to non set if a default can be inferred, just write the default value then

It would allow us to distinguish between unloaded options and default options. But yeah maybe that's not very useful xD

honestly i don't see a need for that, they are one and the same for me

SpaceWalkerRS commented 1 year ago

fair enough - might as well just reset all options before parsing the config file - that way if some options are missing from the file, those options are still set to default

SRAZKVT commented 1 year ago

anything more necessary for this API ?

Also, for the Network serializer, i propose NBT as a format, as it is basically already standard to send NBT through network in Minecraft, so just for consistency, it would make sense in my opinion.

SpaceWalkerRS commented 1 year ago

Also, for the Network serializer, i propose NBT as a format, as it is basically already standard to send NBT through network in Minecraft, so just for consistency, it would make sense in my opinion.

You may be right. All Vanilla packets are serialized straight to/from the PacketByteBuf. But whereas Vanilla has no incentive to keep the packet format forwards compatible, mods do, especially those that keep updating for a given Minecraft version.

anything more necessary for this API ?

I'm not sure. I think most important pieces are in place, at least for the API part. There's still stuff that needs to be done like saving configs. I suppose once we start shipping it and using it we'll notice if we're still missing something lol

SRAZKVT commented 1 year ago

Also, for the Network serializer, i propose NBT as a format, as it is basically already standard to send NBT through network in Minecraft, so just for consistency, it would make sense in my opinion.

You may be right. All Vanilla packets are serialized straight to/from the PacketByteBuf. But whereas Vanilla has no incentive to keep the packet format forwards compatible, mods do, especially those that keep updating for a given Minecraft version.

that is fair, what do you propose then ? abstracting out into a SendConfig packet that just sends the entire config of a mod to a client, and what is happening underneath may be unstable, and mirrors the target version's packet format ?

anything more necessary for this API ?

I'm not sure. I think most important pieces are in place, at least for the API part. There's still stuff that needs to be done like saving configs. I suppose once we start shipping it and using it we'll notice if we're still missing something lol

That works. I suppose then we should put OSL on the maven repository, and maybe have examples available, so that people don't need to read the code to understand how to use it ?

SpaceWalkerRS commented 1 year ago

Also, for the Network serializer, i propose NBT as a format, as it is basically already standard to send NBT through network in Minecraft, so just for consistency, it would make sense in my opinion.

You may be right. All Vanilla packets are serialized straight to/from the PacketByteBuf. But whereas Vanilla has no incentive to keep the packet format forwards compatible, mods do, especially those that keep updating for a given Minecraft version.

that is fair, what do you propose then ? abstracting out into a SendConfig packet that just sends the entire config of a mod to a client, and what is happening underneath may be unstable, and mirrors the target version's packet format ?

Sorry, I wasn't very clear. I think you're right that serializing to NBT is the right way to go for us, as we'll likely be updating the mod, and so serializing it this way helps keeping new versions of the mod (at least mostly) compatible with older ones.

anything more necessary for this API ?

I'm not sure. I think most important pieces are in place, at least for the API part. There's still stuff that needs to be done like saving configs. I suppose once we start shipping it and using it we'll notice if we're still missing something lol

That works. I suppose then we should put OSL on the maven repository, and maybe have examples available, so that people don't need to read the code to understand how to use it ?

Yup. Perhaps this should be discussed in a separate issue, but we need to decide how we're gonna do versioning, and how publishing is gonna be set up. And yeah we need to start writing documentation too.