Address code reviews

Author: DerEchtePilz

README.md

@@ -414,7 +414,7 @@ This is the current roadmap for the CommandAPI (as of 30th April 2024):
             <td valign="top">???</td>
             <td valign="top">
                 <ul>
-                    <li>https://github.com/JorelAli/CommandAPI/pull/594 Adds a config option to allow the CommandAPI to be more lenient when updating to a new minor version (e. g. from 1.21 to 1.21.1)</li>
+                    <li>https://github.com/JorelAli/CommandAPI/pull/594 Adds a config option to allow the CommandAPI to be more lenient when updating to a new minor version (e.g. from 1.21 to 1.21.1)</li>
                 </ul>
             </td>
         </tr>

commandapi-core/src/main/java/dev/jorel/commandapi/CommandAPI.java

@@ -112,6 +112,11 @@ public static void onLoad(CommandAPIConfig<?> config) {
 			CommandAPIPlatform<?, ?, ?> platform = CommandAPIVersionHandler.getPlatform();
 			new CommandAPIHandler<>(platform);
 
+			if (CommandAPI.getConfiguration().shouldUseLatestNMSVersion() || CommandAPI.getConfiguration().shouldBeLenientForMinorVersions()) {
+				CommandAPI.logWarning("Loading the CommandAPI with a potentially incompatible NMS implementation.");
+				CommandAPI.logWarning("While you may find success with this, further updates might be necessary to fully support the version you are using.");
+			}
+
 			// Log platform load
 			final String platformClassHierarchy;
 			{

commandapi-core/src/main/java/dev/jorel/commandapi/CommandAPIConfig.java

@@ -39,7 +39,7 @@ public abstract class CommandAPIConfig<Impl
 	boolean verboseOutput = false;
 	boolean silentLogs = false;
 	boolean useLatestNMSVersion = false;
-	boolean lenientForMinorVersions = false;
+	boolean beLenientForMinorVersions = false;
 	String missingExecutorImplementationMessage = "This command has no implementations for %s";
 
 	File dispatcherFile = null;
@@ -93,7 +93,7 @@ public Impl useLatestNMSVersion(boolean value) {
 
 	/**
 	 * Sets whether the CommandAPI should load a (potentially unsupported) NMS version
-	 * when updating to a minor release of Minecraft, for example this setting would
+	 * when updating to a minor release of Minecraft. For example, this setting would
 	 * allow running a minor version of the game before the CommandAPI makes a proper
 	 * release for it. Unlike {@link #useLatestNMSVersion(boolean)}, this setting does
 	 * not blindly load the latest NMS version and as a result break cross version compatibility
@@ -102,8 +102,8 @@ public Impl useLatestNMSVersion(boolean value) {
 	 * @param value whether the CommandAPI should assume that minor Minecraft releases do not cause incompatibilities
 	 * @return this CommandAPIConfig
 	 */
-	public Impl lenientForMinorVersions(boolean value) {
-		this.lenientForMinorVersions = value;
+	public Impl beLenientForMinorVersions(boolean value) {
+		this.beLenientForMinorVersions = value;
 		return instance();
 	}
 

commandapi-core/src/main/java/dev/jorel/commandapi/InternalConfig.java

@@ -40,7 +40,7 @@ public class InternalConfig {
 	// Whether we should use the latest NMS version (which may not be compatible)
 	private final boolean useLatestNMSVersion;
 
-	private final boolean beMoreLenientForMinorVerions;
+	private final boolean beLenientForMinorVersions;
 
 	// The message to display when an executor implementation is missing
 	private final String messageMissingExecutorImplementation;
@@ -67,7 +67,7 @@ public InternalConfig(CommandAPIConfig<?> config) {
 		this.verboseOutput = config.verboseOutput;
 		this.silentLogs = config.silentLogs;
 		this.useLatestNMSVersion = config.useLatestNMSVersion;
-		this.beMoreLenientForMinorVerions = config.lenientForMinorVersions;
+		this.beLenientForMinorVersions = config.beLenientForMinorVersions;
 		this.messageMissingExecutorImplementation = config.missingExecutorImplementationMessage;
 		this.dispatcherFile = config.dispatcherFile;
 		this.skipSenderProxy = config.skipSenderProxy;
@@ -100,8 +100,8 @@ public boolean shouldUseLatestNMSVersion() {
 	/**
 	 * @return Whether the CommandAPI should assume that minor versions of officially unsupported versions do not cause incompatibilities
 	 */
-	public boolean shouldBeMoreLenientForMinorVersions() {
-		return this.beMoreLenientForMinorVerions;
+	public boolean shouldBeLenientForMinorVersions() {
+		return this.beLenientForMinorVersions;
 	}
 
 	/**

commandapi-platforms/commandapi-bukkit/commandapi-bukkit-plugin-mojang-mapped/src/main/java/dev/jorel/commandapi/CommandAPIMain.java

@@ -52,7 +52,7 @@ public void onLoad() {
 			.dispatcherFile(fileConfig.getBoolean("create-dispatcher-json") ? new File(getDataFolder(), "command_registration.json") : null)
 			.shouldHookPaperReload(fileConfig.getBoolean("hook-paper-reload"))
 			.skipReloadDatapacks(fileConfig.getBoolean("skip-initial-datapack-reload"))
-			.lenientForMinorVersions(fileConfig.getBoolean("lenient-for-minor-versions"));
+			.beLenientForMinorVersions(fileConfig.getBoolean("lenient-for-minor-versions"));
 
 		for (String pluginName : fileConfig.getStringList("skip-sender-proxy")) {
 			if (Bukkit.getPluginManager().getPlugin(pluginName) != null) {

commandapi-platforms/commandapi-bukkit/commandapi-bukkit-plugin-mojang-mapped/src/main/resources/config.yml

@@ -44,8 +44,8 @@ use-latest-nms-version: false
 # Be lenient with version checks when loading for new minor Minecraft versions (default: false)
 # If "true", the CommandAPI loads NMS implementations for (potentially unsupported) Minecraft versions.
 # For example, this setting may allow updating from 1.21.1 to 1.21.2 as only the minor version is changing
-# but will not allow an update from 1.21.2 to 1.22
-# Keep in mind that implementations my vary and actually updating the CommandAPI might be necessary.
+# but will not allow an update from 1.21.2 to 1.22.
+# Keep in mind that implementations may vary and actually updating the CommandAPI might be necessary.
 lenient-for-minor-versions: false
 
 # Hook into Paper's ServerResourcesReloadedEvent (default: true)

commandapi-platforms/commandapi-bukkit/commandapi-bukkit-plugin/src/main/java/dev/jorel/commandapi/CommandAPIMain.java

@@ -52,7 +52,7 @@ public void onLoad() {
 			.dispatcherFile(fileConfig.getBoolean("create-dispatcher-json") ? new File(getDataFolder(), "command_registration.json") : null)
 			.shouldHookPaperReload(fileConfig.getBoolean("hook-paper-reload"))
 			.skipReloadDatapacks(fileConfig.getBoolean("skip-initial-datapack-reload"))
-			.lenientForMinorVersions(fileConfig.getBoolean("lenient-for-minor-versions"));
+			.beLenientForMinorVersions(fileConfig.getBoolean("lenient-for-minor-versions"));
 
 		for (String pluginName : fileConfig.getStringList("skip-sender-proxy")) {
 			if (Bukkit.getPluginManager().getPlugin(pluginName) != null) {

commandapi-platforms/commandapi-bukkit/commandapi-bukkit-plugin/src/main/resources/config.yml

@@ -44,8 +44,8 @@ use-latest-nms-version: false
 # Be lenient with version checks when loading for new minor Minecraft versions (default: false)
 # If "true", the CommandAPI loads NMS implementations for (potentially unsupported) Minecraft versions.
 # For example, this setting may allow updating from 1.21.1 to 1.21.2 as only the minor version is changing
-# but will not allow an update from 1.21.2 to 1.22
-# Keep in mind that implementations my vary and actually updating the CommandAPI might be necessary.
+# but will not allow an update from 1.21.2 to 1.22.
+# Keep in mind that implementations may vary and actually updating the CommandAPI might be necessary.
 lenient-for-minor-versions: false
 
 # Hook into Paper's ServerResourcesReloadedEvent (default: true)

commandapi-platforms/commandapi-bukkit/commandapi-bukkit-vh/src/main/java/dev/jorel/commandapi/CommandAPIVersionHandler.java

@@ -49,13 +49,12 @@ public interface CommandAPIVersionHandler {
 	 * @return an instance of NMS which can run on the specified Minecraft version
 	 */
 	static CommandAPIPlatform<?, ?, ?> getPlatform() {
-		CommandAPIPlatform<?, ?, ?> latestNMS = new NMS_1_21_R1();
+		int latestMajorVersion = 21; // Change this for Minecraft's major update
 		if (CommandAPI.getConfiguration().shouldUseLatestNMSVersion()) {
-			return latestNMS;
+			return new NMS_1_21_R1();
 		} else {
 			String version = Bukkit.getBukkitVersion().split("-")[0];
-			Version minecraftVersion = new Version(version);
-			CommandAPIPlatform<?, ?, ?> platform = switch (version) {
+			return switch (version) {
 				case "1.16.5" -> new NMS_1_16_R3();
 				case "1.17" -> new NMS_1_17();
 				case "1.17.1" -> new NMS_1_17_R1();
@@ -70,54 +69,16 @@ public interface CommandAPIVersionHandler {
 				case "1.20.3", "1.20.4" -> new NMS_1_20_R3();
 				case "1.20.5", "1.20.6" -> new NMS_1_20_R4();
 				case "1.21", "1.21.1" -> new NMS_1_21_R1();
-				default -> null;
-			};
-			if (platform != null) {
-				return platform;
-			} else {
-				if (CommandAPI.getConfiguration().shouldBeMoreLenientForMinorVersions()) {
-					return minecraftVersion.matchesPatch((NMS<?>) latestNMS);
-				}
-				throw new UnsupportedVersionException(version);
-			}
-		}
-	}
-
-	class Version {
-
-		private final String version;
-
-		// The minor version is something like 1.x
-		private final int minor;
-
-		private Version(String version) {
-			this.version = version;
-			String[] versionParts = version.split("\\.");
-			minor = Integer.parseInt(versionParts[1]);
-		}
-
-		private CommandAPIPlatform<?, ?, ?> matchesPatch(NMS<?> latestNMS) {
-			if (minor <= 16) {
-				// We absolutely do not support versions older than 1.16.5
-				// As we match 1.16.5 in the getPlatform() method, this if branch means that
-				// the server is on 1.16.4 or older
-				throw new UnsupportedVersionException(version);
-			} else {
-				// Any other Minecraft that is older or exactly the last version we match in getPlatform()
-				// does not appear here. Furthermore, we do not want to be lenient with 1.x versions so we
-				// can simply return the latest NMS version here if its supported Minecraft version's minor version
-				// is the same as the current Minecraft version
-				String[] supportedNMSVersions = latestNMS.compatibleVersions();
-				int minorVersion = Integer.parseInt(supportedNMSVersions[0].split("\\.")[1]); // Index 1 returns the minor version by SemVer's rules
-				if (this.minor == minorVersion) {
-					return (CommandAPIPlatform<?, ?, ?>) latestNMS;
+				default -> {
+					int currentMajorVersion = Integer.parseInt(version.split("\\.")[1]);
+					if (CommandAPI.getConfiguration().shouldBeLenientForMinorVersions() && latestMajorVersion == currentMajorVersion) {
+						yield new NMS_1_21_R1();
+					} else {
+						throw new UnsupportedVersionException(version);
+					}
 				}
-			}
-			// If we end up here, that means the server's current minor version in SemVer terms is newer than the
-			// latest minor version of Minecraft in SemVer terms that this method allows. We do not support that.
-			throw new UnsupportedVersionException(version);
+			};
 		}
-
 	}
 
 }

docssrc/src/config.md

@@ -132,6 +132,18 @@ Take the warning from the [`use-latest-nms-version`](#use-latest-nms-version) an
 
 </div>
 
+**Default value**
+
+```yml
+lenient-for-minor-versions: false
+```
+
+**Example value**
+
+```yml
+lenient-for-minor-versions: true
+```
+
 -----
 
 ### `hook-paper-reload`