GH-226 Add Lands support. #226
Conversation
WalkthroughThis update adds support for the Lands plugin by introducing new classes and logic for region handling. It removes all PacketEvents references and related code. The build scripts are updated to include the LandsAPI as a dependency and adjust plugin dependencies. The region provider system is improved to support multiple providers at once, allowing both Lands and WorldGuard regions to be recognized together. Several new classes handle region detection and event listening for Lands. No changes were made to public APIs except for some constructor and method signature updates to support the new features. Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/lands/LandsBridgeInitializer.java (2)
32-41: Add null check for Lands integrationConsider adding a null check for the Lands integration to handle cases where the Lands plugin might not be properly loaded.
@Override public void initialize() { LandsIntegration lands = LandsIntegration.of(this.plugin); + if (lands == null) { + this.plugin.getLogger().warning("Failed to initialize Lands integration."); + return; + } this.regionProvider = new LandsRegionProvider(lands); this.eventManager.subscribe(new LandsRegionController( this.fightManager, this.noticeService, lands )); }
43-45: Add null check for region providerThe getRegionProvider method should check if the provider is initialized before returning it.
public RegionProvider getRegionProvider() { + if (this.regionProvider == null) { + throw new IllegalStateException("Region provider not initialized. Call initialize() first."); + } return this.regionProvider; }eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/lands/LandsRegionProvider.java (1)
48-55: Consider using newer Java collections methodsThe
Collectors.toList()method is deprecated in newer Java versions. Consider usingtoList()orCollectors.toCollection(ArrayList::new)instead.@Override public Collection<Region> getRegions(World world) { return this.lands.getLands().stream() .map(land -> land.getContainer(world)) .filter(container -> container != null && !container.getChunks().isEmpty()) .map(container -> new LandChunksRegion(world, container.getChunks())) - .collect(Collectors.toList()); + .toList(); }eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/BridgeService.java (4)
51-63: Consider extracting plugin namesRepeating raw strings like
"Lands"and"WorldGuard"can invite typos later. A small constant for each name would tidy this up.
78-81: Guard against an empty provider list earlyIf nothing is found, you fall back to
DefaultRegionProvider, which is great.
For clarity, you could return right after adding the default to show there’s no need to continue.
83-88: Freeze the provider list before wrapping
CompositeRegionProviderreceives the mutableproviders. Locking it in prevents accidental edits later.- this.regionProvider = new CompositeRegionProvider(providers); + this.regionProvider = new CompositeRegionProvider(List.copyOf(providers));
97-104: Shorten the log messageThe info log already states a bridge was initialised; adding the plugin name twice feels redundant. Trimming it keeps logs clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
buildSrc/src/main/kotlin/eternalcombat-repositories.gradle.kts(1 hunks)eternalcombat-plugin/build.gradle.kts(2 hunks)eternalcombat-plugin/src/main/java/com/eternalcode/combat/CombatPlugin.java(3 hunks)eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/BridgeService.java(1 hunks)eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/lands/LandChunksRegion.java(1 hunks)eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/lands/LandsBridgeInitializer.java(1 hunks)eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/lands/LandsRegionController.java(1 hunks)eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/lands/LandsRegionProvider.java(1 hunks)eternalcombat-plugin/src/main/java/com/eternalcode/combat/region/CompositeRegionProvider.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/lands/LandsRegionController.java (1)
eternalcombat-plugin/src/main/java/com/eternalcode/combat/notification/NoticeService.java (1)
NoticeService(15-48)
eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/lands/LandsBridgeInitializer.java (2)
eternalcombat-plugin/src/main/java/com/eternalcode/combat/event/EventManager.java (1)
EventManager(8-37)eternalcombat-plugin/src/main/java/com/eternalcode/combat/notification/NoticeService.java (1)
NoticeService(15-48)
eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/BridgeService.java (5)
eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/lands/LandsBridgeInitializer.java (1)
LandsBridgeInitializer(11-46)eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/placeholder/FightTagPlaceholder.java (1)
FightTagPlaceholder(15-98)eternalcombat-plugin/src/main/java/com/eternalcode/combat/event/EventManager.java (1)
EventManager(8-37)eternalcombat-plugin/src/main/java/com/eternalcode/combat/notification/NoticeService.java (1)
NoticeService(15-48)eternalcombat-plugin/src/main/java/com/eternalcode/combat/region/CompositeRegionProvider.java (1)
CompositeRegionProvider(11-38)
🔇 Additional comments (15)
buildSrc/src/main/kotlin/eternalcombat-repositories.gradle.kts (1)
16-16: Good addition of JitPack repository.Adding the JitPack repository is necessary for fetching the Lands API dependency. This is the correct approach.
eternalcombat-plugin/build.gradle.kts (3)
49-50: Correctly added Lands dependency.The LandsAPI dependency has been properly added as compile-only, which is appropriate for a plugin integration.
55-55: Changed PacketEvents to compileOnly.Good change to make PacketEvents a compileOnly dependency since it's now a hard dependency of the plugin.
66-71: Updated plugin dependencies correctly.The changes to dependencies make sense:
- Simplified soft dependencies to just include Lands
- Added PacketEvents as a hard dependency
This matches the implementation changes in the code.
eternalcombat-plugin/src/main/java/com/eternalcode/combat/region/CompositeRegionProvider.java (3)
11-18: Good implementation of composite pattern.The CompositeRegionProvider is well designed to combine multiple region providers. The constructor properly accepts and stores the list of providers.
19-28: Efficient region lookup implementation.The getRegion method returns the first match found, which is efficient and avoids unnecessary lookups.
30-37: Thorough implementation of getRegions method.The getRegions method correctly collects all regions from all providers, ensuring no regions are missed.
eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/lands/LandChunksRegion.java (3)
9-33: Well-implemented region conversion.The LandChunksRegion class effectively converts Lands chunk coordinates into a region. The constructor correctly calculates the min/max coordinates by iterating through all chunks.
Note on line 31-32: The addition of 15 is correct as it accounts for the full width/length of the last chunk (each chunk is 16x16 blocks).
35-40: Simple and effective center calculation.The getCenter method provides a reasonable center point at y-level 64, which works well for most situations.
42-50: Proper min/max location implementations.Both getMin and getMax methods correctly return locations at the boundaries of the region, with appropriate y-levels (0 for min and world max height for max).
eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/lands/LandsRegionController.java (1)
31-61: The player movement event handler looks good!The event handling logic is correctly implemented to prevent players in combat from entering different Lands regions.
eternalcombat-plugin/src/main/java/com/eternalcode/combat/CombatPlugin.java (2)
101-109: Good local variable changes!The conversion from fields to local variables makes the code cleaner and follows better encapsulation practices.
123-133: Bridge service setup looks goodThe updated BridgeService construction with additional parameters helps support the new Lands integration.
eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/lands/LandsRegionProvider.java (1)
25-46: Region retrieval logic looks goodThe getRegion method properly handles null checks and empty collections before creating a region.
eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/BridgeService.java (1)
27-30: Nice addition of fresh servicesBringing in
EventManager,FightManager, andNoticeServicekeeps the class self-contained. Looks good.
...alcombat-plugin/src/main/java/com/eternalcode/combat/bridge/lands/LandsRegionController.java
Show resolved
Hide resolved
| import org.bukkit.event.player.PlayerMoveEvent; | ||
| import org.bukkit.event.player.PlayerTeleportEvent; | ||
|
|
||
| public class LandsRegionController implements Listener { |
There was a problem hiding this comment.
To wszystko jest do wywalenia, jeśli wysyłasz wiadomości i robisz to co miał robić KnockbackController to raczej problem jest z providerem
| import me.angeschossen.lands.api.LandsIntegration; | ||
| import org.bukkit.plugin.Plugin; | ||
|
|
||
| public class LandsBridgeInitializer implements BridgeInitializer { |
There was a problem hiding this comment.
Ta klasa też do wywalenia, wystarczy wszystko w lambda zrobić w BridgeService
| int _minX = Integer.MAX_VALUE, _minZ = Integer.MAX_VALUE; | ||
| int _maxX = Integer.MIN_VALUE, _maxZ = Integer.MIN_VALUE; | ||
|
|
||
| for (ChunkCoordinate chunkCoordinate : chunks) { |
There was a problem hiding this comment.
a to mamy gwarację, że region będzie kwadratowy/prostokątny skoro możemy zajmować chunki? bo nie jestem pewien czy tak zawsze będzie
| import java.util.List; | ||
| import java.util.Optional; | ||
|
|
||
| public class CompositeRegionProvider implements RegionProvider { |
There was a problem hiding this comment.
Fajne rozwiązanie podoba mi się
It's work well with the WorldGuard + Lands. (Two region provider at the same time.)
https://github.com/user-attachments/assets/d2521840-36e6-4395-bc4a-f8b306d3db79