10000 Address code review · CommandAPI/CommandAPI@4a36b1e · GitHub
[go: up one dir, main page]

Skip to content

Commit 4a36b1e

Browse files
committed
Address code review
This could also help with further rewriting to aid development for multiple platforms.
1 parent 259540b commit 4a36b1e

File tree

3 files changed

+52
-72
lines changed

3 files changed

+52
-72
lines changed

commandapi-platforms/commandapi-bukkit/commandapi-bukkit-core/src/main/java/dev/jorel/commandapi/config/CommentedConfigOption.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@
22

33
import java.util.List;
44

5-
record CommentedConfigOption<T>(List<String> comment, String path, T option) {
5+
record CommentedConfigOption<T>(List<String> comment, T option) {
66
}

commandapi-platforms/commandapi-bukkit/commandapi-bukkit-core/src/main/java/dev/jorel/commandapi/config/ConfigGenerator.java

Lines changed: 19 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
package dev.jorel.commandapi.config;
22

3-
import org.bukkit.configuration.ConfigurationSection;
43
import org.bukkit.configuration.InvalidConfigurationException;
54
import org.bukkit.configuration.file.YamlConfiguration;
65
import org.jetbrains.annotations.ApiStatus;
76

87
import java.util.ArrayList;
98
import java.util.HashSet;
109
import java.util.List;
10+
import java.util.Map;
1111
import java.util.Objects;
1212
import java.util.Set;
1313

@@ -19,13 +19,13 @@ private ConfigGenerator() {}
1919
public static YamlConfiguration generateDefaultConfig() throws InvalidConfigurationException {
2020
YamlConfiguration config = new YamlConfiguration();
2121
Set<String> sections = new HashSet<>();
22-
for (CommentedConfigOption<?> commentedConfigOption : DefaultedBukkitConfig.ALL_OPTIONS) {
23-
String path = commentedConfigOption.path();
22+
for (Map.Entry<String, CommentedConfigOption<?>> commentedConfigOption : DefaultedBukkitConfig.ALL_OPTIONS.entrySet()) {
23+
String path = commentedConfigOption.getKey();
2424

2525
tryCreateSection(config, path, sections);
2626

27-
config.set(path, commentedConfigOption.option());
28-
config.setComments(path, commentedConfigOption.comment());
27+
config.set(path, commentedConfigOption.getValue().option());
28+
config.setComments(path, commentedConfigOption.getValue().comment());
2929
}
3030
return process(config.saveToString());
3131
}
@@ -37,8 +37,8 @@ public static YamlConfiguration generateWithNewValues(YamlConfiguration existing
3737

3838
boolean wasConfigUpdated = false;
3939
Set<String> sections = new HashSet<>();
40-
for (CommentedConfigOption<?> commentedConfigOption : DefaultedBukkitConfig.ALL_OPTIONS) {
41-
String path = commentedConfigOption.path();
40+
for (Map.Entry<String, CommentedConfigOption<?>> commentedConfigOption : DefaultedBukkitConfig.ALL_OPTIONS.entrySet()) {
41+
String path = commentedConfigOption.getKey();
4242

4343
// Update config option
4444
if (existingConfig.contains(path)) {
@@ -47,7 +47,7 @@ public static YamlConfiguration generateWithNewValues(YamlConfiguration existing
4747
} else {
4848
wasConfigUpdated = true;
4949
tryCreateSection(config, path, sections);
50-
config.set(path, commentedConfigOption.option());
50+
config.set(path, commentedConfigOption.getValue().option());
5151
}
5252

5353
// Update config option comments
@@ -57,12 +57,12 @@ public static YamlConfiguration generateWithNewValues(YamlConfiguration existing
5757
// As a result, we wrap them in new ArrayLists first to be able to compare them
5858
List<String> existingComment = new ArrayList<>(existingConfig.getComments(path));
5959
existingComment.removeIf(Objects::isNull);
60-
List<String> defaultComment = new ArrayList<>(commentedConfigOption.comment());
60+
List<String> defaultComment = new ArrayList<>(commentedConfigOption.getValue().comment());
6161

6262
if (!existingComment.equals(defaultComment)) {
6363
wasConfigUpdated = true;
6464
}
65-
config.setComments(path, commentedConfigOption.comment());
65+
config.setComments(path, commentedConfigOption.getValue().comment());
6666
}
6767
if (shouldRemoveValues) {
6868
wasConfigUpdated = true;
@@ -93,11 +93,7 @@ private static void tryCreateSection(YamlConfiguration config, String path, Set<
9393
String sectionName = sectionNames[i];
9494
if (!existingSections.contains(sectionName)) {
9595
config.createSection(sectionName);
96-
for (CommentedConfigOption<?> commentedSection : DefaultedBukkitConfig.ALL_SECTIONS) {
97-
if (commentedSection.path().equals(sectionName)) {
98-
config.setComments(sectionName, commentedSection.comment());
99-
}
100-
}
96+
config.setComments(sectionName, DefaultedBukkitConfig.ALL_SECTIONS.get(sectionName).comment());
10197
}
10298
existingSections.add(sectionName);
10399
}
@@ -106,27 +102,18 @@ private static void tryCreateSection(YamlConfiguration config, String path, Set<
106102

107103
private static boolean shouldRemoveOptions(YamlConfiguration config) {
108104
Set<String> configOptions = config.getKeys(true);
109-
List<String> sections = new ArrayList<>();
110-
for (String configOption : configOptions) {
111-
ConfigurationSection section = config.getConfigurationSection(configOption);
112-
if (section != null) {
113-
sections.add(configOption);
114-
}
115-
}
116-
for (String sectionName : sections) {
117-
configOptions.remove(sectionName);
118-
}
119-
Set<String> defaultConfigOptions = new HashSet<>();
120-
for (CommentedConfigOption<?> defaultConfigOption : DefaultedBukkitConfig.ALL_OPTIONS) {
121-
defaultConfigOptions.add(defaultConfigOption.path());
122-
}
123-
List<String> optionsToRemove = new ArrayList<>();
105+
configOptions.removeIf(config::isConfigurationSection);
106+
107+
Set<String> defaultConfigOptions = DefaultedBukkitConfig.ALL_OPTIONS.keySet();
108+
109+
boolean shouldRemoveOptions = false;
124110
for (String option : configOptions) {
125111
if (!defaultConfigOptions.contains(option)) {
126-
optionsToRemove.add(option);
112+
shouldRemoveOptions = true;
113+
break;
127114
}
128115
}
129-
return !optionsToRemove.isEmpty();
116+
return shouldRemoveOptions;
130117
}
131118

132119
}
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,30 @@
11
package dev.jorel.commandapi.config;
22

3+
import org.jetbrains.annotations.ApiStatus;
4+
35
import java.util.ArrayList;
46
import java.util.List;
7+
import java.util.Map;
58

69
/**
710
* Default config values for the plugin's config.yml file
811
*/
9-
class DefaultedBukkitConfig {
12+
@SuppressWarnings("ClassEscapesDefinedScope")
13+
@ApiStatus.Internal
14+
public class DefaultedBukkitConfig {
1015

1116
public static final CommentedConfigOption<Boolean> VERBOSE_OUTPUTS = new CommentedConfigOption<>(
1217
List.of(
1318
"Verbose outputs (default: false)",
1419
"If \"true\", outputs command registration and unregistration logs in the console"
15-
),
16-
"verbose-outputs", false
20+
), false
1721
);
1822

1923
public static final CommentedConfigOption<Boolean> SILENT_LOGS = new CommentedConfigOption<>(
2024
List.of(
2125
"Silent logs (default: false)",
2226
"If \"true\", turns off all logging from the CommandAPI, except for errors."
23-
),
24-
"silent-logs", false
27+
), false
2528
);
2629

2730
public static final CommentedConfigOption<String> MISSING_EXECUTOR_IMPLEMENTATION = new CommentedConfigOption<>(
@@ -31,8 +34,7 @@ class DefaultedBukkitConfig {
3134
"parameters are:",
3235
" %s - the executor class (lowercase)",
3336
" %S - the executor class (normal case)"
34-
),
35-
"messages.missing-executor-implementation", "This command has no implementations for %s"
37+
), "This command has no implementations for %s"
3638
);
3739

3840
public static final CommentedConfigOption<Boolean> CREATE_DISPATCHER_JSON = new CommentedConfigOption<>(
@@ -41,8 +43,7 @@ class DefaultedBukkitConfig {
4143
"If \"true\", the CommandAPI creates a command_registration.json file showing the",
4244
"mapping of registered commands. This is designed to be used by developers -",
4345
"setting this to \"false\" will improve command registration performance."
44-
),
45-
"create-dispatcher-json", false
46+
), false
4647
);
4748

4849
public static final CommentedConfigOption<Boolean> USE_LATEST_NMS_VERSION = new CommentedConfigOption<>(
@@ -51,8 +52,7 @@ class DefaultedBukkitConfig {
5152
"If \"true\", the CommandAPI will use the latest available NMS implementation",
5253
"when the CommandAPI is used. This avoids all checks to see if the latest NMS",
5354
"implementation is actually compatible with the current Minecraft version."
54-
),
55-
"use-latest-nms-version", false
55+
), false
5656
);
5757

5858
public static final CommentedConfigOption<Boolean> BE_LENIENT_FOR_MINOR_VERSIONS = new CommentedConfigOption<>(
@@ -62,8 +62,7 @@ class DefaultedBukkitConfig {
6262
"For example, this setting may allow updating from 1.21.1 to 1.21.2 as only the minor version is changing",
6363
"but will not allow an update from 1.21.2 to 1.22.",
6464
"Keep in mind that implementations may vary and actually updating the CommandAPI might be necessary."
65-
),
66-
"be-lenient-for-minor-versions", false
65+
), false
6766
);
6867

6968
public static final CommentedConfigOption<Boolean> SHOULD_HOOK_PAPER_RELOAD = new CommentedConfigOption<>(
@@ -75,8 +74,7 @@ class DefaultedBukkitConfig {
7574
"function which allows CommandAPI commands to be used in datapacks.",
7675
"If you set this to false, CommandAPI commands may not work inside datapacks after",
7776
"reloading datapacks."
78-
),
79-
"hook-paper-reload", false
77+
), false
8078
);
8179

8280
public static final CommentedConfigOption<Boolean> SKIP_RELOAD_DATAPACKS = new CommentedConfigOption<>(
@@ -85,25 +83,22 @@ class DefaultedBukkitConfig {
8583
"If \"true\", the CommandAPI will not reload datapacks when the server has finished",
8684
"loading. Datapacks will still be reloaded if performed manually when \"hook-paper-reload\"",
8785
"is set to \"true\" and /minecraft:reload is run."
88-
),
89-
"skip-initial-datapack-reload", false
86+
), false
9087
);
9188

9289
public static final CommentedConfigOption<List<?>> PLUGINS_TO_CONVERT = new CommentedConfigOption<>(
9390
List.of(
9491
"Plugins to convert (default: [])",
9592
"Controls the list of plugins to process for command conversion."
96-
),
97-
"plugins-to-convert", new ArrayList<>()
93+
), new ArrayList<>()
9894
);
9995

10096
public static final CommentedConfigOption<List<String>> OTHER_COMMANDS_TO_CONVERT = new CommentedConfigOption<>(
10197
List.of(
10298
"Other commands to convert (default: [])",
10399
"A list of other commands to convert. This should be used for commands which",
104100
"are not declared in a plugin.yml file."
105-
),
106-
"other-commands-to-convert", new ArrayList<>()
101+
), new ArrayList<>()
107102
);
108103

109104
public static final CommentedConfigOption<List<String>> SKIP_SENDER_PROXY = new CommentedConfigOption<>(
@@ -112,34 +107,32 @@ class DefaultedBukkitConfig {
112107
"Determines whether the proxy sender should be skipped when converting a",
113108
"command. If you are having issues with plugin command conversion, add the",
114109
"plugin to this list."
115-
),
116-
"skip-sender-proxy", new ArrayList<>()
110+
), new ArrayList<>()
117111
);
118112

119-
public static final List<CommentedConfigOption<?>> ALL_OPTIONS = List.of(
120-
VERBOSE_OUTPUTS,
121-
SILENT_LOGS,
122-
MISSING_EXECUTOR_IMPLEMENTATION,
123-
CREATE_DISPATCHER_JSON,
124-
USE_LATEST_NMS_VERSION,
125-
BE_LENIENT_FOR_MINOR_VERSIONS,
126-
SHOULD_HOOK_PAPER_RELOAD,
127-
SKIP_RELOAD_DATAPACKS,
128-
PLUGINS_TO_CONVERT,
129-
OTHER_COMMANDS_TO_CONVERT,
130-
SKIP_SENDER_PROXY
113+
public static final Map<String, CommentedConfigOption<?>> ALL_OPTIONS = Map.ofEntries(
114+
Map.entry("verbose-outputs", VERBOSE_OUTPUTS),
115+
Map.entry("silent-logs", SILENT_LOGS),
116+
Map.entry("messages.missing-executor-implementation", MISSING_EXECUTOR_IMPLEMENTATION),
117+
Map.entry("create-dispatcher-json", CREATE_DISPATCHER_JSON),
118+
Map.entry("use-latest-nms-version", USE_LATEST_NMS_VERSION),
119+
Map.entry("be-lenient-for-minor-versions", BE_LENIENT_FOR_MINOR_VERSIONS),
120+
Map.entry("hook-paper-reload", SHOULD_HOOK_PAPER_RELOAD),
121+
Map.entry("skip-initial-datapack-reload", SKIP_RELOAD_DATAPACKS),
122+
Map.entry("plugins-to-convert", PLUGINS_TO_CONVERT),
123+
Map.entry("other-commands-to-convert", OTHER_COMMANDS_TO_CONVERT),
124+
Map.entry("skip-sender-proxy", SKIP_SENDER_PROXY)
131125
);
132126

133127
public static final CommentedConfigOption<?> SECTION_MESSAGE = new CommentedConfigOption<>(
134128
List.of(
135129
"Messages",
136130
"Controls messages that the CommandAPI displays to players"
137-
),
138-
"messages", null
131+
), null
139132
);
140133

141-
public static final List<CommentedConfigOption<?>> ALL_SECTIONS = List.of(
142-
SECTION_MESSAGE
134+
public static final Map<String, CommentedConfigOption<?>> ALL_SECTIONS = Map.of(
135+
"messages", SECTION_MESSAGE
143136
);
144137

145138
}

0 commit comments

Comments
 (0)
0