-
-
Notifications
You must be signed in to change notification settings - Fork 56
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
Configuration system #127
base: main
Are you sure you want to change the base?
Configuration system #127
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution!
My general comments is that everything is shallowly explained. The overview attempts to explain too much too quickly, and the example seems strange in general. I believe you should think of the docs similar to a textbook introduction and how they would explain a concept.
That's not to say the organization is bad, it's just trying to do too much at once.
@@ -21,3 +21,5 @@ | |||
npm-debug.log* | |||
yarn-debug.log* | |||
yarn-error.log* | |||
|
|||
.project |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Eclipse .project file. Don't want to check that in by accident.
Yes, Eclipse also has a markdown editor.
Configurations define settings and consumer preferences that can be applied to a mod instance. NeoForge uses a configuration system using [TOML][toml] files and read with [NightConfig][nightconfig]. | ||
## Overview | ||
|
||
In its base form, a configuration is a persistent collection of named values that can be changed by the user. Neoforge provides a configuration system that takes care of the "persistent" and "can be changed by the user" parts, as well as more Minecraft-specific tasks like syncing configs from the server to the client. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence feels a tiny bit misleading since not all configs are synced from the server to the client. Since humans tend to remember the first thing they read, I would rephrase it to not make any assumptions about the underlying system, especially since you explain it later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about a "some" between "syncing" and "configs"? That's the shortest thing I can think of to break that assumption.
|
||
In its base form, a configuration is a persistent collection of named values that can be changed by the user. Neoforge provides a configuration system that takes care of the "persistent" and "can be changed by the user" parts, as well as more Minecraft-specific tasks like syncing configs from the server to the client. | ||
|
||
Configuration values in this system have a name (called "key" in the code) and are part of a section, i.e. a named collection of values. They also have a data type, which may be `String`, `Integer`, `Long`, `Double`, `Enum` or `List` (of all of these but Enum). In addition, you can define a translation key that is used by the UI to show the name of the value, a comment that is added to the file the configuration is saved in, a tooltip for the UI (this one defaults to the comment), and further restrictions on the value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is a section here? What does 'of all of these but Enum' mean? This feels like a simplifying line which tries to sum up every feature within a configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That belongs to List, those are the types that work in lists (i.e. are read back the same from the toml---you can stuff in anything but then you get its toString() back). But I agree, listing all the data types here is too early, this should go in a lower section.
|
||
Configuration values in this system have a name (called "key" in the code) and are part of a section, i.e. a named collection of values. They also have a data type, which may be `String`, `Integer`, `Long`, `Double`, `Enum` or `List` (of all of these but Enum). In addition, you can define a translation key that is used by the UI to show the name of the value, a comment that is added to the file the configuration is saved in, a tooltip for the UI (this one defaults to the comment), and further restrictions on the value. | ||
|
||
Sections can be nested to any depth, allowing you to build a tree to your liking. However, you don't just have one root; you have 4. Those represent the four different types of config: startup, client, server and common. Each is stored in its own file. The startup config is loaded early in the startup process, the client config only is loaded on the physical client, the common config is loaded on both client and server independently, and the server config is loaded on the server when a world is loaded and is synced over the network to the client. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, are you referring to TOML maps for sections? This overview feels like its attempting to explain too much in so little.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I probably can thin the overview out once the detailed sections present the data in a nicer form.
|
||
Sections can be nested to any depth, allowing you to build a tree to your liking. However, you don't just have one root; you have 4. Those represent the four different types of config: startup, client, server and common. Each is stored in its own file. The startup config is loaded early in the startup process, the client config only is loaded on the physical client, the common config is loaded on both client and server independently, and the server config is loaded on the server when a world is loaded and is synced over the network to the client. | ||
|
||
For this to work, NeoForge's configuration system naturally needs to know about your configuration values. To accomplish this, you first acquire a `Builder`, then tell that Builder about all your configuration values. For each one, the Builder will give you a provider you can later use to get the current value. When you're done, the Builder will give you a `ModConfigSpec` object that you need to register with the configuration system. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What? I understand what the code is doing here, but this is way to glossy to understand what's going on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is basically the "top of the man page you come back to to copy the correct syntax that you understand because you read the whole page ages ago" part. The text form is very high-level and only points you in the right direction, either by giving you the keywords to remember what you read before, or by preparing your mind for the concepts that will be explained later. The code is pretty much copy&paste-ready.
public ExampleMod(ModContainer modContainer) { | ||
modContainer.registerConfig(ModConfig.Type.COMMON, Config.SPEC); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, there's a lot going on here that isn't explained.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what later sections with all the details are there for (i.e. those that I haven't touched yet).
If you want to use NeoForge's configuration UI, you need to register for it like this: | ||
|
||
```java | ||
@Mod(ExampleMod.MODID, dist = Dist.CLIENT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey look, why should it only be on the client? Explain.
} | ||
``` | ||
|
||
Note that the value you get is live. It can change at any time, for example, when a config file is changed and reloaded or when the user uses the configuration UI. If you need to rely on some config values to stay stable and/or in sync, you need to make a copy of the values. This can also be used to transform values that cannot be stored directly, e.g. converting `String`s first to `ResourceLocation`s and then to `Item`s. To update your copy, listen to the `ModConfigEvent`s on the mod event bus. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a good way of explaining the configuration value. The value is cached when get is first called and then invalidated on load and reload. So, you don't really need to make a copy unless you are not using the config for its intended purpose. And why are you transforming values in configs to Item
s? That feels odd since that would generally make sense as part of a datapack or something else.
Also what do the events do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's why I'm explaining that people don't need to copy the values out of the Supplier unless they have a very special need.
And that transformation example is a straight reference to the MDK again
And about datapacks...I have a meme for you:
Players cannot make datapacks. They can't even start a game without using a launcher that autostarts with the system. There even is a mod out there that changes a config value in Ender IO that is available in the ingame config---just because players are not even able to use the config menu. Um, sorry, I'm starting to rant slightly off-topic.
@@ -51,6 +110,11 @@ The `ConfigValue` specific methods take in two additional components: | |||
- A validator to make sure the deserialized object is valid | |||
- A class representing the data type of the config value | |||
|
|||
For lists, there are two additional components you can supply: | |||
|
|||
- A supplier for new elements. This is used by the UI to allow the user adding new elements. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems wrong. The supplier past in is the default list. Are you talking about the default value in a list of acceptable values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is new with neoforged/NeoForge#1199
Sorry, should have linked that PR at the top, but it was kinda late and I was tired.
For lists, there are two additional components you can supply: | ||
|
||
- A supplier for new elements. This is used by the UI to allow the user adding new elements. | ||
- A size range to limit the size of the list, e.g. to require them to have at least one entry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you talking about defineInRange
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR1199, too
Yes, I only made the new introduction sections so far and haven't touched the detailed sections below. The introduction can be thinned out of details once I got the other sections, but I at least want to touch the important data points up-front to. That way someone who only reads the overview has enough of an idea to work with the system and do "the 90%" when they start with the MDK. "the 90%": 90% of the mods and 90% of the config settings of the other 10% will have the form of a couple of bools and ints with maybe one or two sections for organisation. In my opinion, everything you need to do this correctly should be a 5-minute read and the MDK as a starting point. The rest of the documentation page should then cover all the "Can I do X?" and "How do I do Y?" questions but not be a requirement for the typical "feature_A_enabled = false" configs. Note that I have mostly answered questions and explained why I did certain things, but I have not commented which suggestions I will incorporate. So don't take a lack of that as rejection. |
I started a rewrite on the configuration system to include the new UI (PR pending), and to make the article more accessible.
Comments welcome.
PS: Is the method using
configure
still important? It looks overly complicated with no real value to me, so I'm tempted to drop it from the documentation.