GH-72 Use UniHologram Library instead of HoloEasy. #72
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces several changes across multiple files in the project. In the Gradle build scripts, a new group ID and version number are defined for the project, and the list of Maven repositories is updated by removing one and adding another. Additionally, the dependency on the Spigot API in the The New utility functions for generating hologram names are introduced, and the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 java37-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.
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
vLuckyyy
changed the title
| @@ -5,7 +5,7 @@ plugins { | |||
| } | |||
|
|
|||
| dependencies { | |||
| api("org.spigotmc:spigot-api:1.20.2-R0.1-SNAPSHOT") | |||
| compileOnlyApi("org.spigotmc:spigot-api:1.20.2-R0.1-SNAPSHOT") | |||
| String string = PlaceholderAPI.setPlaceholders(player, replace); | ||
| Component deserialize = this.miniMessage.deserialize(string); | ||
| String serialize = AdventureUtil.SECTION_SERIALIZER.serialize(deserialize); | ||
| String placeholderText = PlaceholderAPI.setPlaceholders(player, headName.replace("{PLAYER}", player.getName())); |
There was a problem hiding this comment.
kurczaki można było by tu zrobić jakiś simple bridge żeby PAPI nie musiało być dependem
| @@ -5,7 +5,7 @@ plugins { | |||
| } | |||
|
|
|||
| dependencies { | |||
| api("org.spigotmc:spigot-api:1.20.2-R0.1-SNAPSHOT") | |||
| compileOnlyApi("org.spigotmc:spigot-api:1.20.2-R0.1-SNAPSHOT") | |||
No description provided.