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

Ref: height limit check #4427

Merged

Conversation

RedstoneFuture
Copy link
Member

@RedstoneFuture RedstoneFuture commented May 15, 2024

Overview

  • The notifyIfOutsideBuildArea method checks the limits and already includes sending the height.height_limit message. This does not need to be called again in the code.
  • Small reformatting

Description

In my test, the message was sent correctly even after the refactoring (min_height: 1; max_height: 256).

grafik

Submitter Checklist

.tag("minheight", Tag.inserting(Component.text(area.getMinBuildHeight())))
.tag("maxheight", Tag.inserting(Component.text(area.getMaxBuildHeight())))
.build()
);
return;
Copy link
Member Author

Choose a reason for hiding this comment

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

Duplicated message sending: already included in notifyIfOutsideBuildArea()

.tag("minheight", Tag.inserting(Component.text(area.getMinBuildHeight())))
.tag("maxheight", Tag.inserting(Component.text(area.getMaxBuildHeight())))
.build()
);
return;
Copy link
Member Author

Choose a reason for hiding this comment

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

Duplicated message sending: already included in notifyIfOutsideBuildArea()

}
if (area.notifyIfOutsideBuildArea(pp, currentLocation.getY())) {
event.setCancelled(true);
break;
}
Copy link
Member Author

@RedstoneFuture RedstoneFuture May 15, 2024

Choose a reason for hiding this comment

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

The check in line 1227 looks exactly like the check in notifyIfOutsideBuildArea().

Line 1227:

currentLocation.getY() >= area.getMaxBuildHeight() || currentLocation.getY() < area.getMinBuildHeight()

/Core/src/main/java/com/plotsquared/core/plot/PlotArea.java#L676-L677:

    public boolean notifyIfOutsideBuildArea(PlotPlayer<?> player, int y) {
        if (!buildRangeContainsY(y) && !player.hasPermission(Permission.PERMISSION_ADMIN_BUILD_HEIGHT_LIMIT)) {
            ...
            // Return true if "failed" as the method will always be inverted otherwise
            return true;
        }
        return false;
    }

@RedstoneFuture RedstoneFuture marked this pull request as ready for review May 15, 2024 19:31
@RedstoneFuture RedstoneFuture requested a review from a team as a code owner May 15, 2024 19:31
@NotMyFault
Copy link
Member

  • What went wrong:
    Execution failed for task ':plotsquared-bukkit:spotlessJavaCheck'.
    The following files had format violations:
    src/main/java/com/plotsquared/bukkit/listener/BlockEventListener.java
    @@ -60,7 +60,6 @@
    import·com.sk89q.worldedit.WorldEdit;
    import·com.sk89q.worldedit.bukkit.BukkitAdapter;
    import·com.sk89q.worldedit.world.block.BlockType;
    -import·net.kyori.adventure.text.Component;
    import·net.kyori.adventure.text.minimessage.tag.Tag;
    import·net.kyori.adventure.text.minimessage.tag.resolver.TagResolver;
    import·org.bukkit.Bukkit;
    Run './gradlew :plotsquared-bukkit:spotlessApply' to fix these violations.

@dordsor21 dordsor21 requested a review from a team May 24, 2024 13:15
@dordsor21 dordsor21 merged commit 215053e into IntellectualSites:main Jun 1, 2024
9 checks passed
@RedstoneFuture RedstoneFuture deleted the refactory/height-limit-check branch June 1, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants