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
Extend plugin configuration docs. Fixes #735. Fixes #553 #766
Conversation
A preview for this pull request is available at https://cdn.rawgit.com/Spongy/SpongeDocs-PRs/ccf2104/. Here are some links to the pages that were modified:
Since the preview frequently changes, please link to this comment, not to the direct url to the preview. |
* Improve loading a default config from the plugin jar file example * Mention also mention the other supported file formats in the loaders page * Mention `@DefaultConfig` in conjunction with `ConfigurationLoader`s
@Simon_Flash mentioned on Discord that there is a different (recommended) way of checking whether the config is empty. Solution 1A ( suggested ) @Inject @DefaultConfig(sharedRoot = true)
private Path configPath;
@Inject @DefaultConfig(sharedRoot = true)
private ConfigurationLoader<CommentedConfigurationNode> configManager;
if (Files.exists(configPath)) {
rootNode = this.configManager.load();
} else {
this.logger.info("No config found - loading default");
URL jarConfigFile = Sponge.getAssetManager().getAsset("defaultConfig.conf").get().getUrl();
ConfigurationLoader<CommentedConfigurationNode> defaultLoader =
HoconConfigurationLoader.builder().setURL(jarConfigFile).build();
rootNode = defaultLoader.load();
} Solution 1B ( not tested ) @Inject @DefaultConfig(sharedRoot = true)
private Path configPath;
@Inject @DefaultConfig(sharedRoot = true)
private ConfigurationLoader<CommentedConfigurationNode> configManager;
if (!Files.exists(configPath)) {
this.logger.info("No config found - loading default");
Asset jarConfigFile = Sponge.getAssetManager().getAsset("defaultConfig.conf").get();
jarConfigFile.copyToFile(configPath);
}
rootNode = this.configManager.load(); Solution 2 @Inject @DefaultConfig(sharedRoot = true)
private ConfigurationLoader<CommentedConfigurationNode> configManager;
rootNode = this.configManager.load();
if (!rootNode.hasMapChildren()) {
this.logger.info("No config found - loading default");
final URL jarConfigFile = Sponge.getAssetManager().getAsset("defaultConfig.conf").get().getUrl();
final ConfigurationLoader<CommentedConfigurationNode> defaultLoader =
HoconConfigurationLoader.builder().setURL(jarConfigFile).build();
rootNode = defaultLoader.load();
} I prefer the later variant as
Which variant should we use in the docs as the recommended way? |
Simon_Flash's reasons for suggesting
|
The main work for this change has been done. So we can start the review period. There are two things that are up for/need discussion though:
|
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 note you often use "it is recommended". Generally we're allowed to refer to the docs using the "royal we", so it might be better to use "We recommend" in the long run. It takes ownership of such recommendations, and people can always quibble it with a PR if they can demonstrate a better recommendation to adopt.
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.
Really minor stuff.
Some last change requests? |
Co-Authored-By: ST-DDT <ST-DDT@gmx.de>
Fixes #735
Fixes #553
Tasks
TypeTokens
which contains already existing type tokens and mention that the user should possibly should use a similar approach if he has multiple type tokens itself.Set
s? is the order retained? sorted?textserialization format does SpongeAPI use in Configurate by default?@ConfigSerializable
instance to Using Objectmappers.@Setting
on a fieldTBD
Text
s.