Skip to content
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

GH-204 Add Border #204

Open
wants to merge 15 commits into
base: 2.0
Choose a base branch
from
Open

GH-204 Add Border #204

wants to merge 15 commits into from

Conversation

Rollczi
Copy link
Member

@Rollczi Rollczi commented Feb 16, 2025

No description provided.

Copy link

coderabbitai bot commented Feb 16, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This pull request introduces several changes across project configuration, API, and plugin functionality. The build setup has been updated by adding a new dependency constant for PAPERLIB, revising repository declarations, and removing legacy checkstyle configurations. The region API now includes additional methods to retrieve region boundaries and check if locations are contained within those boundaries. Moreover, the RegionProvider interface has been enhanced to return multiple regions from a given world. In the plugin code, new dependencies were added, and various new classes and controllers were introduced to manage border visualization—using blocks and particles—and lazy border point updates. Knockback functionality has been expanded with new services and settings, and region controllers have been refactored to support these changes. Lastly, region creation logic has been refactored in both default and WorldGuard-based providers to streamline spatial management in combat scenarios.


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

# Conflicts:
#	eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/knockback/KnockbackRegionController.java
@Rollczi Rollczi changed the title Add Border GH-204 Add Border Feb 16, 2025
@vLuckyyy
Copy link
Member

@coderabbitai review

Copy link

coderabbitai bot commented Feb 16, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (36)
eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderSettings.java (2)

8-11: Consider making the refresh delay configurable.

The one-second delay is hardcoded. Making it configurable would allow for easier adjustments based on performance needs.

-    default Duration indexRefreshDelay() {
-        return Duration.ofSeconds(1);
-    }
+    Duration indexRefreshDelay();

13-13: Add documentation to explain the distance method.

A brief comment would help explain what this distance represents and its units.

+    /**
+     * Gets the border distance in blocks.
+     * @return the distance from the center to the border
+     */
     double distance();
eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/particle/BorderParticleController.java (4)

30-32: Consider making constants private and adding documentation.

These constants are only used within this class. Making them private would better encapsulate the implementation details.

-    public static final PacketEventsAPI<?> PACKET_EVENTS_API = PacketEvents.getAPI();
-    public static final PlayerManager PLAYER_MANAGER = PACKET_EVENTS_API.getPlayerManager();
-    public static final Vector3f OFFSET = new Vector3f(0.1f, 0.1f, 0.1f);
+    private static final PacketEventsAPI<?> PACKET_EVENTS_API = PacketEvents.getAPI();
+    private static final PlayerManager PLAYER_MANAGER = PACKET_EVENTS_API.getPlayerManager();
+    // Defines the spread of particles in each direction
+    private static final Vector3f OFFSET = new Vector3f(0.1f, 0.1f, 0.1f);

38-42: Add parameter validation and extract magic number.

Two quick improvements would make this more robust:

  1. Validate constructor parameters
  2. Make the update interval configurable
+    private static final Duration UPDATE_INTERVAL = Duration.ofMillis(200);
+
     public BorderParticleController(BorderService borderService, Scheduler scheduler, Server server) {
+        this.borderService = Objects.requireNonNull(borderService, "borderService cannot be null");
+        this.server = Objects.requireNonNull(server, "server cannot be null");
-        this.borderService = borderService;
-        this.server = server;
-        scheduler.timerAsync(() -> this.updatePlayers(), Duration.ofMillis(200), Duration.ofMillis(200));
+        Objects.requireNonNull(scheduler, "scheduler cannot be null");
+        scheduler.timerAsync(this::updatePlayers, UPDATE_INTERVAL, UPDATE_INTERVAL);
     }

61-75: Add null check for player and use method reference.

Two small improvements:

  1. Check if player is still online
  2. Use method reference for playParticle (as suggested in previous review)
     private void updatePlayers() {
         for (UUID uuid : this.playersToUpdate) {
             Player player = this.server.getPlayer(uuid);
+            if (player == null) {
+                this.playersToUpdate.remove(uuid);
+                continue;
+            }
             Set<BorderPoint> border = this.borderService.getActiveBorder(player);

             if (border.isEmpty()) {
                 this.playersToUpdate.remove(uuid);
                 continue;
             }

             for (BorderPoint point : border) {
-                playParticle(player, point);
+                this.playParticle(player, point);
             }
         }
     }

77-91: Extract magic numbers in particle configuration.

The particle count (5) and other configuration values should be named constants.

+    private static final int PARTICLE_COUNT = 5;
+    private static final float PARTICLE_SIZE = 1.0F;
+    private static final float EXTRA = 0.0f;
+
     private void playParticle(Player player, BorderPoint point) {
         Color color = BorderColorUtil.xyzToColor(point.x(), point.y(), point.z());
-        Particle<ParticleDustData> dust = new Particle<>(ParticleTypes.DUST, new ParticleDustData(1F, color));
+        Particle<ParticleDustData> dust = new Particle<>(ParticleTypes.DUST, new ParticleDustData(PARTICLE_SIZE, color));
         WrapperPlayServerParticle particle = new WrapperPlayServerParticle(
             dust,
             true,
             new Vector3d(point.x(), point.y(), point.z()),
             OFFSET,
-            0.0f,
-            5,
+            EXTRA,
+            PARTICLE_COUNT,
             true
         );
eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/particle/BorderColorUtil.java (2)

45-59: Consider using a more accurate color distance formula.

The current RGB distance calculation is simple but might not match human color perception. Consider using CIE94 or CIEDE2000 for better results.


61-67: Extract magic numbers as constants.

The numbers 0.05, 0.8f, and 0.2f could be named constants to improve readability.

+    private static final float COORDINATE_SCALE = 0.05f;
+    private static final float BASE_BRIGHTNESS = 0.8f;
+    private static final float HEIGHT_BRIGHTNESS_FACTOR = 0.2f;
+
     private static java.awt.Color xyzToRGB(int x, int y, int z) {
-        float hue = (float) (((Math.sin(x * 0.05) + Math.cos(z * 0.05)) * 0.5 + 0.5) % 1.0);
+        float hue = (float) (((Math.sin(x * COORDINATE_SCALE) + Math.cos(z * COORDINATE_SCALE)) * 0.5 + 0.5) % 1.0);
         float saturation = 1.0f;
-        float brightness = 0.8f + 0.2f * Math.max(0.0f, Math.min(1.0f, (float) y / 255));
+        float brightness = BASE_BRIGHTNESS + HEIGHT_BRIGHTNESS_FACTOR * Math.max(0.0f, Math.min(1.0f, (float) y / 255));
eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/particle/BorderBlockController.java (4)

45-46: Consider a dedicated lock type for clarity.

Using a plain Object for locking works but using a ReentrantLock or a dedicated lock class might simplify reading and maintenance.


51-55: Check update frequency.

Running the update task every 200ms could be frequent for large borders or many players. A longer interval may reduce potential lag.


60-60: Use this. for consistency.

For styling consistency, consider adding this. before method calls, as done elsewhere in the class.


108-124: Avoid sending large updates.

Sending a big set of changes every cycle can cause performance issues. Consider handling fewer blocks or only changed areas to keep things smooth.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/region/DefaultRegionProvider.java (2)

29-33: Single region returned
This method only returns the spawn region. If future needs require multiple regions, consider expanding this approach.


35-44: Check off-by-one behavior
+ this.radius - 1 might skip the outer border. Confirm it matches your intended region size.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/knockback/KnockbackOutsideRegionGenerator.java (1)

61-63: Random usage
Consider using a shared Random instance for more consistent results or testing.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/knockback/KnockbackRegionController.java (1)

63-68: Method name check
konckbackLater appears misspelled. Consider renaming to stay consistent.

eternalcombat-api/src/main/java/com/eternalcode/combat/region/Region.java (1)

20-22: Consider inclusive bounds check

The max bounds check uses < which might miss edge cases. Consider using <= for more inclusive bounds.

-        return x >= min.getX() && x < max.getX()
-            && y >= min.getY() && y < max.getY()
-            && z >= min.getZ() && z < max.getZ();
+        return x >= min.getX() && x <= max.getX()
+            && y >= min.getY() && y <= max.getY()
+            && z >= min.getZ() && z <= max.getZ();
eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/knockback/KnockbackService.java (1)

52-56: Consider adding vertical knockback configuration

The vertical knockback (0.5) is hardcoded. Consider making it configurable like the multiplier.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/region/WorldGuardRegionProvider.java (4)

50-63: Simplify the stream operation.

The stream operation can be made more concise.

-            .stream()
-            .filter(region -> isCombatRegion(region))
-            .map(region -> (Region) new WorldGuardRegion(world, region))
-            .toList();
+            .stream()
+            .filter(this::isCombatRegion)
+            .map(region -> new WorldGuardRegion(world, region))
+            .toList();

83-92: Simplify the center calculation.

The center calculation can be more concise by combining the coordinate calculations.

-            double x = (double) (min.getX() + max.getX()) / 2;
-            double z = (double) (min.getZ() + max.getZ()) / 2;
-            double y = (double) (min.getY() + max.getY()) / 2;
-
-            return new Location(this.context, x, y, z);
+            return new Location(this.context,
+                (min.getX() + max.getX()) / 2.0,
+                (min.getY() + max.getY()) / 2.0,
+                (min.getZ() + max.getZ()) / 2.0);

61-61: Remove unnecessary cast.

The cast to Region is not needed as WorldGuardRegion already implements Region.

-            .map(region -> (Region) new WorldGuardRegion(world, region))
+            .map(region -> new WorldGuardRegion(world, region))

83-92: Simplify the center calculation.

The center calculation can be more concise by combining the coordinate calculations.

-            double x = (double) (min.getX() + max.getX()) / 2;
-            double z = (double) (min.getZ() + max.getZ()) / 2;
-            double y = (double) (min.getY() + max.getY()) / 2;
-
-            return new Location(this.context, x, y, z);
+            return new Location(this.context,
+                (min.getX() + max.getX()) / 2.0,
+                (min.getY() + max.getY()) / 2.0,
+                (min.getZ() + max.getZ()) / 2.0);
eternalcombat-plugin/src/main/java/com/eternalcode/combat/CombatPlugin.java (3)

131-132: Consider making the border radius configurable.

The border radius is hardcoded to 6.5. Consider moving this to the configuration.

-        BorderService borderService = new BorderServiceImpl(scheduler, server, regionProvider, eventCaller, () -> 6.5);
+        BorderService borderService = new BorderServiceImpl(scheduler, server, regionProvider, eventCaller, () -> this.pluginConfig.border.radius);

131-131: Consider moving the hardcoded value to configuration.

The value 6.5 should be in the configuration for better flexibility.

-        BorderService borderService = new BorderServiceImpl(scheduler, server, regionProvider, eventCaller, () -> 6.5);
+        BorderService borderService = new BorderServiceImpl(scheduler, server, regionProvider, eventCaller, () -> this.pluginConfig.border.radius);

131-131: Consider making the border value configurable.

The hardcoded value 6.5 should be moved to the configuration for better flexibility.

-        BorderService borderService = new BorderServiceImpl(scheduler, server, regionProvider, eventCaller, () -> 6.5);
+        BorderService borderService = new BorderServiceImpl(scheduler, server, regionProvider, eventCaller, () -> this.pluginConfig.border.size);
eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderServiceImpl.java (2)

99-168: Long method but still understandable.

Collecting border points involves many loops. If this grows more complex, consider breaking it into smaller helper methods for clarity.


176-178: Straightforward distance check.

This simple approach is fine. If performance becomes a concern, you could compare squared distances to avoid calling Math.sqrt.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderPoint.java (1)

11-17: Simplify the toInclusive method.

The method can be more concise by using a single return statement.

-    public BorderPoint toInclusive() {
-        if (inclusive == null) {
-            return this;
-        }
-
-        return inclusive;
-    }
+    public BorderPoint toInclusive() {
+        return inclusive == null ? this : inclusive;
+    }
eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderTrigger.java (1)

14-18: Make the coordinate checks more readable.

The coordinate checks can be grouped by axis for better readability.

-    public boolean isTriggered(int x, int y, int z) {
-        return x >= triggerMin.x() && x <= triggerMax.x()
-            && y >= triggerMin.y() && y <= triggerMax.y()
-            && z >= triggerMin.z() && z <= triggerMax.z();
-    }
+    public boolean isTriggered(int x, int y, int z) {
+        boolean withinX = x >= triggerMin.x() && x <= triggerMax.x();
+        boolean withinY = y >= triggerMin.y() && y <= triggerMax.y();
+        boolean withinZ = z >= triggerMin.z() && z <= triggerMax.z();
+        return withinX && withinY && withinZ;
+    }
eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/event/BorderShowAsyncEvent.java (2)

15-16: Consider making the points field final and using defensive copying.

Since this is an async event, consider making the points field final and using defensive copying in the constructor to prevent concurrent modification issues.

-    private Set<BorderPoint> points;
+    private final Set<BorderPoint> points;

     public BorderShowAsyncEvent(Player player, Set<BorderPoint> points) {
         super(true);
         this.player = player;
-        this.points = points;
+        this.points = new HashSet<>(points);
     }

28-30: Add defensive copying to prevent external modification.

Return a copy of the points set to prevent external code from modifying the internal state.

     public Set<BorderPoint> getPoints() {
-        return points;
+        return new HashSet<>(points);
     }
eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderTriggerIndexBucket.java (2)

52-54: Extract magic numbers to named constants.

The bit manipulation values should be constants with descriptive names.

+    private static final long CHUNK_MASK = 0xFFFFFFFFL;
+    private static final int CHUNK_SHIFT = 32;
+
     private static long packChunk(int bigChunkX, int bigChunkZ) {
-        return (long) bigChunkX & 0xFFFFFFFFL | ((long) bigChunkZ & 0xFFFFFFFFL) << 32;
+        return (long) bigChunkX & CHUNK_MASK | ((long) bigChunkZ & CHUNK_MASK) << CHUNK_SHIFT;
     }

25-30: Simplify using forEach.

The for loop can be replaced with a more concise forEach.

     private BorderTriggerIndexBucket with(Collection<BorderTrigger> triggers) {
-        for (BorderTrigger trigger : triggers) {
-            withTrigger(trigger);
-        }
+        triggers.forEach(this::withTrigger);
         return this;
     }
eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderTriggerController.java (1)

27-40: Optimize the move event handler.

Consider checking combat status before location comparison to avoid unnecessary calculations.

     void onMove(PlayerMoveEvent event) {
+        Player player = event.getPlayer();
+        if (!fightManager.isInCombat(player.getUniqueId())) {
+            return;
+        }
+
         Location to = event.getTo();
         Location from = event.getFrom();
         if (to.getBlockX() == from.getBlockX() && to.getBlockY() == from.getBlockY() && to.getBlockZ() == from.getBlockZ()) {
             return;
         }
 
-        Player player = event.getPlayer();
-        if (!fightManager.isInCombat(player.getUniqueId())) {
-            return;
-        }
-
         borderService.updateBorder(player, to);
     }
eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderTriggerIndex.java (2)

16-16: Consider making the class public.

The class might need to be accessed from other packages. If that's the case, change it from package-private to public.

-class BorderTriggerIndex {
+public class BorderTriggerIndex {

73-76: Consider using a regular loop for better performance.

Streams might be slower here. A simple for loop could be more efficient.

-        return index.getTriggers(x, z)
-            .stream()
-            .filter(trigger -> trigger.isTriggered(x, y, z))
-            .toList();
+        List<BorderTrigger> triggered = new ArrayList<>();
+        for (BorderTrigger trigger : index.getTriggers(x, z)) {
+            if (trigger.isTriggered(x, y, z)) {
+                triggered.add(trigger);
+            }
+        }
+        return triggered;
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a6984e2 and 1a9682a.

📒 Files selected for processing (29)
  • buildSrc/src/main/kotlin/Versions.kt (1 hunks)
  • buildSrc/src/main/kotlin/eternalcombat-java.gradle.kts (0 hunks)
  • buildSrc/src/main/kotlin/eternalcombat-repositories.gradle.kts (1 hunks)
  • eternalcombat-api/src/main/java/com/eternalcode/combat/region/Region.java (1 hunks)
  • eternalcombat-api/src/main/java/com/eternalcode/combat/region/RegionProvider.java (2 hunks)
  • eternalcombat-plugin/build.gradle.kts (2 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/CombatPlugin.java (7 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderActivePointsIndex.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderPoint.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderResult.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderService.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderServiceImpl.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderSettings.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderTrigger.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderTriggerController.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderTriggerIndex.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderTriggerIndexBucket.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/LazyBorderResult.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/event/BorderHideAsyncEvent.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/event/BorderShowAsyncEvent.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/particle/BorderBlockController.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/particle/BorderColorUtil.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/particle/BorderParticleController.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/knockback/KnockbackOutsideRegionGenerator.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/knockback/KnockbackRegionController.java (3 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/knockback/KnockbackService.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/knockback/KnockbackSettings.java (1 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/region/DefaultRegionProvider.java (2 hunks)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/region/WorldGuardRegionProvider.java (3 hunks)
💤 Files with no reviewable changes (1)
  • buildSrc/src/main/kotlin/eternalcombat-java.gradle.kts
👮 Files not reviewed due to content moderation or server errors (2)
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/region/WorldGuardRegionProvider.java
  • eternalcombat-plugin/src/main/java/com/eternalcode/combat/CombatPlugin.java
🔇 Additional comments (48)
eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderSettings.java (3)

1-5: LGTM! Clean and organized imports.


6-6: LGTM! Clear and descriptive interface name.


15-17: LGTM! Simple and effective rounding implementation.

The ceiling rounding works well for border calculations.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/particle/BorderParticleController.java (1)

44-54: Add ignoreCancelled to event handlers.

This suggestion was made in a previous review and is still applicable.

-    @EventHandler
+    @EventHandler(ignoreCancelled = true)
     void onBorderShowAsyncEvent(BorderShowAsyncEvent event) {

-    @EventHandler
+    @EventHandler(ignoreCancelled = true)
     void onBorderHideAsyncEvent(BorderHideAsyncEvent event) {
eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/particle/BorderColorUtil.java (4)

1-11: LGTM! Good utility class setup.

The class is properly set up as a utility class with final modifier.


13-30: Consider adding BLACK dye color mapping.

The color mapping looks good, but it's missing the BLACK dye color. This might be intentional, but worth checking.


32-34: LGTM! Proper utility class constructor.


35-43: Add import for java.awt.Color.

Let's clean up the fully qualified name usage.

+import java.awt.Color;
+
 static Color xyzToColor(int x, int y, int z) {
-    java.awt.Color rgb = xyzToRGB(x, y, z);
+    Color rgb = xyzToRGB(x, y, z);
     return new Color(rgb.getRed(), rgb.getGreen(), rgb.getBlue());
 }
eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/particle/BorderBlockController.java (1)

144-170: Verify chunk snapshot freshness.

Chunk snapshots might be out of date if blocks change, leading to slight inaccuracies. Make sure this caching strategy is safe for your needs.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/region/DefaultRegionProvider.java (3)

3-4: Imports look straightforward
Nice addition of Collection and List. Everything seems tidy.


20-27: Optional usage
Returning Optional.empty() is a clean way to handle no matching region.


46-60: Concise record
Neatly wraps min, max, and center into a simple structure. Looks good.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/knockback/KnockbackOutsideRegionGenerator.java (5)

15-23: Lightweight record
Point2D is clear and works nicely for 2D coordinates.


30-59: Weighted random approach
The logic for distance-based weighting is neat and straightforward.


65-69: Strong exponent
Raising the log difference to the 10th power heavily skews the result. Double-check if that's intended.


71-91: Edge gathering
Clever way to collect boundary points around the region. Well done.


93-95: Distance calculation
Simple, easy-to-read formula for distance.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/knockback/KnockbackRegionController.java (3)

7-8: Region imports
Brings region logic into knockback control. Looks fine.


26-27: New fields
KnockbackService and Server provide flexible knockback handling. Good addition.


96-114: Event handling
The onTag method is a straightforward way to deal with tagged players in a region.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/knockback/KnockbackSettings.java (1)

7-11: Looks good!

The configuration is clear and well-documented.

eternalcombat-api/src/main/java/com/eternalcode/combat/region/RegionProvider.java (1)

16-16: Nice addition!

The new method for getting regions by world is a helpful addition to the interface.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/knockback/KnockbackService.java (1)

17-20: Reorder fields for better organization

eternalcombat-plugin/src/main/java/com/eternalcode/combat/region/WorldGuardRegionProvider.java (5)

10-21: LGTM!

The new imports support the added functionality for region management.


81-104: LGTM!

The WorldGuardRegion record properly implements the Region interface with accurate calculations for center, min, and max points.


60-60: Simplify the method reference.


60-60: Simplify lambda to method reference.

The lambda expression can be simplified to a method reference.


95-104: Well-structured boundary methods!

The getMin and getMax methods are implemented consistently and provide clear access to region boundaries.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/CombatPlugin.java (8)

3-58: LGTM!

The imports are well-organized and support the new border and knockback features.


92-97: LGTM!

PacketEvents is properly initialized in the onLoad phase.


175-182: LGTM!

The new controllers are properly registered with the plugin manager.


204-204: LGTM!

PacketEvents is properly terminated during plugin shutdown.


93-96: LGTM!

The PacketEvents setup follows the correct plugin lifecycle with proper initialization and cleanup.

Also applies to: 204-204


179-181: LGTM!

The border controllers are properly registered with the plugin manager.


93-96: Good PacketEvents lifecycle management!

The PacketEvents setup and cleanup are properly handled in the plugin lifecycle methods.

Also applies to: 204-204


179-181: Clean controller registration!

The border-related controllers are properly integrated into the existing event registration pattern.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderServiceImpl.java (2)

50-65: Smooth scheduling approach.

Running this in an async task helps performance, especially if computing border points is expensive. This looks great.


84-97: Optional usage is tidy.

Returning an empty optional keeps things simple and clear. Nice job here!

eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderResult.java (1)

5-9: Interface design looks good.

This method neatly defines how border points are gathered. Everything seems straightforward here.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderService.java (1)

7-14: Clear and easy to follow.

These method signatures are direct and intuitive, making maintenance simpler.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/LazyBorderResult.java (1)

20-25: Great implementation!

The collect method efficiently handles lazy loading and ensures immutability of the result.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/event/BorderHideAsyncEvent.java (1)

24-30: Format getters consistently.

Previous review suggested a specific formatting for getter methods.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderActivePointsIndex.java (1)

9-65: Well-structured thread-safe implementation!

The implementation is clean, thread-safe, and handles edge cases properly.

eternalcombat-plugin/src/main/java/com/eternalcode/combat/border/BorderTriggerIndex.java (1)

38-55: Watch out for concurrent map updates.

The async update of borderIndexes could cause issues if multiple worlds update at the same time.

Consider using:

-            this.borderIndexes.put(world, index);
+            synchronized(this.borderIndexes) {
+                this.borderIndexes.put(world, index);
+            }
buildSrc/src/main/kotlin/Versions.kt (1)

32-32:

✅ Verification successful

Let's check if we're using the latest PaperLib version


🌐 Web query:

What is the latest version of PaperLib (io.papermc:paperlib)?

💡 Result:

The latest version of PaperLib (io.papermc:paperlib) is 1.0.8, released on December 31, 2022[3]. This update included improvements to version detection for pre-release builds and added support for obtaining InventoryHolder data without snapshots[3].

Key details from recent updates:

  • Version 1.0.7 (Nov 25, 2021) improved detection for Minecraft 1.18 pre-releases[3]
  • Version 1.0.6 (Dec 02, 2020) fixed critical NPE issues and pre-release detection[3]
  • Version 1.0.5 (July 27, 2020) resolved cross-world teleportation issues[3]

Developers should note that while some documentation examples still reference 1.0.7[1], official release history confirms 1.0.8 as current[3]. For implementation:

dependencies {
    implementation "io.papermc:paperlib:1.0.8"
}

Always check official repositories for updates beyond what's shown in documentation samples[1][10].

Citations:


Confirmed: PaperLib version up-to-date

The constant PAPERLIB at line 32 is set to "1.0.8", which is the latest stable version per the official release history. No changes are needed.

buildSrc/src/main/kotlin/eternalcombat-repositories.gradle.kts (1)

8-15: Nice cleanup of repository declarations!

The simplified syntax makes the configuration easier to read, and the new CodeMC repository will help with dependency resolution.

eternalcombat-plugin/build.gradle.kts (2)

57-58:

✅ Verification successful

Let's verify the PacketEvents version


🌐 Web query:

What is the latest version of PacketEvents (com.github.retrooper:packetevents-spigot)?

💡 Result:

The latest version of PacketEvents for Spigot (com.github.retrooper:packetevents-spigot) is v2.7.0, released on December 10, 2024[1][5][6]. This update added support for Minecraft 1.21.4 and included bug fixes related to NPC team names, potion item components, and window click validation[1][4].

Key details:

  • Maven/Gradle dependency available through CodeMC repository[5]
  • Supports Minecraft versions 1.8–1.21.4[1][6]
  • Includes multi-platform compatibility (Bukkit/Folia/Paper/Purpur/Spigot)[2]

The update followed v2.6.0 (November 3, 2024) which introduced support for Minecraft 1.21.2-1.21.3 and added Fabric/Sponge platform support[2][4].

Citations:


PacketEvents version and Minecraft compatibility verified

  • The dependency declaration for com.github.retrooper:packetevents-spigot using version 2.7.0 is correct and up-to-date.
  • This version supports Minecraft 1.21.4 as required.
  • The paperlib dependency appears to be in order.

73-73:

✅ Verification successful

Double-check Minecraft version compatibility


🌐 Web query:

Is Minecraft 1.21.4 released? What is the latest Minecraft version?

💡 Result:

Minecraft 1.21, named The Garden Awakens, was officially released on December 3, 2024, for both Java and Bedrock Editions[1][7][9]. This update introduced major features like the eerie Pale Garden biome, the Creaking mob, and new blocks such as resin bricks[1][6][7].

Latest Versions as of February 16, 2025:

  • Java Edition: Version 1.21.4 remains the most recent stable release for Java Edition[9].
  • Bedrock Edition: Version 1.21.60 was released in February 2025 with experimental features like Warm/Cold Pig variants, wildflowers, and falling leaf particles[8].

Key Features of Minecraft Updates:

The Garden Awakens (Java/Bedrock v1.21.4):

  • Pale Garden Biome: A desaturated biome with Pale Oak trees and ambient silence[1][6].
  • Creaking Mob: A hostile entity that only moves when unobserved and drops resin clumps[6][7].
  • Resin Blocks: Crafted from smelted resin clumps found by defeating Creakings[7].

Recent Bedrock v1.21.60 (February 2025):

  • New Pig Variants: Warm Pigs (desert/mesa) and Cold Pigs (snowy biomes)[8].
  • Falling Leaves: All leaf blocks now emit falling particles[8].
  • Lodestone Tweaks: Adjusted functionality for better usability[8].

For future updates, Mojang plans smaller quarterly drops instead of annual major updates[2]. The next drop is speculated for March/April 2025[2][8].

Citations:


Verification Complete: Minecraft version 1.21.4 confirmed valid.

After checking the latest Minecraft version details, version 1.21.4 is indeed the most recent stable release for Java Edition. The usage in eternalcombat-plugin/build.gradle.kts is correct. No further changes are required.

@Rollczi
Copy link
Member Author

Rollczi commented Feb 18, 2025

co do code style to połowę zignorowałem trochę męczące - Confingi zrobię później bo czasu zabrakło

@vLuckyyy vLuckyyy removed the request for review from Kamicjusz March 2, 2025 13:58
return calculateRemovedPoints(points, oldPoints);
}

return Set.of();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't Collections#emptySet better?

Comment on lines +23 to +24
public static final PacketEventsAPI<?> PACKET_EVENTS_API = PacketEvents.getAPI();
public static final PlayerManager PLAYER_MANAGER = PACKET_EVENTS_API.getPlayerManager();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do these need to be public? same in BorderBlockController

Copy link
Member

@CitralFlo CitralFlo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adjusted some minor config things in commit. From user perspective configuration looks good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants