-
Notifications
You must be signed in to change notification settings - Fork 6
Reset branch #344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reset branch #344
Conversation
Essence 1.11.0
Next update
|
Thank you for opening a pull request to Essence. Someone will review your request shortly. If you don't hear from us within 5 working days, please feel free to ping Lewis or email dev@lewmc.net Thank you! |
WalkthroughThis pull request releases version 1.11.0 and restructures spawn configuration by replacing flat keys (always-spawn, main-spawn-world) with a nested hierarchy (global-spawn.enabled, global-spawn.world, force-spawn.on-join, force-spawn.on-death). Spawn behavior in event handlers is updated to use the new configuration keys, and numeric types are applied to coordinate storage in the migration utility. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
dependency-reduced-pom.xml(1 hunks)src/main/java/net/lewmc/essence/EssenceConfiguration.java(1 hunks)src/main/java/net/lewmc/essence/core/EventJoin.java(2 hunks)src/main/java/net/lewmc/essence/core/EventRespawn.java(2 hunks)src/main/java/net/lewmc/essence/core/UtilUpdate.java(1 hunks)src/main/java/net/lewmc/essence/teleportation/ModuleTeleportation.java(1 hunks)src/main/java/net/lewmc/essence/teleportation/spawn/CommandSpawn.java(1 hunks)src/main/resources/config.yml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/resources/config.yml (3)
src/main/java/net/lewmc/essence/teleportation/spawn/CommandSetspawn.java (3)
CommandSetspawn(14-66)Override(30-33)Override(42-65)src/main/java/net/lewmc/essence/core/UtilImport.java (1)
essentialsSpawns(137-176)src/main/java/net/lewmc/essence/teleportation/tp/UtilTeleport.java (1)
sendToSpawn(424-470)
src/main/java/net/lewmc/essence/core/EventRespawn.java (2)
src/main/java/net/lewmc/essence/teleportation/spawn/CommandSetspawn.java (1)
CommandSetspawn(14-66)src/main/java/net/lewmc/essence/teleportation/tp/UtilTeleport.java (1)
sendToSpawn(424-470)
🔇 Additional comments (7)
src/main/java/net/lewmc/essence/core/EventRespawn.java (2)
39-39: LGTM: Configuration key updated consistently.The configuration key has been correctly updated from
teleportation.spawn.always-spawntoteleportation.spawn.force-spawn.on-death, aligning with the new nested spawn configuration structure.
60-64: LGTM: Spawn logic correctly implements global-spawn behavior.The conditional logic properly handles the global spawn feature:
- When enabled, players respawn at the configured global spawn world
- When disabled, players respawn in their current world
This is consistent with the spawn behavior updates in
EventJoin.javaandCommandSpawn.java.src/main/java/net/lewmc/essence/EssenceConfiguration.java (1)
136-139: LGTM: Configuration keys properly registered with correct types and defaults.All four new spawn configuration keys are correctly registered with appropriate types and sensible defaults:
global-spawn.worldandglobal-spawn.enabledcontrol the global spawn featureforce-spawn.on-deathandforce-spawn.on-joincontrol forced spawning behaviorThe implementation follows the existing pattern and includes proper type checking via the
getValue()method.src/main/java/net/lewmc/essence/teleportation/ModuleTeleportation.java (1)
78-78: LGTM: Tab completer properly registered for spawn command.The tab completer registration follows the established pattern and correctly checks if the spawn command is disabled before registering.
src/main/java/net/lewmc/essence/teleportation/spawn/CommandSpawn.java (1)
63-65: LGTM: Command correctly implements global-spawn feature.The spawn command now properly checks the
global-spawn.enabledflag and usesglobal-spawn.worldwhen teleporting without arguments. This is consistent with the spawn behavior updates inEventJoin.javaandEventRespawn.java.src/main/java/net/lewmc/essence/core/EventJoin.java (2)
60-60: LGTM: Join spawn condition updated to use force-spawn.on-join.The condition correctly uses the new
force-spawn.on-joinconfiguration key, maintaining the same behavior while aligning with the restructured spawn configuration.
77-82: LGTM: Spawn method simplified and correctly implements global-spawn logic.The method signature has been appropriately simplified by removing the unused
Loggerparameter. The spawn logic correctly:
- Checks if global spawn is enabled
- Teleports to the global spawn world when enabled
- Falls back to the player's current world when disabled
This implementation is consistent with the spawn behavior in
EventRespawn.java.
dependency-reduced-pom.xml
Outdated
| <artifactId>essence</artifactId> | ||
| <name>Essence</name> | ||
| <version>1.11.0-SNAPSHOT</version> | ||
| <version>1.11.0</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
paper-api 1.21-R0.1 release version PaperMC Maven coordinates
💡 Result:
Use the PaperMC Maven repository and this dependency:
Repository:
papermc
https://repo.papermc.io/repository/maven-public/
Dependency:
io.papermc.paper
paper-api
1.21-R0.1
provided
Sources: PaperMC docs/README (official repo). [1][2]
References
[1] https://github.com/PaperMC/Paper
[2] https://papermc.io/news/1-21/
🌐 Web query:
PaperMC plugin development snapshot dependencies release builds best practices
💡 Result:
Concise best-practices for PaperMC plugin development (snapshot deps, release builds, dependencies, packaging, compatibility)
-
Use the server API as compileOnly (don’t ship Paper/Spigot classes). Declare Paper as compileOnly and target a stable API version for releases; only use snapshots for development or CI builds. Test snapshots separately and pin exact snapshot coordinates when used. [1][2][5]
-
Avoid snapshot dependencies in release artifacts. If you must depend on a snapshot (hotfix/testing), isolate it to development branches/CI and clearly document the requirement for users. Prefer stable releases for public releases. [5]
-
Shade/relocate third‑party libraries you bundle. Use the Gradle Shadow (or Maven Shade) plugin to produce a single “fat” jar, and relocate packages to prevent runtime conflicts with other plugins. Only shade libraries you control; keep server API as provided/compileOnly. [3][4]
-
Use classloader/isolation features correctly. Follow Paper’s dependency declaration and paper-plugin.yml options (bootstrap vs server, load BEFORE/AFTER, join-classpath) rather than relying on old Bukkit load-order semantics. Avoid Bukkit serialization unless necessary. [1]
-
Build strategy & CI:
- Produce two artifacts (recommended): a release jar targeting stable API (no snapshots) and a dev/ci artifact (with snapshots or extra debug). Automate tests and server integration tests in CI across supported Minecraft/Paper versions.
- Tag releases, use semantic versioning, and include the targeted server/API version in artifact names. [2]
-
Compatibility matrix & testing:
- Test across the server versions you claim to support (install each Paper/Spigot/vanilla mapping used).
- Prefer using API-stable features; when using internals or mappings that change between versions, isolate with abstraction layers and feature-detect at runtime. [2][1]
-
Dependency management rules:
- compileOnly for server API (Paper), implementation/shade for libs you ship, provided/runtime for server-provided libs.
- Relocate to your namespace to prevent clashes; exclude native or non-jar artifacts from shading. Validate shaded output. [3][4]
-
Publishing & user guidance:
- Publish releases that do not require snapshots. If a release depends on a snapshot, document repository URLs and exact versions (or provide a shaded build).
- Include clear plugin.yml/paper-plugin.yml dependency/load instructions and runtime requirements. [1][8]
Quick references:
- Paper plugin/dependency docs and paper-plugin.yml guidance. [1]
- Paper project setup / Gradle guidance (how Paper recommends builds). [2]
- Gradle Shadow shading/relocation usage and caveats. [3]
- Example/shading guidance from CommandAPI (relocate recommendation). [4]
- Snapshot usage guidance (example: Adventure snapshot instructions). [5]
If you want, I can:
- provide a sample Gradle build.gradle.kts showing compileOnly Paper, shadowJar with relocations, and separate release/dev tasks; or
- produce a CI (GitHub Actions) workflow template that builds release and snapshot artifacts and runs integration tests against multiple Paper versions.
Replace snapshot paper-api dependency with stable release version.
The reduced POM declares paper-api:1.21-R0.1-SNAPSHOT as a provided dependency in the 1.11.0 release build. Per PaperMC best practices, snapshot dependencies should be avoided in release artifacts—they reduce reproducibility and violate the documented guideline to "prefer stable releases for public releases." The non-snapshot version paper-api:1.21-R0.1 is available in the PaperMC Maven repository and should be used instead.
🤖 Prompt for AI Agents
In dependency-reduced-pom.xml around line 7, the POM uses a snapshot paper-api
dependency (paper-api:1.21-R0.1-SNAPSHOT) which is not appropriate for a
release; update the dependency declaration to reference the stable non-SNAPSHOT
version (paper-api:1.21-R0.1) so the reduced POM and release artifact avoid
snapshot dependencies and ensure reproducible builds.
| worldsFile.set("world."+uid+".spawn.x", spawnsFile.getInt("spawn."+spawnName+".X")); | ||
| worldsFile.set("world."+uid+".spawn.y", spawnsFile.getInt("spawn."+spawnName+".Y")); | ||
| worldsFile.set("world."+uid+".spawn.z", spawnsFile.getInt("spawn."+spawnName+".Z")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Use getDouble() for spawn coordinates to preserve precision.
The migration reads X, Y, Z coordinates using getInt(), but spawn coordinates should be stored as doubles throughout the codebase. Evidence from other files shows:
CommandSetspawn.java(lines 50-52) saves coordinates usingloc.getX(),loc.getY(),loc.getZ(), which returndoublevaluesUtilTeleport.java(lines 436-438) reads spawn coordinates usinggetDouble()UtilImport.java(lines 160-162) saves Essentials spawn imports usinggetDouble()
Using getInt() will truncate decimal values, causing players to spawn at incorrect positions with potential precision loss of up to 1 block.
Apply this diff to fix the coordinate precision issue:
- worldsFile.set("world."+uid+".spawn.x", spawnsFile.getInt("spawn."+spawnName+".X"));
- worldsFile.set("world."+uid+".spawn.y", spawnsFile.getInt("spawn."+spawnName+".Y"));
- worldsFile.set("world."+uid+".spawn.z", spawnsFile.getInt("spawn."+spawnName+".Z"));
+ worldsFile.set("world."+uid+".spawn.x", spawnsFile.getDouble("spawn."+spawnName+".X"));
+ worldsFile.set("world."+uid+".spawn.y", spawnsFile.getDouble("spawn."+spawnName+".Y"));
+ worldsFile.set("world."+uid+".spawn.z", spawnsFile.getDouble("spawn."+spawnName+".Z"));🤖 Prompt for AI Agents
In src/main/java/net/lewmc/essence/core/UtilUpdate.java around lines 392 to 394,
the migration currently reads spawn coordinates with getInt() which truncates
fractional precision; replace getInt() with getDouble() for X, Y and Z (i.e.,
use spawnsFile.getDouble("spawn."+spawnName+".X") / ".Y" / ".Z") so worldsFile
stores the full double values consistent with CommandSetspawn, UtilTeleport and
UtilImport; ensure the types passed to worldsFile.set remain doubles.
| global-spawn: | ||
| enabled: false | ||
| world: world | ||
| force-spawn: | ||
| on-join: false | ||
| on-death: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for migration logic handling old spawn configuration keys
rg -nP -C3 'main-spawn-world|always-spawn' --type=javaRepository: LewMC/Essence
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Search for configuration-related classes and migration logic
# Find configuration-related files
fd -e java | xargs rg -l 'Config|config' --type=java | head -20
# Search for migration or update patterns
rg -nP 'migration|migrate|update.*config|version' --type=java -i | head -30
# Look for spawn configuration handling
rg -nP 'spawn' --type=java -i | head -20Repository: LewMC/Essence
Length of output: 8045
🏁 Script executed:
#!/bin/bash
# Read UtilUpdate.java to see the migrateSpawns implementation
wc -l src/main/java/net/lewmc/essence/core/UtilUpdate.java
cat -n src/main/java/net/lewmc/essence/core/UtilUpdate.javaRepository: LewMC/Essence
Length of output: 19916
🏁 Script executed:
#!/bin/bash
# Search for old config keys in all files, including YAML resources
rg -i 'main-spawn-world|always-spawn' --type=yaml
# Also check for any old config examples or backups
fd -e yml -e yaml | xargs grep -l 'spawn' 2>/dev/null | head -10Repository: LewMC/Essence
Length of output: 428
🏁 Script executed:
#!/bin/bash
# Check the size and read config.yml
wc -l src/main/resources/config.yml
# Find the spawn configuration section
grep -n -A 10 -B 5 'spawn' src/main/resources/config.yml | head -100Repository: LewMC/Essence
Length of output: 747
🏁 Script executed:
#!/bin/bash
# Check git log for changes to config.yml
git log --oneline -20 -- src/main/resources/config.yml 2>/dev/null || echo "Git history not available"
# Search for any documentation or comments mentioning old config keys
rg -i 'main-spawn|always-spawn' --type=txt --type=md || echo "No references found"
# Check if there's a CHANGELOG or similar
fd -i 'changelog\|changes\|history' -e md -e txtRepository: LewMC/Essence
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Search for any TODO or FIXME related to spawn config migration
rg -i 'todo|fixme|migration' src/main/java/net/lewmc/essence/core/UtilUpdate.java
# Check if there are config version checks for this specific migration
rg -i 'config-version|spawn' src/main/java/net/lewmc/essence/core/UtilUpdate.java | head -20
# Search for any handling of teleportation spawn configuration loading
rg -B5 -A10 'teleportation.*spawn\|global-spawn' --type=java | head -50Repository: LewMC/Essence
Length of output: 1088
🏁 Script executed:
#!/bin/bash
# Search for where spawn config is loaded and used
rg -B5 -A10 'global-spawn|force-spawn' --type=java | head -80
# Check CommandSpawn to see how spawn settings are accessed
cat -n src/main/java/net/lewmc/essence/teleportation/spawn/CommandSpawn.java | head -100Repository: LewMC/Essence
Length of output: 13416
🏁 Script executed:
#!/bin/bash
# Check the getValue method implementation
rg -B10 -A10 'getValue.*teleportation.spawn.global-spawn.world' src/main/java/net/lewmc/essence/EssenceConfiguration.java
# Look at the getValue method definition
rg -B5 -A15 'protected.*getValue|private.*getValue' src/main/java/net/lewmc/essence/EssenceConfiguration.java | head -50Repository: LewMC/Essence
Length of output: 3556
Add configuration migration for old spawn settings.
The spawn configuration structure in config.yml uses a nested hierarchy (global-spawn.enabled, global-spawn.world, force-spawn.on-join, force-spawn.on-death), but no migration logic exists to transfer old flat configuration keys to this new structure. Users upgrading from previous versions will have their spawn settings reset to defaults, losing any custom configuration.
The configuration loader in EssenceConfiguration.java only checks for the new nested keys via getValue(), and migrateSpawns() handles spawn location data (data/spawns.yml) but not config.yml settings. Add migration logic to convert old spawn configuration keys to the new nested structure during config version updates.
|



Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.