Kryptonite 2.1.0 #58
Open
Kryptonite 2.1.0 #58
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reset NU
|
Thank you for opening a pull request to Essence. You've requested to merge something into We unfortunately do not accept any pull requests into Thank you! |
Dependency ReviewThe following issues were found:
License Issuespom.xml
OpenSSF Scorecard
Scanned Files
|
WalkthroughIntroduces a generic config framework (GenericConfigItem and typed items), adds Minecraft and Leaf config collections, migrates logging to Foundry Logger, updates startup to initialize FoundryConfig/Logger, replaces ServerProperties with MinecraftConfig across code and GUIs, adds a unified KOS_ConfigItemGui, deprecates several legacy classes/GUI pages, updates build/shade/repos, and bumps API/versioning. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Server as Bukkit Server
participant P as Kryptonite Plugin
participant F as FoundryConfig
participant L as Logger
participant SD as SoftwareUtil
Server->>P: onEnable()
P->>F: new FoundryConfig()\nsetVerbose(...)\nsetPluginId(...)
P->>L: new Logger(F)
P->>SD: checkSoftware()
SD-->>P: Supported configurations (MINECRAFT, LEAF, ...)
P-->>Server: Startup complete
note over P,L: Logging now via Foundry Logger
sequenceDiagram
autonumber
actor Admin as Player
participant GUI as KOS_ConfigItemGui
participant CC as ConfigCollection\n(MinecraftConfig/LeafConfig)
participant Item as GenericConfigItem<T>
participant IO as YAML/.properties
Admin->>GUI: Open (type = MINECRAFT/LEAF)
GUI->>CC: Initialize values
loop For each item
GUI->>Item: buildDisplayItem()
Item->>IO: loadFile() and read
IO-->>Item: current value
Item-->>GUI: metadata (valid/ideal/value)
end
Admin->>GUI: Click item (toggle/adjust)
GUI->>Item: setValue(newValue)
Item->>IO: write and save
GUI->>Admin: Refresh view
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_2.java (1)
139-146: Bug: wrong key used for Wither Skull save limit.Reads TRIDENT instead of WITHER_SKULL; GUI shows/edits inconsistent value.
- int value = this.paperWorld.getInt(PaperWorld.Key.ENTITY_PER_CHUNK_SAVE_LIMIT_TRIDENT); + int value = this.paperWorld.getInt(PaperWorld.Key.ENTITY_PER_CHUNK_SAVE_LIMIT_WITHER_SKULL);src/main/java/net/lewmc/kryptonite/kos/gui/KOS_BukkitGui.java (1)
404-437: Range checks: “too low” thresholds use 5 but ideal range starts at 100.Else-if branches should use
< 100to classify “too low”; current logic mislabels 5–99 as “too high”.- } else if (value < 5) { + } else if (value < 100) {(Apply to all five methods above.)
Also applies to: 439-472, 474-507, 509-542, 544-577
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_1.java (1)
102-145: Bugs: String comparison and unsafe cast in delayChunkUnloadsBy.
- Use
.equalsfor String comparison.- Don’t cast String to int; handle Integer or parse String safely.
- if (value instanceof String && value == "default") { + if ("default".equals(value)) { ... - } else if (value instanceof String && value != "default") { - this.gui.addElement(new StaticGuiElement(id, - new ItemStack(Material.WHITE_CONCRETE), - 1, - click -> this.setInt(PaperWorld.Key.DELAY_CHUNK_UNLOADS_BY, click, (int) value, true), + } else if (value instanceof Integer) { + this.gui.addElement(new StaticGuiElement(id, + new ItemStack(Material.WHITE_CONCRETE), + 1, + click -> this.setInt(PaperWorld.Key.DELAY_CHUNK_UNLOADS_BY, click, (Integer) value, true), ChatColor.WHITE + "Delay Chunk Unloads By", ChatColor.GRAY + String.valueOf(value), ChatColor.GRAY + "'default' (lowest value) is ideal for most servers.", ChatColor.BLUE + "Right click to increase - left click to decrease." )); + } else if (value instanceof String) { + try { + int parsed = Integer.parseInt((String) value); + this.gui.addElement(new StaticGuiElement(id, + new ItemStack(Material.WHITE_CONCRETE), + 1, + click -> this.setInt(PaperWorld.Key.DELAY_CHUNK_UNLOADS_BY, click, parsed, true), + ChatColor.WHITE + "Delay Chunk Unloads By", + ChatColor.GRAY + String.valueOf(value), + ChatColor.GRAY + "'default' (lowest value) is ideal for most servers.", + ChatColor.BLUE + "Right click to increase - left click to decrease." + )); + } catch (NumberFormatException ignored) { + // fall through to barrier below + }src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_5.java (1)
75-89: Hotkey collision with KOS_GuiConstants; Exit ('v') is overridden by “Previous page”.
KOS_GuiConstants now uses t/u/v; this page still uses 'v' for “Previous page” and the layout lacks 't'/'u'. Result: Exit is hidden and Help/Alerts (restartRequired=true) won’t render.Apply this to align with constants and avoid collisions:
- this.gui.addElement(new StaticGuiElement('v', + this.gui.addElement(new StaticGuiElement('y', new ItemStack(Material.OAK_SIGN), 1, click -> { click.getGui().close(); KOS_PaperWorld_4 prevGui = new KOS_PaperWorld_4(this.plugin, this.user); prevGui.show(); return true; }, ChatColor.WHITE + "Previous page" ));- return new String[]{ - "abcdefghi", - "jklmnopqr", - "v w x y z" - }; + return new String[]{ + "abcdefghi", + "jklmnopqr", + "t x v y z" + };Also applies to: 95-102
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_3.java (1)
85-108: Hotkey mismatch with KOS_GuiConstants; Exit/help/alerts may not render; 'v' conflict.
Constants moved to t/u/v; this page still uses 'v' for Previous and lacks 't'/'u' in layout.- this.gui.addElement(new StaticGuiElement('v', + this.gui.addElement(new StaticGuiElement('y', new ItemStack(Material.OAK_SIGN), ... - return new String[]{ - "abcdefghi", - "jklmnopqr", - "v w x y z" - }; + return new String[]{ + "abcdefghi", + "jklmnopqr", + "t x v y z" + };Also applies to: 116-121
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PufferfishGui.java (2)
72-79: Constants layout out-of-sync (missing t/u/v).
With KOS_GuiConstants using t/u/v, this GUI won’t show Help/Alerts(true)/Exit unless slots exist.- return new String[]{ - "abcdefgh ", - " ", - " w x y " - }; + return new String[]{ + "abcdefgh ", + " ", + " t x v " + };
118-150: Toggle handlers set the same value in both branches; options cannot be turned off.
E.g., when DAB is true, clicking sets true again.Apply these fixes to toggle correctly:
@@ - if (value) { + if (value) { this.gui.addElement(new StaticGuiElement(id, new ItemStack(Material.LIME_CONCRETE), 1, click -> { click.getGui().close(); - this.pufferfish.setBoolean(Pufferfish.Key.DAB_ENABLED, true); + this.pufferfish.setBoolean(Pufferfish.Key.DAB_ENABLED, false); this.show(); return true; }, @@ - } else { + } else { this.gui.addElement(new StaticGuiElement(id, new ItemStack(Material.RED_CONCRETE), 1, click -> { click.getGui().close(); this.pufferfish.setBoolean(Pufferfish.Key.DAB_ENABLED, true); this.show(); return true; },@@ - this.pufferfish.setBoolean(Pufferfish.Key.ENABLE_ASYNC_MOB_SPAWNING, true); + this.pufferfish.setBoolean(Pufferfish.Key.ENABLE_ASYNC_MOB_SPAWNING, false); @@ - this.pufferfish.setBoolean(Pufferfish.Key.ENABLE_SUFFOCATION_OPTIMIZATION, true); + this.pufferfish.setBoolean(Pufferfish.Key.ENABLE_SUFFOCATION_OPTIMIZATION, false); @@ - this.pufferfish.setBoolean(Pufferfish.Key.INACTIVE_GOAL_SELECTOR_THROTTLE, true); + this.pufferfish.setBoolean(Pufferfish.Key.INACTIVE_GOAL_SELECTOR_THROTTLE, false); @@ - this.pufferfish.setBoolean(Pufferfish.Key.DISABLE_METHOD_PROFILER, true); + this.pufferfish.setBoolean(Pufferfish.Key.DISABLE_METHOD_PROFILER, false);Also applies to: 229-261, 264-296, 299-333, 335-369
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_GuiConstants.java (1)
31-44: Hotkey migration breaks existing GUIs; provide backward‑compat or update all layouts.
Help moved to 't', Alerts(true) to 'u', Exit to 'v', but many GUIs still place these at w/x/y, and some use 'v' for navigation.Two options:
- Short term (recommended): add elements at both old and new keys to avoid regressions.
- Long term: update all GUIs’ layouts to include t/u/v and stop using 'v' for navigation.
Back‑compat patch:
- this.gui.addElement(new StaticGuiElement('t', + this.gui.addElement(new StaticGuiElement('t', new ItemStack(Material.BOOK), ... + // Legacy position + this.gui.addElement(new StaticGuiElement('w', + new ItemStack(Material.BOOK), 1, click -> { + click.getWhoClicked().sendMessage(ChatColor.YELLOW + "https://wiki.lewmc.net/kr-kos-guide.html"); + click.getGui().close(); + return true; + }, + ChatColor.DARK_GREEN + "Help", + ChatColor.GREEN + "Not sure what to do? Click here for", + ChatColor.GREEN + "a link to our Wiki for help." + ));- if (plugin.restartRequired) { - this.gui.addElement(new StaticGuiElement('u', + if (plugin.restartRequired) { + this.gui.addElement(new StaticGuiElement('u', new ItemStack(Material.YELLOW_CONCRETE), ... - } else { + // Legacy position for alerts + this.gui.addElement(new StaticGuiElement('x', + new ItemStack(Material.YELLOW_CONCRETE), 1, click -> true, + ChatColor.GOLD + "Alerts", + ChatColor.YELLOW + "Restart Required - You must restart your", + ChatColor.YELLOW + "server for changes to be applied." + )); + } else { this.gui.addElement(new StaticGuiElement('x',- this.gui.addElement(new StaticGuiElement('v', + this.gui.addElement(new StaticGuiElement('v', new ItemStack(Material.OAK_DOOR), ... ChatColor.DARK_RED + "Exit" )); + // Legacy position + this.gui.addElement(new StaticGuiElement('y', + new ItemStack(Material.OAK_DOOR), 1, click -> { click.getGui().close(); return true; }, + ChatColor.DARK_RED + "Exit" + ));Also applies to: 45-54, 63-72
src/main/java/net/lewmc/kryptonite/utils/UpdateUtil.java (1)
35-61: Update check does blocking I/O without timeouts and leaks the Scanner.
url.openStream()has no connect/read timeouts and can hang the server duringonEnable(). Also,Scannerisn’t closed. UseHttpURLConnectionwith timeouts and try‑with‑resources.Apply this diff:
--- a/src/main/java/net/lewmc/kryptonite/utils/UpdateUtil.java +++ b/src/main/java/net/lewmc/kryptonite/utils/UpdateUtil.java @@ import com.tchristofferson.configupdater.ConfigUpdater; import net.lewmc.foundry.Logger; import net.lewmc.kryptonite.Kryptonite; import java.io.File; import java.io.IOException; +import java.io.InputStream; import java.net.MalformedURLException; import java.net.URL; +import java.net.HttpURLConnection; +import java.net.SocketTimeoutException; +import java.nio.charset.StandardCharsets; import java.util.Scanner; @@ - Scanner s = new Scanner(url.openStream()); - if (s.hasNextLine()) { - String response = s.nextLine(); + HttpURLConnection conn = (HttpURLConnection) url.openConnection(); + conn.setRequestMethod("GET"); + conn.setConnectTimeout(3500); + conn.setReadTimeout(4000); + conn.setRequestProperty("User-Agent", "Kryptonite/" + this.plugin.getDescription().getVersion()); + try (InputStream is = conn.getInputStream(); + Scanner s = new Scanner(is, StandardCharsets.UTF_8)) { + if (s.hasNextLine()) { + String response = s.nextLine(); if (response.isEmpty()) { log.severe("Unable to perform update check: There was no response from the server."); this.log.info(""); } else if (response.equals(this.plugin.getDescription().getVersion())) { log.info("You are running the latest version of Kryptonite."); this.log.info(""); } else { this.plugin.updatePending = true; log.warn("UPDATE > There's a new version of Kryptonite available."); log.warn("UPDATE > Your version: "+this.plugin.getDescription().getVersion()+" - latest version: "+response); log.warn("UPDATE > You can download the latest version from lewmc.net/kryptonite"); this.log.info(""); } - } else { - log.severe("Unable to perform update check: There was no response from the server."); - this.log.info(""); - } + } else { + log.severe("Unable to perform update check: There was no response from the server."); + this.log.info(""); + } + } catch (SocketTimeoutException e) { + log.warn("Unable to perform update check: timeout - " + e); + this.log.info(""); + }
🧹 Nitpick comments (50)
pom.xml (1)
127-132: SNAPSHOT dependency on FoundryUsing 1.3.0-SNAPSHOT on main can reduce reproducibility. Prefer a released version or pin via a property for easy updates.
- <dependency> - <groupId>net.lewmc</groupId> - <artifactId>foundry</artifactId> - <version>1.3.0-SNAPSHOT</version> - <scope>compile</scope> - </dependency> + <properties> + <foundry.version>1.3.0</foundry.version> + </properties> + <dependency> + <groupId>net.lewmc</groupId> + <artifactId>foundry</artifactId> + <version>${foundry.version}</version> + <scope>compile</scope> + </dependency>src/main/java/net/lewmc/kryptonite/kos/config/PaperWorld.java (1)
10-13: Deprecation metadata: add reason/replacement and plan removalInclude since/forRemoval and point to the new config path to guide users.
-/** - * The PaperWorld class manages the paper-world-defaults.yml configuration file. - * @deprecated - */ -@Deprecated +/** + * The PaperWorld class manages the paper-world-defaults.yml configuration file. + * @deprecated since 2.1.0 — replaced by MinecraftConfig/KOS_ConfigItemGui; will be removed in a future release. + */ +@Deprecated(forRemoval = true, since = "2.1.0")src/main/java/net/lewmc/kryptonite/kos/config/Bukkit.java (1)
10-13: Deprecation metadata: add reason/replacement and plan removalMatch the project’s new config direction and include since/forRemoval.
-/** - * The Bukkit class manages the bukkit.yml configuration file. - * @deprecated - */ -@Deprecated +/** + * The Bukkit class manages the bukkit.yml configuration file. + * @deprecated since 2.1.0 — replaced by MinecraftConfig/KOS_ConfigItemGui; scheduled for removal. + */ +@Deprecated(forRemoval = true, since = "2.1.0")src/main/java/net/lewmc/kryptonite/kos/gui/KOS_SpigotGui_2.java (1)
18-21: Deprecation metadata: clarify path forwardAdd since/forRemoval and reference the new GUI (KOS_ConfigItemGui) to aid migration.
-/** - * KOS Spigot GUI (Page 2/2) - * @deprecated - */ -@Deprecated +/** + * KOS Spigot GUI (Page 2/2) + * @deprecated since 2.1.0 — superseded by KOS_ConfigItemGui (MinecraftConfig-backed). + */ +@Deprecated(forRemoval = true, since = "2.1.0")src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_4.java (2)
18-21: Deprecation metadata: add reason/replacement and plan removalSame treatment as other deprecated classes.
-/** - * KOS Paper World GUI (Page 4/5) - * @deprecated - */ -@Deprecated +/** + * KOS Paper World GUI (Page 4/5) + * @deprecated since 2.1.0 — superseded by KOS_ConfigItemGui (MinecraftConfig-backed). + */ +@Deprecated(forRemoval = true, since = "2.1.0")
665-667: Minor UI string fixesSmall copy/paste glitches in disabled-text messages.
- ChatColor.GRAY + "The Alt Item Despawn Rate Jungle is disabled.", + ChatColor.GRAY + "The Alt Item Despawn Rate feature is disabled.", ... - ChatColor.GRAY + "The Alt Item Despawn Rate Jungle is disabled.", + ChatColor.GRAY + "The Alt Item Despawn Rate feature is disabled.",Also applies to: 800-802
src/main/java/net/lewmc/kryptonite/kos/config/Spigot.java (2)
10-13: Deprecation: add guidance.Include since/replacement to aid migration, e.g. “Use MinecraftConfig” and optionally
@Deprecated(since="2.1.0", forRemoval=true).
121-123: Typo in log method name (“veboseInfo”).Likely meant “verboseInfo”; current spelling risks compile/runtime errors depending on LogUtil.
Apply:
- log.veboseInfo("KOS>spigot.yml set '" + key + "' to '" + value + "'"); + log.verboseInfo("KOS>spigot.yml set '" + key + "' to '" + value + "'");(Repeat same fix for the other two occurrences.)
Also applies to: 147-149, 173-175
src/main/resources/plugin.yml (1)
21-21: YAML: space after comma in flow sequence.Conform to linter by adding a space.
- aliases: ['exploit','exploitdb'] + aliases: ['exploit', 'exploitdb']src/main/java/net/lewmc/kryptonite/kos/config/Purpur.java (2)
10-13: Deprecation: add guidance.Add
sinceand replacement link (e.g., new config framework) for consumers.
81-83: Typo in log method name (“veboseInfo”).Use “verboseInfo”.
- log.veboseInfo("KOS>purpur.yml set '" + key + "' to '" + value + "'"); + log.verboseInfo("KOS>purpur.yml set '" + key + "' to '" + value + "'");(Repeat for the other two.)
Also applies to: 107-109, 133-135
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PurpurGui.java (1)
95-100: Typo in user-facing text (“compatability”).Correct spelling.
- ChatColor.YELLOW + "has a known compatability issue and may not work correctly.", + ChatColor.YELLOW + "has a known compatibility issue and may not work correctly.",- ChatColor.GREEN + "has a known compatability issue and may not work correctly.", + ChatColor.GREEN + "has a known compatibility issue and may not work correctly.",Also applies to: 129-134
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_BukkitGui.java (3)
369-401: Label copy/paste: says “Monster” in Animal spawns section.Fix three occurrences.
- ChatColor.DARK_GREEN + "Ticks Per Monster Spawns", + ChatColor.DARK_GREEN + "Ticks Per Animal Spawns", ... - ChatColor.DARK_RED + "Ticks Per Monster Spawns", + ChatColor.DARK_RED + "Ticks Per Animal Spawns", ... - ChatColor.GOLD + "Ticks Per Monster Spawns", + ChatColor.GOLD + "Ticks Per Animal Spawns",
236-240: Label copy/paste: “Water Underground Creature” section labeled as “Water Ambient”.Adjust the title in all three places.
- ChatColor.DARK_GREEN + "Spawn Limit (Water Ambient)", + ChatColor.DARK_GREEN + "Spawn Limit (Water Underground Creature)", ... - ChatColor.GOLD + "Spawn Limit (Water Ambient)", + ChatColor.GOLD + "Spawn Limit (Water Underground Creature)", ... - ChatColor.DARK_RED + "Spawn Limit (Water Ambient)", + ChatColor.DARK_RED + "Spawn Limit (Water Underground Creature)",Also applies to: 246-260
580-611: GC period thresholds: low check should match range floor.Ideal is 400–600; use
< 400for “too low” instead of< 5.- } else if (value < 5) { + } else if (value < 400) {src/main/java/net/lewmc/kryptonite/utils/PermissionUtil.java (2)
9-13: Deprecation noted—consider pointing to replacement.Add
@deprecatedJavadoc message with the new permission path or API.
14-21: Avoid null lookup; cast to Player directly.
commandSenderis already a Player here; current code risks NPE if lookup fails and is wasteful.- public boolean isOperator(CommandSender commandSender) { - if (commandSender instanceof Player) { - String player = commandSender.getName(); - return Objects.requireNonNull(Bukkit.getServer().getPlayer(player)).isOp(); - } else { - return true; - } - } + public boolean isOperator(CommandSender commandSender) { + if (commandSender instanceof Player) { + return ((Player) commandSender).isOp(); + } + return true; // console and non-player senders + }Also remove the now-unused
java.util.Objectsimport.src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_1.java (1)
56-56: Typo: “Chucks” → “Chunks”.Rename method and call site.
- this.preventMovingIntoUnloadedChucks('c'); + this.preventMovingIntoUnloadedChunks('c');- private void preventMovingIntoUnloadedChucks(char id) { + private void preventMovingIntoUnloadedChunks(char id) {Also applies to: 172-172
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_5.java (1)
441-447: Misleading tooltip text for mob spawner tick rate.
“Items may despawn too quickly.” is unrelated to spawner tick rate.- ChatColor.RED + "Items may despawn too quickly.", + ChatColor.RED + "Too low may hurt performance or stall spawns.",src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_3.java (2)
66-68: Duplicate slot key 'e' used twice; second element will overwrite the first.
Two elements share id 'e' (armorStandsTick / armorStandsDoCollisionEntityLookups).Consider assigning a unique id (e.g., 'r') and adding it to the layout if this page remains used despite deprecation.
451-459: Inconsistent “ideal” coloring (GREEN text with ORANGE item).
Visual cue is mixed (ideal range but orange concrete).Use GREEN_CONCRETE for the ideal branch to match other pages for these five sections.
Also applies to: 484-495, 519-530, 554-565, 589-600
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PufferfishGui.java (1)
43-49: Cast to Player without guard can NPE/CCE if sender is not a player.
If invoked from console, this will throw.- this.gui.show((Player) this.user); + if (this.user instanceof Player p) { + this.gui.show(p); + } else { + return; + }dependency-reduced-pom.xml (1)
71-80: New repositories/distributionManagement: confirm availability/creds in CI.
Avoid build breaks on release/snapshot deploys.Also applies to: 90-99
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_SpigotGui_1.java (1)
19-22: Deprecation needs guidance (what to use instead and since when).Add a message and
sinceto both Javadoc and annotation.Apply this diff:
-/** - * KOS Spigot GUI (Page 1/2) - * @deprecated - */ -@Deprecated +/** + * KOS Spigot GUI (Page 1/2) + * @deprecated Since 2.1.0 — replaced by KOS_ConfigItemGui (Minecraft/Spigot pages unified). + */ +@Deprecated(forRemoval = true, since = "2.1.0")src/main/java/net/lewmc/kryptonite/edb/Check.java (1)
408-408: Gate switch to supportsMinecraft: LGTM. Minor label consistency nit.Else-branch currently logs expected
"MinecraftServer". Consider"Minecraft"to match the enum and other labels.Apply this diff:
- "MinecraftServer"); + "Minecraft");src/main/java/net/lewmc/kryptonite/utils/config/ConfigCollection.java (1)
3-3: Prefer Map interface and prevent reassignment.Use
Mapandfinalto avoid accidental field rebinds; behavior remains the same.Apply this diff:
import java.util.HashMap; +import java.util.Map; @@ - public HashMap<String, GenericConfigItem> values = new HashMap<>(); + public final Map<String, GenericConfigItem> values = new HashMap<>();Also applies to: 13-13
src/main/java/net/lewmc/kryptonite/Kryptonite.java (2)
40-44: Load config before reading “verbose”You read getConfig().getBoolean("verbose") before ensuring the config exists. Initialize defaults first to avoid silent false.
Apply:
public void onEnable() { - this.foundryConfig = new FoundryConfig(this); + this.saveDefaultConfig(); // ensure config exists before reading + this.foundryConfig = new FoundryConfig(this); this.foundryConfig.setVerbose(this.getConfig().getBoolean("verbose")); this.foundryConfig.setPluginId("kr"); this.log = new Logger(this.foundryConfig);Optional cleanup (since called above):
- this.saveDefaultConfig();
132-134: Log message wording: prefer “Minecraft” to match enumKeep user-facing text consistent with MINECRAFT.
- supportedConfigs = supportedConfigs + "Server Properties"; + supportedConfigs = supportedConfigs + "Minecraft";src/main/java/net/lewmc/kryptonite/kos/AutoKOS.java (1)
94-96: Message consistencyUpdate wording to reflect the new gate.
- this.log.info("[KOS] 1/6 - Server does not support Server Properties, skipping..."); + this.log.info("[KOS] 1/6 - Server does not support Minecraft (server.properties), skipping...");src/main/java/net/lewmc/kryptonite/config/LeafConfig.java (5)
31-37: Rename “Parallel World Tracing” → “Parallel World Ticking”Keys are parallel-world-ticking; align display names.
- "Parallel World Tracing", + "Parallel World Ticking", - "Parallel World Tracing Threads", + "Parallel World Ticking Threads", - "Parallel World Tracing Log Container Creation Stacktraces", + "Parallel World Ticking Log Container Creation Stacktraces", - "Parallel World Tracing Disable Hard Throw", + "Parallel World Ticking Disable Hard Throw", - "Parallel World Tracing Run Async Tasks Sync", + "Parallel World Ticking Run Async Tasks Sync",Also applies to: 46-50, 61-68, 71-82, 85-96
191-197: Typo: “aynchronous” → “asynchronous”- "Make playerdata saving aynchronous. Warning: might", + "Make playerdata saving asynchronous. Warning: might",
241-247: Label mismatchThis section is for async pathfinding; fix the title.
- "Async Entity Tracker Queue Size", + "Async Pathfinding Queue Size",
256-259: Label mismatchReject-policy is for pathfinding.
- "Async Entity Tracker Queue Size", + "Async Pathfinding Reject Policy",
271-283: Label mismatchMob spawning section labeled as pathfinding.
- "Async Pathfinding", + "Async Mob Spawning",src/main/java/net/lewmc/kryptonite/utils/config/DoubleConfigItem.java (1)
51-72: Use extension checks with endsWith and ensure YAML is always closed.
contains(".yml")/contains(".yaml")can mis-detect backup names; preferendsWith.- Wrap YAML get in a try/finally or ensure close is called on all paths.
Apply this diff:
@Override public Double getValue() { - if (this.file.contains(".properties")) { + if (this.file.endsWith(".properties")) { try { return Double.parseDouble(propFile.getProperty(key)); } catch (NumberFormatException e) { Logger l = new Logger(this.plugin.foundryConfig); l.severe("Unable to parse key '"+this.key+"' in file '"+this.file+"' NumberFormatException"); l.severe(e.getMessage()); return 0.0; } - } else if (this.file.contains(".yml") || file.contains(".yaml")) { + } else if (this.file.endsWith(".yml") || this.file.endsWith(".yaml")) { this.loadFile(); Double value = yamlFile.getDouble(key); yamlFile.close(); return value; } else { new Logger(this.plugin.foundryConfig).severe("Unable to load file: '"+this.file+"' extension not supported."); return 0.0; } }src/main/java/net/lewmc/kryptonite/utils/config/StringConfigItem.java (2)
63-71: Close YAML after save and prefer endsWith for extension checks.Apply this diff:
@Override public void setValue(String value) { - if (this.file.contains(".properties")) { + if (this.file.endsWith(".properties")) { propFile.setProperty(this.key, value); - } else if (file.contains(".yml") || file.contains(".yaml")) { + } else if (this.file.endsWith(".yml") || this.file.endsWith(".yaml")) { this.loadFile(); yamlFile.set(this.key, value); - yamlFile.save(); + yamlFile.save(); + yamlFile.close(); } }
45-56: Tighten extension checks and consistent error handling.Use
endsWithto avoid false positives on backup filenames.Apply this diff:
@Override public String getValue() { - if (this.file.contains(".properties")) { + if (this.file.endsWith(".properties")) { return propFile.getProperty(key); - } else if (this.file.contains(".yml") || file.contains(".yaml")) { + } else if (this.file.endsWith(".yml") || this.file.endsWith(".yaml")) { this.loadFile(); String value = yamlFile.getString(key); yamlFile.close(); return value; } else { new Logger(this.plugin.foundryConfig).severe("Unable to load file: '"+this.file+"' extension not supported."); return null; }src/main/java/net/lewmc/kryptonite/utils/config/BooleanConfigItem.java (1)
56-64: Close YAML after save and tighten extension checks.Apply this diff:
@Override public void setValue(Boolean value) { - if (this.file.contains(".properties")) { + if (this.file.endsWith(".properties")) { propFile.setProperty(this.key, String.valueOf(value)); - } else if (file.contains(".yml") || file.contains(".yaml")) { + } else if (this.file.endsWith(".yml") || this.file.endsWith(".yaml")) { this.loadFile(); yamlFile.set(this.key, value); - yamlFile.save(); + yamlFile.save(); + yamlFile.close(); } }src/main/java/net/lewmc/kryptonite/kos/gui/KOS_ConfigItemGui.java (4)
151-168: Avoid repeated disk reads; reusecurrentfor validation.
getValue()may hit disk; reuse local value to cut I/O.int current = intItem.getValue(); if (click.getType() == ClickType.RIGHT) { - if (intItem.willBeValid(intItem.getValue() + 1)) { + if (intItem.willBeValid(current + 1)) { intItem.setValue(current + 1); } } else if (click.getType() == ClickType.SHIFT_RIGHT) { - if (intItem.willBeValid(intItem.getValue() + 10)) { + if (intItem.willBeValid(current + 10)) { intItem.setValue(current + 10); } } else if (click.getType() == ClickType.LEFT) { - if (intItem.willBeValid(intItem.getValue() - 1)) { + if (intItem.willBeValid(current - 1)) { intItem.setValue(current - 1); } } else if (click.getType() == ClickType.SHIFT_LEFT) { - if (intItem.willBeValid(intItem.getValue() - 10)) { + if (intItem.willBeValid(current - 10)) { intItem.setValue(current - 10); } }
170-187: Same optimization for doubles; reuse local value.Double current = doubleItem.getValue(); if (click.getType() == ClickType.RIGHT) { - if (doubleItem.willBeValid(doubleItem.getValue() + 1.0)) { + if (doubleItem.willBeValid(current + 1.0)) { doubleItem.setValue(current + 1.0); } } else if (click.getType() == ClickType.SHIFT_RIGHT) { - if (doubleItem.willBeValid(doubleItem.getValue() + 10.0)) { + if (doubleItem.willBeValid(current + 10.0)) { doubleItem.setValue(current + 10.0); } } else if (click.getType() == ClickType.LEFT) { - if (doubleItem.willBeValid(doubleItem.getValue() - 1.0)) { + if (doubleItem.willBeValid(current - 1.0)) { doubleItem.setValue(current - 1.0); } } else if (click.getType() == ClickType.SHIFT_LEFT) { - if (doubleItem.willBeValid(doubleItem.getValue() - 10.0)) { + if (doubleItem.willBeValid(current - 10.0)) { doubleItem.setValue(current - 10.0); } }
188-190: Implement cycling for String options.Quick win: cycle through
allowedValueson click; SHIFT to reverse. I can wire this if you confirm desired UX.
68-86: Optional: deterministic ordering.
HashMapiteration order is undefined; consider sorting entries bygetName()for stable GUI.src/main/java/net/lewmc/kryptonite/utils/config/IntegerConfigItem.java (2)
78-87: Close YAML after save and prefer endsWith.Apply this diff:
@Override public void setValue(Integer value) { - if (this.file.contains(".properties")) { + if (this.file.endsWith(".properties")) { propFile.setProperty(this.key, String.valueOf(value)); - } else if (file.contains(".yml") || file.contains(".yaml")) { + } else if (this.file.endsWith(".yml") || this.file.endsWith(".yaml")) { this.loadFile(); yamlFile.set(this.key, value); - yamlFile.save(); + yamlFile.save(); + yamlFile.close(); } }
53-72: Tighten extension checks and ensure YAML close on all paths (already closes here).- if (this.file.contains(".properties")) { + if (this.file.endsWith(".properties")) { ... - } else if (this.file.contains(".yml") || file.contains(".yaml")) { + } else if (this.file.endsWith(".yml") || this.file.endsWith(".yaml")) {src/main/java/net/lewmc/kryptonite/utils/LogUtil.java (3)
52-60: Public API typo: veboseInfo → verboseInfo (keep both for compatibility)Expose a correctly spelled alias to avoid propagating the typo while keeping existing calls working.
Add alongside the existing method:
/** Correctly spelled alias; delegates to legacy method. */ public void verboseInfo(String message) { veboseInfo(message); }
68-72: Only verbose() writes to file; make info/warn/severe honor logfile tooParity with verbose logging improves diagnosability when
logfile=true.public void info(String message) { message = "[Kryptonite] " + message; - Bukkit.getLogger().info(message); + Bukkit.getLogger().info(message); + this.logToFile(Type.LOG_INFO, message); } public void warn(String message) { message = "[Kryptonite] " + message; - Bukkit.getLogger().warning(message); + Bukkit.getLogger().warning(message); + this.logToFile(Type.LOG_WARNING, message); } public void severe(String message) { message = "[Kryptonite] " + message; - Bukkit.getLogger().severe(message); + Bukkit.getLogger().severe(message); + this.logToFile(Type.LOG_SEVERE, message); }Also applies to: 80-84, 92-96
111-111: File append robustness + redundant task cancel
- Use CREATE/WRITE so logging survives deleted/missing files.
- Build the path portably.
cancelTask(task)after a one-shot async run is unnecessary.- Path p = Paths.get(this.plugin.getDataFolder().getAbsolutePath() + "/kryptonite.log"); + Path p = Paths.get(this.plugin.getDataFolder().getAbsolutePath(), "kryptonite.log"); ... - try (BufferedWriter writer = Files.newBufferedWriter(p, StandardOpenOption.APPEND)) { + try (BufferedWriter writer = Files.newBufferedWriter( + p, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.APPEND)) { writer.write(s); } catch (IOException e) { Bukkit.getLogger().severe("[Kryptonite] Unable to write to file: " + e.getMessage()); } - foliaLib.getScheduler().cancelTask(task);Also applies to: 114-114, 119-119
src/main/java/net/lewmc/kryptonite/utils/config/GenericConfigItem.java (3)
71-73: Null‑safety for dependency flagGuard against
nullto match the doc “If none, set to true.”- this.dependencyIsEnabled = dependencyIsEnabled; + this.dependencyIsEnabled = (dependencyIsEnabled != null) ? dependencyIsEnabled : Boolean.TRUE; this.plugin = plugin;
140-142: Avoid exposing internal mutable listReturn an unmodifiable view to prevent accidental external mutation.
- public List<String> getDescription() { - return this.description; - } + public List<String> getDescription() { + return java.util.Collections.unmodifiableList(this.description); + }
96-101: Javadoc nits
setValue: “The value to set.” (not “current value”).willBeValid: add@param valuedescription.Also applies to: 110-115
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
dependency-reduced-pom.xml(5 hunks)pom.xml(3 hunks)src/main/java/net/lewmc/kryptonite/Kryptonite.java(5 hunks)src/main/java/net/lewmc/kryptonite/Metrics.java(1 hunks)src/main/java/net/lewmc/kryptonite/config/LeafConfig.java(1 hunks)src/main/java/net/lewmc/kryptonite/config/MinecraftConfig.java(1 hunks)src/main/java/net/lewmc/kryptonite/edb/Check.java(3 hunks)src/main/java/net/lewmc/kryptonite/edb/Patch.java(1 hunks)src/main/java/net/lewmc/kryptonite/kos/AutoKOS.java(4 hunks)src/main/java/net/lewmc/kryptonite/kos/config/Bukkit.java(1 hunks)src/main/java/net/lewmc/kryptonite/kos/config/Leaf.java(0 hunks)src/main/java/net/lewmc/kryptonite/kos/config/PaperWorld.java(1 hunks)src/main/java/net/lewmc/kryptonite/kos/config/Pufferfish.java(1 hunks)src/main/java/net/lewmc/kryptonite/kos/config/Purpur.java(1 hunks)src/main/java/net/lewmc/kryptonite/kos/config/ServerProperties.java(0 hunks)src/main/java/net/lewmc/kryptonite/kos/config/Spigot.java(1 hunks)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_BukkitGui.java(1 hunks)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_ConfigItemGui.java(1 hunks)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_GuiConstants.java(3 hunks)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_LeafGui.java(0 hunks)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_ManualGUI.java(3 hunks)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_1.java(1 hunks)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_2.java(1 hunks)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_3.java(1 hunks)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_4.java(1 hunks)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_5.java(1 hunks)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PufferfishGui.java(1 hunks)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PurpurGui.java(1 hunks)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_ServerPropertiesGui.java(0 hunks)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_SpigotGui_1.java(3 hunks)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_SpigotGui_2.java(1 hunks)src/main/java/net/lewmc/kryptonite/report/Report.java(1 hunks)src/main/java/net/lewmc/kryptonite/utils/LogUtil.java(6 hunks)src/main/java/net/lewmc/kryptonite/utils/PermissionUtil.java(1 hunks)src/main/java/net/lewmc/kryptonite/utils/SoftwareUtil.java(1 hunks)src/main/java/net/lewmc/kryptonite/utils/UpdateUtil.java(2 hunks)src/main/java/net/lewmc/kryptonite/utils/config/BooleanConfigItem.java(1 hunks)src/main/java/net/lewmc/kryptonite/utils/config/ConfigCollection.java(1 hunks)src/main/java/net/lewmc/kryptonite/utils/config/DoubleConfigItem.java(1 hunks)src/main/java/net/lewmc/kryptonite/utils/config/GenericConfigItem.java(1 hunks)src/main/java/net/lewmc/kryptonite/utils/config/IntegerConfigItem.java(1 hunks)src/main/java/net/lewmc/kryptonite/utils/config/StringConfigItem.java(1 hunks)src/main/resources/plugin.yml(2 hunks)src/main/resources/profiles/FarmFriendly.kos(1 hunks)src/main/resources/profiles/YouHaveTrouble.kos(1 hunks)
💤 Files with no reviewable changes (4)
- src/main/java/net/lewmc/kryptonite/kos/gui/KOS_LeafGui.java
- src/main/java/net/lewmc/kryptonite/kos/config/ServerProperties.java
- src/main/java/net/lewmc/kryptonite/kos/gui/KOS_ServerPropertiesGui.java
- src/main/java/net/lewmc/kryptonite/kos/config/Leaf.java
🧰 Additional context used
🧬 Code graph analysis (26)
src/main/java/net/lewmc/kryptonite/utils/config/GenericConfigItem.java (2)
src/main/java/net/lewmc/kryptonite/Kryptonite.java (1)
Kryptonite(20-198)src/main/java/net/lewmc/kryptonite/utils/PropertiesUtil.java (1)
PropertiesUtil(8-48)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_1.java (4)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_2.java (1)
Deprecated(20-1061)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_3.java (1)
Deprecated(21-903)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_4.java (1)
Deprecated(20-944)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_5.java (1)
Deprecated(20-788)
src/main/java/net/lewmc/kryptonite/utils/SoftwareUtil.java (1)
src/main/java/net/lewmc/kryptonite/Kryptonite.java (1)
Kryptonite(20-198)
src/main/java/net/lewmc/kryptonite/config/MinecraftConfig.java (4)
src/main/java/net/lewmc/kryptonite/utils/config/BooleanConfigItem.java (1)
BooleanConfigItem(12-95)src/main/java/net/lewmc/kryptonite/utils/config/ConfigCollection.java (1)
ConfigCollection(9-14)src/main/java/net/lewmc/kryptonite/utils/config/GenericConfigItem.java (1)
GenericConfigItem(16-151)src/main/java/net/lewmc/kryptonite/utils/config/IntegerConfigItem.java (1)
IntegerConfigItem(12-125)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_5.java (2)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_1.java (1)
Deprecated(20-599)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_4.java (1)
Deprecated(20-944)
src/main/java/net/lewmc/kryptonite/kos/config/Purpur.java (1)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PurpurGui.java (1)
Deprecated(20-465)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_4.java (3)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_1.java (1)
Deprecated(20-599)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_3.java (1)
Deprecated(21-903)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_5.java (1)
Deprecated(20-788)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_2.java (4)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_1.java (1)
Deprecated(20-599)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_3.java (1)
Deprecated(21-903)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_4.java (1)
Deprecated(20-944)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_5.java (1)
Deprecated(20-788)
src/main/java/net/lewmc/kryptonite/kos/config/Pufferfish.java (1)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PufferfishGui.java (1)
Deprecated(20-388)
src/main/java/net/lewmc/kryptonite/utils/config/DoubleConfigItem.java (1)
src/main/java/net/lewmc/kryptonite/Kryptonite.java (1)
Kryptonite(20-198)
src/main/java/net/lewmc/kryptonite/kos/config/Spigot.java (2)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_SpigotGui_1.java (1)
Deprecated(21-757)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_SpigotGui_2.java (1)
Deprecated(20-140)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_SpigotGui_2.java (2)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_SpigotGui_1.java (1)
Deprecated(21-757)src/main/java/net/lewmc/kryptonite/kos/config/Spigot.java (1)
Deprecated(12-186)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PurpurGui.java (3)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PufferfishGui.java (1)
Deprecated(20-388)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_SpigotGui_1.java (1)
Deprecated(21-757)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_BukkitGui.java (1)
Deprecated(20-629)
src/main/java/net/lewmc/kryptonite/utils/config/BooleanConfigItem.java (1)
src/main/java/net/lewmc/kryptonite/Kryptonite.java (1)
Kryptonite(20-198)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_3.java (5)
src/main/java/net/lewmc/kryptonite/kos/config/PaperWorld.java (1)
Deprecated(12-390)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_1.java (1)
Deprecated(20-599)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_2.java (1)
Deprecated(20-1061)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_4.java (1)
Deprecated(20-944)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_5.java (1)
Deprecated(20-788)
src/main/java/net/lewmc/kryptonite/utils/config/IntegerConfigItem.java (1)
src/main/java/net/lewmc/kryptonite/Kryptonite.java (1)
Kryptonite(20-198)
src/main/java/net/lewmc/kryptonite/kos/config/PaperWorld.java (5)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_1.java (1)
Deprecated(20-599)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_2.java (1)
Deprecated(20-1061)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_3.java (1)
Deprecated(21-903)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_4.java (1)
Deprecated(20-944)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PaperWorld_5.java (1)
Deprecated(20-788)
src/main/java/net/lewmc/kryptonite/kos/AutoKOS.java (2)
src/main/java/net/lewmc/kryptonite/Kryptonite.java (1)
Kryptonite(20-198)src/main/java/net/lewmc/kryptonite/config/MinecraftConfig.java (1)
MinecraftConfig(15-96)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PufferfishGui.java (2)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_SpigotGui_1.java (1)
Deprecated(21-757)src/main/java/net/lewmc/kryptonite/kos/config/Pufferfish.java (1)
Deprecated(12-132)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_SpigotGui_1.java (1)
src/main/java/net/lewmc/kryptonite/config/MinecraftConfig.java (1)
MinecraftConfig(15-96)
src/main/java/net/lewmc/kryptonite/utils/config/StringConfigItem.java (1)
src/main/java/net/lewmc/kryptonite/Kryptonite.java (1)
Kryptonite(20-198)
src/main/java/net/lewmc/kryptonite/config/LeafConfig.java (1)
src/main/java/net/lewmc/kryptonite/Kryptonite.java (1)
Kryptonite(20-198)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_BukkitGui.java (2)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_PufferfishGui.java (1)
Deprecated(20-388)src/main/java/net/lewmc/kryptonite/kos/gui/KOS_SpigotGui_1.java (1)
Deprecated(21-757)
src/main/java/net/lewmc/kryptonite/kos/config/Bukkit.java (1)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_BukkitGui.java (1)
Deprecated(20-629)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_ManualGUI.java (1)
src/main/java/net/lewmc/kryptonite/Kryptonite.java (1)
Kryptonite(20-198)
src/main/java/net/lewmc/kryptonite/kos/gui/KOS_ConfigItemGui.java (3)
src/main/java/net/lewmc/kryptonite/Kryptonite.java (1)
Kryptonite(20-198)src/main/java/net/lewmc/kryptonite/config/LeafConfig.java (1)
LeafConfig(15-285)src/main/java/net/lewmc/kryptonite/config/MinecraftConfig.java (1)
MinecraftConfig(15-96)
🪛 YAMLlint (1.37.1)
src/main/resources/plugin.yml
[warning] 21-21: too few spaces after comma
(commas)
| /** | ||
| * Holds configuration data for the leaf-global.yml file. | ||
| */ | ||
| public HashMap<String, GenericConfigItem> values = new HashMap<>(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid shadowing values from base class
ConfigCollection already defines values. Drop the redeclaration to prevent shadowing.
- public HashMap<String, GenericConfigItem> values = new HashMap<>();
+ // use values from ConfigCollection📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public HashMap<String, GenericConfigItem> values = new HashMap<>(); | |
| // use values from ConfigCollection |
🤖 Prompt for AI Agents
In src/main/java/net/lewmc/kryptonite/config/LeafConfig.java around line 19, the
class redeclares a HashMap field named `values` which shadows the `values` field
from the base class ConfigCollection; remove the redeclaration line and rely on
the inherited `values` field instead, updating any access modifiers or
references in this class to use the base class field (if necessary, adjust
visibility in the base class to protected or provide accessor methods) so no
duplicate field remains.
| /** | ||
| * Holds configuration data for the server.properties file. | ||
| */ | ||
| public HashMap<String, GenericConfigItem> values = new HashMap<>(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid shadowing values from base class
Use the inherited map to keep a single source of truth.
- public HashMap<String, GenericConfigItem> values = new HashMap<>();
+ // use values from ConfigCollection📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public HashMap<String, GenericConfigItem> values = new HashMap<>(); | |
| // use values from ConfigCollection |
🤖 Prompt for AI Agents
In src/main/java/net/lewmc/kryptonite/config/MinecraftConfig.java around line
19, the class defines a new field "public HashMap<String, GenericConfigItem>
values" which shadows the inherited "values" from the base class; remove this
field declaration and any direct accesses to the local map, update all usages to
reference the inherited map (e.g., this.values or super.values as appropriate),
ensure visibility/typing matches the base class (cast or change generics only if
compatible), and run/adjust any constructors or serialization code that assumed
the local field so the class uses the single inherited source of truth.
Comment on lines
+58
to
+68
| values.put("simulation-distance", new IntegerConfigItem( | ||
| file, | ||
| "simulation-distance", | ||
| "Simulation Distance", | ||
| List.of("The distance mobs will be simulated."), | ||
| true, | ||
| 1, | ||
| 50, | ||
| "5 - "+values.get("view-distance").getValue(), | ||
| plugin | ||
| )); |
There was a problem hiding this comment.
Critical: NPE risk when constructing simulation-distance ideal
values.get("view-distance").getValue() runs before any file is loaded; for .properties, propFile is null and will NPE. Don’t read dependent values in the constructor.
Minimal fix:
- "5 - "+values.get("view-distance").getValue(),
+ "5 - 10",Alternative: compute this ideal lazily in getIdealValue() or load the file before reading, but avoid I/O in constructors.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| values.put("simulation-distance", new IntegerConfigItem( | |
| file, | |
| "simulation-distance", | |
| "Simulation Distance", | |
| List.of("The distance mobs will be simulated."), | |
| true, | |
| 1, | |
| 50, | |
| "5 - "+values.get("view-distance").getValue(), | |
| plugin | |
| )); | |
| values.put("simulation-distance", new IntegerConfigItem( | |
| file, | |
| "simulation-distance", | |
| "Simulation Distance", | |
| List.of("The distance mobs will be simulated."), | |
| true, | |
| 1, | |
| 50, | |
| "5 - 10", | |
| plugin | |
| )); |
🤖 Prompt for AI Agents
In src/main/java/net/lewmc/kryptonite/config/MinecraftConfig.java around lines
58-68, constructing the "simulation-distance" IntegerConfigItem currently calls
values.get("view-distance").getValue() during construction which can NPE because
config files (propFile) may not be loaded yet; remove that eager dependent read
and avoid I/O in the constructor. Fix by supplying a static/neutral ideal (or
null/empty string) when constructing the item and move the dependent computation
into a lazy getter (e.g., override/getIdealValue() to compute "5 -
<view-distance>" at runtime using values.get("view-distance").getValue()), or
compute and set the ideal after all values are loaded; do not call
values.get(...).getValue() inside the constructor.
Comment on lines
82
to
93
| private void runVanilla() { | ||
| if (this.softwareUtil.supportsServerProperties()) { | ||
| if (this.softwareUtil.supportsMinecraft()) { | ||
| this.log.info("[KOS] 1/6 - Running Vanilla optimisations"); | ||
|
|
||
| ServerProperties properties = new ServerProperties(this.plugin); | ||
| MinecraftConfig m = new MinecraftConfig(this.plugin); | ||
|
|
||
| properties.set(ServerProperties.Key.NETWORK_COMPRESSION_THRESHOLD, this.patches.getString("server.network-compression-threshold")); | ||
| properties.set(ServerProperties.Key.SIMULATION_DISTANCE, this.patches.getString("server.distance.simulation")); | ||
| properties.set(ServerProperties.Key.VIEW_DISTANCE, this.patches.getString("server.distance.view")); | ||
| properties.set(ServerProperties.Key.SYNC_CHUNK_WRITES, this.patches.getString("server.sync-chunk-writes")); | ||
| m.values.get("network-compression-threshold").setValue(this.patches.getInt("server.network-compression-threshold")); | ||
| m.values.get("simulation-distance").setValue(this.patches.getInt("server.distance.simulation")); | ||
| m.values.get("view-distance").setValue(this.patches.getInt("server.distance.view")); | ||
| m.values.get("sync-chunk-writes").setValue(this.patches.getBoolean("server.sync-chunk-writes")); | ||
| m.values.get("allow-flight").setValue(this.patches.getBoolean("server.allow-flight")); | ||
| } else { |
There was a problem hiding this comment.
Critical: server.properties items may NPE without loading file
GenericConfigItem for .properties doesn’t auto-load; setValue/getValue can hit null propFile. Load files before writes.
private void runVanilla() {
if (this.softwareUtil.supportsMinecraft()) {
this.log.info("[KOS] 1/6 - Running Vanilla optimisations");
MinecraftConfig m = new MinecraftConfig(this.plugin);
+ // Ensure server.properties is loaded for all items before writes
+ m.values.forEach((k, item) -> item.loadFile());
m.values.get("network-compression-threshold").setValue(this.patches.getInt("server.network-compression-threshold"));
m.values.get("simulation-distance").setValue(this.patches.getInt("server.distance.simulation"));
m.values.get("view-distance").setValue(this.patches.getInt("server.distance.view"));
m.values.get("sync-chunk-writes").setValue(this.patches.getBoolean("server.sync-chunk-writes"));
m.values.get("allow-flight").setValue(this.patches.getBoolean("server.allow-flight"));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private void runVanilla() { | |
| if (this.softwareUtil.supportsServerProperties()) { | |
| if (this.softwareUtil.supportsMinecraft()) { | |
| this.log.info("[KOS] 1/6 - Running Vanilla optimisations"); | |
| ServerProperties properties = new ServerProperties(this.plugin); | |
| MinecraftConfig m = new MinecraftConfig(this.plugin); | |
| properties.set(ServerProperties.Key.NETWORK_COMPRESSION_THRESHOLD, this.patches.getString("server.network-compression-threshold")); | |
| properties.set(ServerProperties.Key.SIMULATION_DISTANCE, this.patches.getString("server.distance.simulation")); | |
| properties.set(ServerProperties.Key.VIEW_DISTANCE, this.patches.getString("server.distance.view")); | |
| properties.set(ServerProperties.Key.SYNC_CHUNK_WRITES, this.patches.getString("server.sync-chunk-writes")); | |
| m.values.get("network-compression-threshold").setValue(this.patches.getInt("server.network-compression-threshold")); | |
| m.values.get("simulation-distance").setValue(this.patches.getInt("server.distance.simulation")); | |
| m.values.get("view-distance").setValue(this.patches.getInt("server.distance.view")); | |
| m.values.get("sync-chunk-writes").setValue(this.patches.getBoolean("server.sync-chunk-writes")); | |
| m.values.get("allow-flight").setValue(this.patches.getBoolean("server.allow-flight")); | |
| } else { | |
| private void runVanilla() { | |
| if (this.softwareUtil.supportsMinecraft()) { | |
| this.log.info("[KOS] 1/6 - Running Vanilla optimisations"); | |
| MinecraftConfig m = new MinecraftConfig(this.plugin); | |
| // Ensure server.properties is loaded for all items before writes | |
| m.values.forEach((k, item) -> item.loadFile()); | |
| m.values.get("network-compression-threshold").setValue(this.patches.getInt("server.network-compression-threshold")); | |
| m.values.get("simulation-distance").setValue(this.patches.getInt("server.distance.simulation")); | |
| m.values.get("view-distance").setValue(this.patches.getInt("server.distance.view")); | |
| m.values.get("sync-chunk-writes").setValue(this.patches.getBoolean("server.sync-chunk-writes")); | |
| m.values.get("allow-flight").setValue(this.patches.getBoolean("server.allow-flight")); | |
| } else { |
🤖 Prompt for AI Agents
In src/main/java/net/lewmc/kryptonite/kos/AutoKOS.java around lines 82 to 93,
the code calls m.values.get(...).setValue(...) on GenericConfigItem instances
that represent .properties entries but GenericConfigItem does not auto-load its
backing file, so getValue/setValue may NPE; fix by ensuring the MinecraftConfig
or each GenericConfigItem loads its properties file first (e.g., call m.load()
or m.values.get(...).load() / ensure config.loadFiles() is invoked) before any
getValue/setValue calls, and handle or propagate any IOException if loading
fails.
Comment on lines
+55
to
+61
| InventoryHolder holder = this.user.getServer().getPlayer(this.user.getName()); | ||
| this.gui = new InventoryGui(this.plugin, holder, "KOS - Server Configuration", this.getElements()); | ||
| this.addElements(); | ||
|
|
||
| this.gui.build(); | ||
| this.gui.show((Player) this.user); | ||
| } |
There was a problem hiding this comment.
Guard against non-player senders; current code can ClassCastException on console.
(Player) this.user will crash if the command is run from console/command block. Also getPlayer(getName()) can return null.
Apply this diff:
public void show() {
- InventoryHolder holder = this.user.getServer().getPlayer(this.user.getName());
- this.gui = new InventoryGui(this.plugin, holder, "KOS - Server Configuration", this.getElements());
+ if (!(this.user instanceof Player player)) {
+ this.user.sendMessage(ChatColor.RED + "This command can only be used by a player.");
+ return;
+ }
+ InventoryHolder holder = player;
+ this.gui = new InventoryGui(this.plugin, holder, "KOS - Server Configuration", this.getElements());
this.addElements();
this.gui.build();
- this.gui.show((Player) this.user);
+ this.gui.show(player);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| InventoryHolder holder = this.user.getServer().getPlayer(this.user.getName()); | |
| this.gui = new InventoryGui(this.plugin, holder, "KOS - Server Configuration", this.getElements()); | |
| this.addElements(); | |
| this.gui.build(); | |
| this.gui.show((Player) this.user); | |
| } | |
| public void show() { | |
| if (!(this.user instanceof Player player)) { | |
| this.user.sendMessage(ChatColor.RED + "This command can only be used by a player."); | |
| return; | |
| } | |
| InventoryHolder holder = player; | |
| this.gui = new InventoryGui(this.plugin, holder, "KOS - Server Configuration", this.getElements()); | |
| this.addElements(); | |
| this.gui.build(); | |
| this.gui.show(player); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/net/lewmc/kryptonite/kos/gui/KOS_ConfigItemGui.java around
lines 55-61, guard against non-player senders and a null player lookup: check if
this.user is an instance of Player and if not send an informative message and
return; cast to Player only after that check; avoid calling getPlayer(getName())
which can return null — use the Player object directly as the InventoryHolder
(or, if you must lookup, check the lookup result for null and handle it). Then
call gui.build() and gui.show(player) using the validated Player instance.
Comment on lines
+49
to
53
| /** | ||
| * @deprecated | ||
| */ | ||
| @Deprecated | ||
| public class Metrics { |
There was a problem hiding this comment.
Do not modify bStats Metrics class — revert deprecation
Header explicitly forbids modifications other than package changes. Adding @deprecated may violate bStats requirements.
-/**
- * @deprecated
- */
-@Deprecated
+/* no modifications allowed by bStats other than package */📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** | |
| * @deprecated | |
| */ | |
| @Deprecated | |
| public class Metrics { | |
| /* no modifications allowed by bStats other than package */ | |
| public class Metrics { |
🤖 Prompt for AI Agents
In src/main/java/net/lewmc/kryptonite/Metrics.java around lines 49 to 53, the
bStats Metrics class was edited to add a @Deprecated annotation and a
deprecation Javadoc; revert those changes so the class matches the original
bStats implementation (remove the @Deprecated annotation and the deprecated
Javadoc), leaving only the allowed package change if that was required, and
ensure no other modifications are present to comply with bStats header
requirements.
Comment on lines
+79
to
+87
| public void setValue(Double value) { | ||
| if (this.file.contains(".properties")) { | ||
| propFile.setProperty(this.key, String.valueOf(value)); | ||
| } else if (file.contains(".yml") || file.contains(".yaml")) { | ||
| this.loadFile(); | ||
| yamlFile.set(this.key, value); | ||
| yamlFile.save(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Close YAML handle after save to avoid file descriptor leaks.
setValue loads the YAML and saves but never closes it. On Windows this can lock the file; on any OS it can leak FDs.
Apply this diff:
@Override
public void setValue(Double value) {
- if (this.file.contains(".properties")) {
+ if (this.file.endsWith(".properties")) {
propFile.setProperty(this.key, String.valueOf(value));
- } else if (file.contains(".yml") || file.contains(".yaml")) {
+ } else if (this.file.endsWith(".yml") || this.file.endsWith(".yaml")) {
this.loadFile();
yamlFile.set(this.key, value);
- yamlFile.save();
+ yamlFile.save();
+ yamlFile.close();
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/net/lewmc/kryptonite/utils/config/DoubleConfigItem.java around
lines 79 to 87, the YAML file handle is left open after calling yamlFile.save(),
which can lock the file or leak file descriptors; fix by ensuring the YAML
handle is closed after saving (e.g., call yamlFile.close() after yamlFile.save()
or wrap loadFile()/set/save in a try/finally so yamlFile.close() always runs)
and handle possible exceptions during save so the close is guaranteed.
Comment on lines
+103
to
+115
| @Override | ||
| public boolean isIdeal() { | ||
| if (idealValue == null) { return true; } | ||
| Double current = this.getValue(); | ||
| if (idealValue.contains("-")) { | ||
| String[] parts = idealValue.split("-"); | ||
| double minIdeal = Double.parseDouble(parts[0]); | ||
| double maxIdeal = Double.parseDouble(parts[1]); | ||
| return current >= minIdeal && current <= maxIdeal; | ||
| } else { | ||
| return current == Double.parseDouble(idealValue); | ||
| } | ||
| } |
There was a problem hiding this comment.
Harden ideal-value parsing; trim parts and avoid exact double equality.
Ranges like "1 - 5" currently throw NumberFormatException (no trim). Exact == on doubles is fragile.
Apply this diff:
@Override
public boolean isIdeal() {
if (idealValue == null) { return true; }
Double current = this.getValue();
- if (idealValue.contains("-")) {
- String[] parts = idealValue.split("-");
- double minIdeal = Double.parseDouble(parts[0]);
- double maxIdeal = Double.parseDouble(parts[1]);
- return current >= minIdeal && current <= maxIdeal;
- } else {
- return current == Double.parseDouble(idealValue);
- }
+ try {
+ if (idealValue.contains("-")) {
+ String[] parts = idealValue.split("-");
+ double minIdeal = Double.parseDouble(parts[0].trim());
+ double maxIdeal = Double.parseDouble(parts[1].trim());
+ return current >= minIdeal && current <= maxIdeal;
+ } else {
+ double target = Double.parseDouble(idealValue.trim());
+ return Math.abs(current - target) <= 1e-9;
+ }
+ } catch (NumberFormatException ex) {
+ new Logger(this.plugin.foundryConfig).severe("Invalid idealValue '"+idealValue+"' for key '"+this.key+"'");
+ return false;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Override | |
| public boolean isIdeal() { | |
| if (idealValue == null) { return true; } | |
| Double current = this.getValue(); | |
| if (idealValue.contains("-")) { | |
| String[] parts = idealValue.split("-"); | |
| double minIdeal = Double.parseDouble(parts[0]); | |
| double maxIdeal = Double.parseDouble(parts[1]); | |
| return current >= minIdeal && current <= maxIdeal; | |
| } else { | |
| return current == Double.parseDouble(idealValue); | |
| } | |
| } | |
| @Override | |
| public boolean isIdeal() { | |
| if (idealValue == null) { return true; } | |
| Double current = this.getValue(); | |
| try { | |
| if (idealValue.contains("-")) { | |
| String[] parts = idealValue.split("-"); | |
| double minIdeal = Double.parseDouble(parts[0].trim()); | |
| double maxIdeal = Double.parseDouble(parts[1].trim()); | |
| return current >= minIdeal && current <= maxIdeal; | |
| } else { | |
| double target = Double.parseDouble(idealValue.trim()); | |
| return Math.abs(current - target) <= 1e-9; | |
| } | |
| } catch (NumberFormatException ex) { | |
| new Logger(this.plugin.foundryConfig).severe("Invalid idealValue '"+idealValue+"' for key '"+this.key+"'"); | |
| return false; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/net/lewmc/kryptonite/utils/config/DoubleConfigItem.java around
lines 103 to 115, the ideal-value parsing is brittle: split parts aren't trimmed
(so "1 - 5" breaks) and exact double equality is unsafe. Update the range branch
to trim each split part before Double.parseDouble, guard against null current
value, and catch/handle NumberFormatException (e.g., treat malformed ideal as
non-ideal or log and return false). Replace the exact equality check with a
tolerance-based comparison using a small epsilon (e.g., Math.abs(current -
target) < 1e-9) so near-equal doubles count as ideal.
Comment on lines
+79
to
+88
| if (file.contains(".properties")) { | ||
| this.propFile = new PropertiesUtil(plugin.getServer().getWorldContainer() + File.separator + file); | ||
| } else if (file.contains(".yaml") || file.contains(".yml")) { | ||
| this.yamlFile = new Files(plugin.foundryConfig, plugin); | ||
| this.yamlFile.loadNoReformat(new File(this.plugin.getDataFolder(),"/../../"+file).getAbsoluteFile()); | ||
| } else { | ||
| new Logger(plugin.foundryConfig).severe("Unable to load file '"+file+"' file extension not supported."); | ||
| new Logger(plugin.foundryConfig).severe("Expect additional errors."); | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: YAML path resolution uses absolute “/../../…”, likely pointing to root (e.g., “/bukkit.yml”)
new File(parent, "/../../"+file) ignores parent on UNIX-like systems. Also, extension checks should use endsWith case‑insensitively. Resolve server root from the plugin data folder and construct paths safely.
- public void loadFile() {
- if (file.contains(".properties")) {
- this.propFile = new PropertiesUtil(plugin.getServer().getWorldContainer() + File.separator + file);
- } else if (file.contains(".yaml") || file.contains(".yml")) {
- this.yamlFile = new Files(plugin.foundryConfig, plugin);
- this.yamlFile.loadNoReformat(new File(this.plugin.getDataFolder(),"/../../"+file).getAbsoluteFile());
- } else {
- new Logger(plugin.foundryConfig).severe("Unable to load file '"+file+"' file extension not supported.");
- new Logger(plugin.foundryConfig).severe("Expect additional errors.");
- }
- }
+ public void loadFile() {
+ // plugins/Kryptonite -> server root
+ final File serverRoot = this.plugin.getDataFolder().getAbsoluteFile().getParentFile().getParentFile();
+ final File target = new File(serverRoot, this.file);
+ final String lower = this.file.toLowerCase(java.util.Locale.ROOT);
+
+ if (lower.endsWith(".properties")) {
+ this.propFile = new PropertiesUtil(target.getPath());
+ } else if (lower.endsWith(".yaml") || lower.endsWith(".yml")) {
+ this.yamlFile = new Files(plugin.foundryConfig, plugin);
+ this.yamlFile.loadNoReformat(target.getAbsoluteFile());
+ } else {
+ Logger log = new Logger(plugin.foundryConfig);
+ log.severe("Unable to load file '" + file + "' file extension not supported.");
+ log.severe("Expect additional errors.");
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (file.contains(".properties")) { | |
| this.propFile = new PropertiesUtil(plugin.getServer().getWorldContainer() + File.separator + file); | |
| } else if (file.contains(".yaml") || file.contains(".yml")) { | |
| this.yamlFile = new Files(plugin.foundryConfig, plugin); | |
| this.yamlFile.loadNoReformat(new File(this.plugin.getDataFolder(),"/../../"+file).getAbsoluteFile()); | |
| } else { | |
| new Logger(plugin.foundryConfig).severe("Unable to load file '"+file+"' file extension not supported."); | |
| new Logger(plugin.foundryConfig).severe("Expect additional errors."); | |
| } | |
| } | |
| public void loadFile() { | |
| // plugins/Kryptonite -> server root | |
| final File serverRoot = this.plugin.getDataFolder().getAbsoluteFile().getParentFile().getParentFile(); | |
| final File target = new File(serverRoot, this.file); | |
| final String lower = this.file.toLowerCase(java.util.Locale.ROOT); | |
| if (lower.endsWith(".properties")) { | |
| this.propFile = new PropertiesUtil(target.getPath()); | |
| } else if (lower.endsWith(".yaml") || lower.endsWith(".yml")) { | |
| this.yamlFile = new Files(plugin.foundryConfig, plugin); | |
| this.yamlFile.loadNoReformat(target.getAbsoluteFile()); | |
| } else { | |
| Logger log = new Logger(plugin.foundryConfig); | |
| log.severe("Unable to load file '" + file + "' file extension not supported."); | |
| log.severe("Expect additional errors."); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/net/lewmc/kryptonite/utils/config/GenericConfigItem.java around
lines 79-88, the YAML branch builds a File with new
File(this.plugin.getDataFolder(), "/../../"+file) which on UNIX treats
"/../../…" as absolute (ignoring parent) and is unsafe; also extension checks
should use endsWith case-insensitively. Replace the fragile concatenation with a
safe resolution: derive the server root from the plugin data folder (e.g.,
plugin.getDataFolder().getParentFile().getParentFile() or resolve two parents),
then construct the YAML File with new File(serverRoot, file) (do not prepend a
leading slash), and change the extension checks to use case-insensitive endsWith
(e.g., file.toLowerCase().endsWith(".yaml") ||
file.toLowerCase().endsWith(".yml")) and similarly for properties. Ensure the
resulting File is absolute and normalized before calling loadNoReformat.
Comment on lines
+79
to
+83
| @Override | ||
| public boolean willBeValid(String value) { | ||
| return dependencyIsEnabled && this.allowedValues.contains(value); | ||
|
|
||
| } |
There was a problem hiding this comment.
Avoid NPE when allowedValues is null; treat null as “allow any”.
allowedValues.contains(value) will NPE if allowedValues is omitted. Aligns with your boolean/integer “null ideal means any” semantics.
Apply this diff:
@Override
public boolean willBeValid(String value) {
- return dependencyIsEnabled && this.allowedValues.contains(value);
+ return this.dependencyIsEnabled && (this.allowedValues == null || this.allowedValues.contains(value));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Override | |
| public boolean willBeValid(String value) { | |
| return dependencyIsEnabled && this.allowedValues.contains(value); | |
| } | |
| @Override | |
| public boolean willBeValid(String value) { | |
| return this.dependencyIsEnabled && (this.allowedValues == null || this.allowedValues.contains(value)); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/net/lewmc/kryptonite/utils/config/StringConfigItem.java around
lines 79 to 83, the willBeValid method currently calls
allowedValues.contains(value) which can NPE if allowedValues is null; change the
logic so null allowedValues means "allow any" by returning dependencyIsEnabled
&& (allowedValues == null || allowedValues.contains(value)). Update the boolean
expression accordingly so it first checks dependencyIsEnabled and then treats a
null allowedValues as permitting all values.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
=
Summary by CodeRabbit
New Features
Improvements
Deprecations