-
-
Notifications
You must be signed in to change notification settings - Fork 0
GH-72 Create universal hologram provider instead of HoloEasy. #72
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request updates the project’s build setup by removing the Java 17 build script and adding a new Java 21 configuration. Maven repositories are changed, replacing some URLs with new ones. The API dependency switches from Spigot to Paper with adjusted scopes. The core plugin updates dependencies, plugin metadata, and removes test configurations. The main plugin class now uses an interface and a provider pattern for holograms. Configuration classes rename fields and add migrations. Several new classes support hologram management via different providers. Event classes are refactored to share a common base. Position handling moves from a custom record to using Bukkit’s Location directly. Various simplifications and cleanups are applied across the codebase, including removing unused serializers and test code. ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 2
🧹 Outside diff range and nitpick comments (2)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramNameUtil.java (2)
7-7
: Add a quick comment explaining the name format.A simple comment would help others understand what the format represents.
+ /** Format: heads_worldname_x_y_z */ private static final String HOLOGRAM_NAME_PREFIX = "heads_%s_%s_%s_%s";
17-19
: Consider using Pattern.compile for better performance.If this gets called often, a pre-compiled pattern would be faster.
+ private static final Pattern INVALID_CHARS = Pattern.compile("[^a-zA-Z0-9_-]"); + private static String sanitize(String input) { - return input.replaceAll("[^a-zA-Z0-9_-]", "_"); + return INVALID_CHARS.matcher(input).replaceAll("_"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
buildSrc/src/main/kotlin/lobbyheads-java-17.gradle.kts
(1 hunks)buildSrc/src/main/kotlin/lobbyheads-repositories.gradle.kts
(1 hunks)lobbyheads-api/build.gradle.kts
(1 hunks)lobbyheads-core/build.gradle.kts
(3 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/HeadsPlugin.java
(3 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/implementation/HeadsConfiguration.java
(0 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramNameUtil.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java
(3 hunks)
💤 Files with no reviewable changes (1)
- lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/implementation/HeadsConfiguration.java
✅ Files skipped from review due to trivial changes (1)
- buildSrc/src/main/kotlin/lobbyheads-java-17.gradle.kts
🧰 Additional context used
🪛 Gitleaks
lobbyheads-api/build.gradle.kts
8-8: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (20)
lobbyheads-api/build.gradle.kts (1)
8-8
: Nice improvement!
Using compileOnlyApi
for the Spigot API is perfect since the server already provides it at runtime. This keeps your plugin lightweight.
🧰 Tools
🪛 Gitleaks
8-8: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
buildSrc/src/main/kotlin/lobbyheads-repositories.gradle.kts (2)
14-14
: LGTM! Repository added for UniHologram Library.
The new Maven repository looks good and aligns with the PR objective of switching to UniHologram.
Line range hint 1-15
: Double-check dependency resolution.
Let's make sure all dependencies can still be resolved after removing the old repository.
✅ Verification successful
All dependencies look good after repository changes
I've checked all the build files and dependencies. None of them use packages from the removed repository, and all current dependencies are pointing to repositories that are still in the configuration. Everything should continue working as expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to packages from the old repository
rg -l "pl.minecodes" .
# Look for potential build files that might need updates
fd -e gradle -e gradle.kts
Length of output: 452
Script:
#!/bin/bash
# Let's check the dependencies in the build files to see if any still reference the old repository
rg -l "minecodes" lobbyheads-api/build.gradle.kts lobbyheads-core/build.gradle.kts
# Also check the actual dependency declarations
rg "implementation|api|compile" lobbyheads-api/build.gradle.kts lobbyheads-core/build.gradle.kts
Length of output: 1964
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramNameUtil.java (1)
1-5
: LGTM! Good choice on package visibility.
The class is properly scoped to its package, which keeps things neat and tidy.
lobbyheads-core/build.gradle.kts (2)
69-70
: Check if ProtocolLib is still needed.
The removal of ProtocolLib dependency needs verification since it's still downloaded in the runServer task.
#!/bin/bash
# Search for ProtocolLib usage in the code
rg -l "ProtocolLib|protocollib" --type java
37-39
: Looks good! Let's verify the dependency.
The new hologram library is properly configured with the CMILib exclusion.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/HeadsPlugin.java (3)
26-27
: Looking good! Clean import additions.
The new imports match perfectly with the switch to UniHologram.
Also applies to: 35-35
50-51
: Nice formatting improvement!
The split makes the code easier to read.
74-77
: Let's double-check the hologram setup.
The switch to UniHologram looks good, but let's verify that all hologram-related code has been updated.
✅ Verification successful
All hologram-related code has been properly updated
The codebase shows a complete transition to UniHologram:
- All hologram classes use the new UniHologram imports
- No traces of HoloEasy found
- HologramService and HologramController properly integrated with the new provider
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining HoloEasy references
rg -i "holoeasy"
# Look for other hologram-related classes that might need updates
rg "Hologram" -g "*.java"
Length of output: 6751
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java (11)
3-4
: Import added successfully
The static import for generateHologramName
looks good.
12-15
: Updated imports to UniHologram library
The new imports from UniHologram are correctly added.
30-32
: New fields added
Fields headManager
and provider
are properly added.
33-39
: Constructor updated
The constructor now includes headManager
and provider
, which is great.
43-44
: Fields initialized correctly
Fields headManager
and provider
are initialized properly.
48-51
: Code update looks good
The placeholder handling and location conversion are correctly implemented.
53-55
: Hologram creation updated successfully
Holograms are now created using the new provider, which is correct.
63-66
: Loading holograms updated
The hologram loading loop using headManager
looks good.
70-71
: Check the hologram type casting
Please ensure that casting to Hologram<Location>
is safe to avoid any issues.
80-83
: Hologram updates look correct
The holograms are updated properly with the new manager.
90-94
: Confirm Java version compatibility
The instanceof
pattern matching requires Java 16 or higher. Make sure the project supports this version.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramNameUtil.java
Show resolved
Hide resolved
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: 0
🧹 Outside diff range and nitpick comments (1)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java (1)
70-71
: Consider adding error handlingThe hologram removal could benefit from some basic error logging when the hologram isn't found.
- this.provider.getHologram(generateHologramName(position)) - .ifPresent(hologram -> ((Hologram<Location>) hologram).clear()); + this.provider.getHologram(generateHologramName(position)) + .ifPresentOrElse( + hologram -> ((Hologram<Location>) hologram).clear(), + () -> System.out.println("No hologram found at position: " + position) + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java
(2 hunks)
🔇 Additional comments (5)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java (5)
12-15
: Nice job with the new imports!
The switch to UniHologram's classes looks clean and organized.
30-31
: Good field organization!
The switch from pool-based to provider-based approach looks cleaner.
33-44
: Constructor looks better now!
The removal of direct Plugin dependency and addition of HologramProvider makes the class more focused and easier to test.
90-94
: Visibility handling looks good!
The new visibility system using PlayerVisibility interface is a nice improvement.
48-55
: Double-check the text processing logic
While the hologram creation looks good, we should verify that the text processing works the same way as before.
Let's check if the text processing matches the old implementation:
✅ Verification successful
Let me check for any previous text processing logic to compare with.
Text processing looks good! 👍
The old and new implementations handle text processing in the same way:
- Replace
{PLAYER}
with player name - Process PlaceholderAPI placeholders
- Deserialize with MiniMessage
- Serialize with section serializer
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any other text processing logic in the old code
rg -A 3 "PlaceholderAPI.setPlaceholders"
Length of output: 781
Script:
#!/bin/bash
# Look for git history of the file to see previous implementation
git log -p lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java
Length of output: 13117
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java
Outdated
Show resolved
Hide resolved
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: 1
🧹 Nitpick comments (1)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/block/BlockController.java (1)
57-57
: Unused variable
skullData
is never used. Remove it if it’s no longer needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
buildSrc/src/main/kotlin/lobbyheads-repositories.gradle.kts
(1 hunks)lobbyheads-core/build.gradle.kts
(3 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/HeadsPlugin.java
(3 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/ConfigurationService.java
(2 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/implementation/HeadsConfiguration.java
(2 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/serializer/SoundSerializer.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/block/BlockController.java
(2 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java
(2 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProvider.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderNotFoundException.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderPicker.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/fancyholograms/FancyHologramsProvider.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/sound/SoundController.java
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- buildSrc/src/main/kotlin/lobbyheads-repositories.gradle.kts
- lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/implementation/HeadsConfiguration.java
🔇 Additional comments (33)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/block/BlockController.java (3)
9-10
: Good imports
Using UUID
and TimeUnit
is a neat addition. No issues here.
60-61
: Smooth scheduling
Placing the update call in a scheduled task helps avoid blocking. Nice approach!
64-65
: Clear ownership setting
Assigning the skull’s owner with the player’s UUID is straightforward and effective.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java (13)
3-3
: Neat static import!
It simplifies name generation nicely.
8-9
: Good choice of direct imports.
This keeps the code clear and organized.
13-13
: Import looks fine.
UUID usage seems straightforward.
26-28
: Well-structured fields.
Declaring them as final is a helpful design choice.
29-35
: Constructor is nicely simplified.
Easy to understand and maintain.
39-40
: Clear assignments.
Keeps references easy to track.
44-44
: Consider lightening PAPI dependency.
A flexible bridge might help in optional usage.
45-47
: MiniMessage and serialization look great.
It keeps the text formatting flexible.
49-49
: Provider usage is tidy.
This new approach is a neat improvement.
55-55
: Loop logic is clear.
Neatly iterates over heads.
62-62
: Removal step is straightforward.
This is easy to follow.
71-71
: Keeps all holograms in sync.
Nice way to ensure updates.
77-77
: Offset is a nice touch.
Helps positioning the hologram better.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderNotFoundException.java (1)
1-7
: Custom exception looks good.
It clarifies missing provider scenarios.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProvider.java (1)
1-13
: Interface-based design is great.
It fosters easy swapping of implementations.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderPicker.java (1)
1-17
: Dynamic picking is clever.
Makes it simpler to select a suitable provider at runtime.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/serializer/SoundSerializer.java (1)
1-30
: Neat serialization approach.
Makes sound configurations easy and consistent.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/sound/SoundController.java (2)
3-3
: Everything looks fine.
Nice addition of the new XSound
import. No problems spotted here.
33-33
: Clear and simple usage of XSound
.
This is easier to read and maintain than raw sound calls.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/ConfigurationService.java (2)
3-3
: Good import placement.
Neat to see SoundSerializer
integrated here.
35-35
: Registration of SoundSerializer
is clear.
This looks straightforward. Ensure the sounds load as expected.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/fancyholograms/FancyHologramsProvider.java (1)
1-67
: Nice provider-based approach.
This new class is laid out clearly and uses FancyHolograms well. Good asynchronous handling for hologram creation and updating. Looks friendly to maintain.
lobbyheads-core/build.gradle.kts (5)
34-34
: Dependency for XSeries
looks good.
This is the correct version and should help with sound features.
36-36
: Compile-only FancyHolograms is well-chosen.
This ensures flexibility without forcing the plugin dependency.
66-66
: Depend on PlaceholderAPI
only.
Seems like a simpler setup. Everything else is optional now.
67-67
: Great use of softDepend
.
This broadens compatibility with other plugins nicely.
107-109
: Re-enable quality checks and tests if possible.
Similar to a prior comment about turning tests and checkstyle back on. It keeps the project tidy.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/HeadsPlugin.java (3)
18-19
: Looking good! 👍
These imports match perfectly with the switch to the new hologram system.
49-50
: Nice formatting! 👍
The split makes the code easier to read.
76-82
: Clean and clear service setup! 👍
The new HologramService setup looks great with the hologram provider parameter.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/HeadsPlugin.java
Show resolved
Hide resolved
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: 6
♻️ Duplicate comments (2)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/HeadsPlugin.java (1)
72-73
: Provider selection needs fallbackAs mentioned before, the plugin will crash if no hologram plugin is available. Consider adding a no-op provider as fallback.
lobbyheads-core/build.gradle.kts (1)
84-106
: Quality checks still disabledThe checkstyle and test tasks are still commented out, which could let issues slip through.
🧹 Nitpick comments (5)
README.md (2)
9-9
: Extra slash in Discord badge URLThere are two consecutive slashes after
main/
, producing
…/main//chat-with-us-on-discord.svg
. Some CDNs tolerate this, but others return 404.-https://raw.githubusercontent.com/vLuckyyy/badges/main//chat-with-us-on-discord.svg +https://raw.githubusercontent.com/vLuckyyy/badges/main/chat-with-us-on-discord.svg
60-60
: Tone tweak (optional)“One exclamation mark is plenty 🙌.”
Consider dropping it for a slightly calmer tone:-We wholeheartedly welcome your contributions to LobbyHeads! +We wholeheartedly welcome your contributions to LobbyHeads.lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderPicker.java (2)
7-7
: Consider making this class final.Since this is a utility class that shouldn't be extended, making it final would be good practice.
-public class HologramProviderPicker { +public final class HologramProviderPicker {
9-19
: Good provider selection logic, but consider making it static.The method doesn't use any instance state, so it could be static. Also, the priority order (FancyHolograms before DecentHolograms) works well but could use a comment explaining why.
- public HologramProvider pickProvider(Plugin plugin) { + public static HologramProvider pickProvider(Plugin plugin) {lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/decentholograms/DecentHologramsProvider.java (1)
36-54
: Consider adding error handlingThe hologram operations could fail if the API has issues. Maybe wrap the DHAPI calls in try-catch blocks to log any problems?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (47)
.editorconfig
(1 hunks).github/CONTRIBUTING.md
(1 hunks).whitesource
(0 hunks)LICENSE
(1 hunks)README.md
(3 hunks)buildSrc/src/main/kotlin/lobbyheads-java-17.gradle.kts
(0 hunks)buildSrc/src/main/kotlin/lobbyheads-java-21.gradle.kts
(1 hunks)buildSrc/src/main/kotlin/lobbyheads-java-unit-test.gradle.kts
(0 hunks)buildSrc/src/main/kotlin/lobbyheads-repositories.gradle.kts
(1 hunks)gradle/wrapper/gradle-wrapper.properties
(1 hunks)lobbyheads-api/build.gradle.kts
(1 hunks)lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/LobbyHeadsApi.java
(1 hunks)lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/LobbyHeadsApiProvider.java
(1 hunks)lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/Head.java
(2 hunks)lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/HeadManager.java
(1 hunks)lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadCreateEvent.java
(1 hunks)lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadEvent.java
(1 hunks)lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadRemoveEvent.java
(1 hunks)lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadUpdateEvent.java
(1 hunks)lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/position/Position.java
(0 hunks)lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/position/PositionAdapter.java
(0 hunks)lobbyheads-core/build.gradle.kts
(3 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/HeadsPlugin.java
(5 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/ConfigurationService.java
(2 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/implementation/HeadsConfiguration.java
(4 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/migration/HeadsConfigMigration_2025_07_17.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/migration/HeadsPositionMigration_2025_07_17.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/serializer/HeadSerializer.java
(0 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/serializer/PositionTransformer.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/HeadController.java
(5 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/HeadManagerImpl.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/HeadRepositoryImpl.java
(2 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/block/BlockController.java
(3 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/block/BlockService.java
(2 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/command/HeadCommand.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramController.java
(2 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramNameUtil.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderPicker.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/decentholograms/DecentHologramsProvider.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/particle/ParticleController.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/sound/SoundController.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/notification/NotificationAnnouncer.java
(1 hunks)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/updater/UpdaterNotificationController.java
(1 hunks)lobbyheads-core/test/com/eternalcode/lobbyheads/position/PositionAdapterTest.java
(0 hunks)lobbyheads-core/test/com/eternalcode/lobbyheads/position/PositionTest.java
(0 hunks)renovate.json
(1 hunks)
💤 Files with no reviewable changes (8)
- .whitesource
- buildSrc/src/main/kotlin/lobbyheads-java-17.gradle.kts
- buildSrc/src/main/kotlin/lobbyheads-java-unit-test.gradle.kts
- lobbyheads-core/test/com/eternalcode/lobbyheads/position/PositionAdapterTest.java
- lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/serializer/HeadSerializer.java
- lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/position/PositionAdapter.java
- lobbyheads-core/test/com/eternalcode/lobbyheads/position/PositionTest.java
- lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/position/Position.java
✅ Files skipped from review due to trivial changes (8)
- .github/CONTRIBUTING.md
- gradle/wrapper/gradle-wrapper.properties
- LICENSE
- buildSrc/src/main/kotlin/lobbyheads-java-21.gradle.kts
- renovate.json
- lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadEvent.java
- lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/notification/NotificationAnnouncer.java
- lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/migration/HeadsConfigMigration_2025_07_17.java
🚧 Files skipped from review as they are similar to previous changes (6)
- lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/ConfigurationService.java
- lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/sound/SoundController.java
- lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramNameUtil.java
- lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/block/BlockController.java
- lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/implementation/HeadsConfiguration.java
- lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java
🧰 Additional context used
🧬 Code Graph Analysis (3)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramController.java (3)
lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadCreateEvent.java (1)
HeadCreateEvent
(11-27)lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadRemoveEvent.java (1)
HeadRemoveEvent
(14-30)lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadUpdateEvent.java (1)
HeadUpdateEvent
(11-27)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderPicker.java (2)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/decentholograms/DecentHologramsProvider.java (1)
DecentHologramsProvider
(13-55)lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/fancyholograms/FancyHologramsProvider.java (1)
FancyHologramsProvider
(15-67)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/HeadsPlugin.java (1)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderPicker.java (1)
HologramProviderPicker
(7-20)
🪛 LanguageTool
README.md
[style] ~60-~60: Using many exclamation marks might seem excessive (in this case: 5 exclamation marks for a text that’s 3038 characters long)
Context: ...welcome your contributions to LobbyHeads! Please refer to our [contribution guide...
(EN_EXCESSIVE_EXCLAMATION)
🔇 Additional comments (33)
README.md (1)
25-30
: Confirm GIF branch pathThe GIFs now reference the
master
branch. If the default branch ismain
, these links will break.-https://github.com/EternalCodeTeam/LobbyHeads/blob/master/ +https://github.com/EternalCodeTeam/LobbyHeads/blob/main/Please double-check that the images load after the merge.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/updater/UpdaterNotificationController.java (1)
31-31
: Configuration field ‘checkUpdates’ verifiedI checked and found
public boolean checkUpdates = true;
in HeadsConfiguration.java, so the rename is correct and everything aligns. No further changes needed—approving this update.lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/serializer/PositionTransformer.java (1)
3-3
: Position abstraction update looks good.The switch to
commons.bukkit.position.Position
and the class rename toPositionTransformer
aligns with the broader refactoring effort to use standardized position handling.Also applies to: 9-9
lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/LobbyHeadsApi.java (1)
4-4
: Excellent API improvements!Adding Javadoc documentation and
@NotNull
annotations makes the API much clearer for developers. These are great best practices for public interfaces.Also applies to: 6-8, 11-11
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/particle/ParticleController.java (1)
23-23
: Approved: Method and config renames are in place
Everything looks great –getPlayerUuid
and theheadSettings
section are used consistently across the codebase. No further changes needed.lobbyheads-api/build.gradle.kts (2)
2-2
: Java 21 upgrade looks good!This modernizes the build to use Java 21, which is a good step forward.
8-8
: Paper API migration is a solid improvement.Switching from Spigot to Paper API provides better features and performance. The
compileOnlyApi
scope is correct here - it prevents Paper from being transitively exposed to API consumers while still allowing compilation.lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/HeadRepositoryImpl.java (2)
8-8
: Great thread safety improvement!Converting to
CopyOnWriteArrayList
prevents concurrent modification issues. Smart defensive programming here.Also applies to: 19-21
26-29
: Excellent atomic operation design.Moving both list mutations and config saves into the same async blocks ensures consistency. The
updateHead
method now handles both update and add cases cleanly within the async context.Also applies to: 34-37, 42-53
lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/HeadManager.java (2)
6-7
: Excellent API documentation and nullability contracts.The
@NotNull
and@Nullable
annotations make the API much safer to use. Clear documentation helps developers understand expected behavior.Also applies to: 17-17, 22-22, 27-27, 32-32, 37-37
9-11
: Well-written Javadoc documentation.The interface and method documentation is clear and concise. Developers will easily understand how to use this API.
Also applies to: 14-16, 19-21, 24-26, 29-31, 34-36
lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadCreateEvent.java (2)
11-11
: Good inheritance refactoring.Extending
HeadEvent
reduces code duplication and creates a cleaner event hierarchy. The constructor delegation is properly implemented.Also applies to: 15-16
19-19
: Proper method overrides.The
@Override
annotations are correctly placed and the method signatures match the parent class.Also applies to: 24-24
lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadRemoveEvent.java (3)
9-13
: Excellent documentation clarification.The note about UUID referring to the head owner (not remover) prevents common confusion. Very helpful for API users.
14-14
: Consistent inheritance pattern.Extending
HeadEvent
maintains consistency with other event classes and reduces duplication. The constructor delegation follows the established pattern.Also applies to: 18-19
22-22
: Proper method overrides.The
@Override
annotations are correctly applied and method signatures are consistent.Also applies to: 27-27
.editorconfig (1)
1-31
: Great editor configuration improvements!The new Java-specific settings will help maintain consistent code style across the project. The IntelliJ IDEA settings look comprehensive and follow good practices, especially the import organization and final parameter generation.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramController.java (2)
29-34
: Nice method name improvement!Renaming
createHologram
toonHeadCreate
makes it much clearer that this is an event handler. The variable renaming fromplayer
toplayerUuid
andofflinePlayer
toplayer
also improves readability.
33-33
: Good configuration path update.The change from
config.headSection.defaultHeadFormat
toconfig.headSettings.defaultHeadFormat
aligns with the configuration restructuring mentioned in the PR summary.lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/HeadController.java (3)
23-35
: Excellent interface-based design improvement!Switching from
HeadManagerImpl
to theHeadManager
interface follows good dependency inversion principles and makes the code more testable and flexible.
52-78
: Good simplification by removing PositionAdapter calls.Passing
location
directly instead of converting it withPositionAdapter.convert()
simplifies the code nicely. The updated notification message keys also align well with the configuration restructuring.
73-73
: Nice fix for the method name casing.Correcting
getPlayerUUID()
togetPlayerUuid()
follows proper Java naming conventions.lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/LobbyHeadsApiProvider.java (3)
5-14
: Excellent utility class design!Making the class final with a private constructor that throws an exception properly prevents instantiation. The Javadoc is also a nice touch for API clarity.
20-26
: Great thread safety improvement!The volatile field with local variable pattern is a good approach to prevent race conditions. The null check with clear exception message makes debugging easier.
31-36
: Proper initialization safety checks.The null check prevents double initialization, which is important for maintaining API consistency. Good defensive programming.
lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadUpdateEvent.java (1)
8-27
: Nice refactoring!The change to extend
HeadEvent
makes the code cleaner by sharing common fields. Good use of@NotNull
annotations too.lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/block/BlockService.java (1)
5-25
: Good move to use the interface!Using
HeadManager
instead of the implementation class makes the code more flexible. The simplified method signatures look cleaner too.lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/decentholograms/DecentHologramsProvider.java (1)
13-34
: Clean implementation of the provider pattern!Good job scheduling operations on the main thread for safety. The immediate visibility to all players makes sense for lobby heads.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/migration/HeadsPositionMigration_2025_07_17.java (1)
12-120
: Solid migration implementation!Great job handling all the edge cases - from parsing failures to already-migrated data. The regex pattern looks spot on for the old format.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/HeadsPlugin.java (1)
41-81
: Great improvements to the plugin structure!Love the switch to interfaces and the cleaner dependency injection. The provider pattern for holograms is much more flexible now.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/command/HeadCommand.java (1)
7-89
: Nice cleanup!The refactoring looks good - simpler naming with
announcer
, better null checks for target blocks, and using the newergetTargetBlockExact
API. The permission check extraction to a private method also makes the code cleaner.lobbyheads-core/build.gradle.kts (1)
64-64
: Check soft dependencies setupMake sure the soft dependencies for DecentHolograms and FancyHolograms are handled properly in your hologram provider picker logic.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/HeadManagerImpl.java (1)
3-89
: Clean refactoring!Good job simplifying the API to use
Location
everywhere and keeping the internalPosition
conversion hidden. The unmodifiable collection return is also a nice touch for safety.
return new DecentHologramsProvider(plugin); | ||
} | ||
|
||
throw new HologramProviderNotFoundException("No supported hologram plugin found."); |
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.
Missing import for HologramProviderNotFoundException.
The code references HologramProviderNotFoundException
but doesn't import it. This will cause a compilation error.
Add the missing import at the top of the file.
🤖 Prompt for AI Agents
In
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderPicker.java
at line 18, the class HologramProviderNotFoundException is used but not
imported, causing a compilation error. Add the appropriate import statement for
HologramProviderNotFoundException at the top of the file with the other imports
to resolve this issue.
public Head() { | ||
this.world = null; | ||
this.x = this.y = this.z = 0; | ||
this.yaw = this.pitch = 0; | ||
} |
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.
Watch out for null world!
The no-arg constructor sets world
to null, which will cause problems when getLocation()
is called later.
🤖 Prompt for AI Agents
In lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/Head.java around
lines 18 to 22, the no-argument constructor sets the world field to null, which
will cause issues when getLocation() is called. To fix this, initialize the
world field with a valid default World instance or modify the constructor to
require a non-null world parameter, ensuring that world is never null when used
later.
return this.world.equals(other.world) && | ||
this.x == other.x && this.y == other.y && this.z == other.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.
Missing null check in equals
The equals
method doesn't check if world
is null before calling .equals()
.
Add a null check:
-return this.world.equals(other.world) &&
+return Objects.equals(this.world, other.world) &&
this.x == other.x && this.y == other.y && this.z == other.z;
📝 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.
return this.world.equals(other.world) && | |
this.x == other.x && this.y == other.y && this.z == other.z; | |
} | |
return Objects.equals(this.world, other.world) && | |
this.x == other.x && this.y == other.y && this.z == other.z; | |
} |
🤖 Prompt for AI Agents
In lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/Head.java around
lines 64 to 66, the equals method calls this.world.equals(other.world) without
checking if this.world is null, which can cause a NullPointerException. Fix this
by adding a null check for this.world before calling equals, for example by
using Objects.equals(this.world, other.world) or explicitly checking if
this.world is null and comparing accordingly.
public @NotNull Location getLocation() { | ||
return new Location( | ||
org.bukkit.Bukkit.getWorld(this.world), | ||
this.x, this.y, this.z, | ||
this.yaw, this.pitch | ||
); | ||
} |
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.
Add null safety for world
The getLocation()
method could throw NPE if world is null or if Bukkit can't find the world.
Consider adding a null check:
public @NotNull Location getLocation() {
+ if (this.world == null) {
+ throw new IllegalStateException("World is not set");
+ }
+ var world = org.bukkit.Bukkit.getWorld(this.world);
+ if (world == null) {
+ throw new IllegalStateException("World '" + this.world + "' not found");
+ }
return new Location(
- org.bukkit.Bukkit.getWorld(this.world),
+ world,
this.x, this.y, this.z,
this.yaw, this.pitch
);
}
📝 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.
public @NotNull Location getLocation() { | |
return new Location( | |
org.bukkit.Bukkit.getWorld(this.world), | |
this.x, this.y, this.z, | |
this.yaw, this.pitch | |
); | |
} | |
public @NotNull Location getLocation() { | |
if (this.world == null) { | |
throw new IllegalStateException("World is not set"); | |
} | |
var world = org.bukkit.Bukkit.getWorld(this.world); | |
if (world == null) { | |
throw new IllegalStateException("World '" + this.world + "' not found"); | |
} | |
return new Location( | |
world, | |
this.x, this.y, this.z, | |
this.yaw, this.pitch | |
); | |
} |
🤖 Prompt for AI Agents
In lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/Head.java around
lines 35 to 41, the getLocation() method may throw a NullPointerException if the
world field is null or if Bukkit.getWorld(this.world) returns null. Add a null
check for the world object returned by Bukkit.getWorld(this.world) before
creating the Location instance. If the world is null, handle it gracefully by
either returning null, throwing a custom exception, or using an Optional to
indicate absence, depending on the intended design.
No description provided.