From 7188a200e7d70561ae16a89682b068130361d4bc Mon Sep 17 00:00:00 2001 From: Geolykt Date: Sun, 22 Oct 2023 00:34:09 +0200 Subject: [PATCH] Do not use Exceptions for code control flow for BlockBreakListener --- .../common/tree/BreakAbortionCause.java | 22 ++++++++++ .../common/tree/BreakTreeResult.java | 13 +++++- .../common/tree/IBreakAttemptResult.java | 14 +++++++ .../fallingtree/common/tree/TreeHandler.java | 40 ++++++++++++++----- .../tree/exception/TreeBreakingException.java | 4 ++ .../fabric/event/BlockBreakListener.java | 21 +++------- .../forge/event/BlockBreakListener.java | 22 ++++------ 7 files changed, 97 insertions(+), 39 deletions(-) create mode 100644 common/src/main/java/fr/rakambda/fallingtree/common/tree/BreakAbortionCause.java create mode 100644 common/src/main/java/fr/rakambda/fallingtree/common/tree/IBreakAttemptResult.java diff --git a/common/src/main/java/fr/rakambda/fallingtree/common/tree/BreakAbortionCause.java b/common/src/main/java/fr/rakambda/fallingtree/common/tree/BreakAbortionCause.java new file mode 100644 index 00000000..d476fb1a --- /dev/null +++ b/common/src/main/java/fr/rakambda/fallingtree/common/tree/BreakAbortionCause.java @@ -0,0 +1,22 @@ +package fr.rakambda.fallingtree.common.tree; + +public enum BreakAbortionCause implements IBreakAttemptResult { + NOT_SERVER(false), + NOT_ENABLED(false), + REQUIRED_TOOL_ABSENT(true), + INVALID_PLAYER_STATE(false), + NO_SUCH_TREE(false), + TREE_TOO_BIG_SCAN(false), + TREE_TOO_BIG_BREAK(false); + + private final boolean cancel; + + BreakAbortionCause(boolean cancel) { + this.cancel = cancel; + } + + @Override + public boolean shouldCancel(){ + return this.cancel; + } +} diff --git a/common/src/main/java/fr/rakambda/fallingtree/common/tree/BreakTreeResult.java b/common/src/main/java/fr/rakambda/fallingtree/common/tree/BreakTreeResult.java index de0245f0..d10f5176 100644 --- a/common/src/main/java/fr/rakambda/fallingtree/common/tree/BreakTreeResult.java +++ b/common/src/main/java/fr/rakambda/fallingtree/common/tree/BreakTreeResult.java @@ -3,8 +3,19 @@ import fr.rakambda.fallingtree.common.config.enums.BreakMode; import org.jetbrains.annotations.NotNull; +/** + * Record that denotes that an attempt to break a tree with the given mode has + * succeeded. Failures are instead denoted as {@link BreakAbortionCause}. + * + * @param shouldCancel Whether the event which triggered the query should be cancelled as a result of this result. + * @param breakMode The mode with which the block was broken. + */ public record BreakTreeResult( boolean shouldCancel, @NotNull BreakMode breakMode -){ +) implements IBreakAttemptResult{ + @Override + public boolean shouldCancel(){ + return this.shouldCancel; + } } diff --git a/common/src/main/java/fr/rakambda/fallingtree/common/tree/IBreakAttemptResult.java b/common/src/main/java/fr/rakambda/fallingtree/common/tree/IBreakAttemptResult.java new file mode 100644 index 00000000..dcd7a0e7 --- /dev/null +++ b/common/src/main/java/fr/rakambda/fallingtree/common/tree/IBreakAttemptResult.java @@ -0,0 +1,14 @@ +package fr.rakambda.fallingtree.common.tree; + +import fr.rakambda.fallingtree.common.wrapper.IBlockPos; +import fr.rakambda.fallingtree.common.wrapper.ILevel; +import fr.rakambda.fallingtree.common.wrapper.IPlayer; + +/** + * The result of a {@link TreeHandler#attemptTreeBreaking(ILevel, IPlayer, IBlockPos)}, whether it succeeded or not. + * Failures are generally instances of {@link BreakAbortionCause}, where are succeeded attempts are instances of + * {@link BreakTreeResult}. + */ +public interface IBreakAttemptResult{ + boolean shouldCancel(); +} diff --git a/common/src/main/java/fr/rakambda/fallingtree/common/tree/TreeHandler.java b/common/src/main/java/fr/rakambda/fallingtree/common/tree/TreeHandler.java index b7c65657..715f3d6d 100644 --- a/common/src/main/java/fr/rakambda/fallingtree/common/tree/TreeHandler.java +++ b/common/src/main/java/fr/rakambda/fallingtree/common/tree/TreeHandler.java @@ -34,27 +34,27 @@ public class TreeHandler{ private final Map speedCache = new ConcurrentHashMap<>(); @NotNull - public BreakTreeResult breakTree(@NotNull ILevel level, @NotNull IPlayer player, @NotNull IBlockPos blockPos) throws TreeBreakingNotEnabledException, PlayerNotInRightState, ToolUseForcedException, TreeBreakingException, NoTreeFoundException, NotServerException{ + public IBreakAttemptResult attemptTreeBreaking(@NotNull ILevel level, @NotNull IPlayer player, @NotNull IBlockPos blockPos) { if(!level.isServer()){ - throw new NotServerException(); + return BreakAbortionCause.NOT_SERVER; } if(!mod.getConfiguration().getTrees().isTreeBreaking()){ - throw new TreeBreakingNotEnabledException(); + return BreakAbortionCause.NOT_ENABLED; } - + if(!mod.checkForceToolUsage(player, level, blockPos)){ mod.notifyPlayer(player, mod.translate("chat.fallingtree.force_tool_usage", mod.getConfiguration().getTrees().getMaxScanSize())); - throw new ToolUseForcedException(); + return BreakAbortionCause.REQUIRED_TOOL_ABSENT; } if(!mod.isPlayerInRightState(player)){ - throw new PlayerNotInRightState(); + return BreakAbortionCause.INVALID_PLAYER_STATE; } try{ var treeOptional = mod.getTreeBuilder().getTree(player, level, blockPos); if(treeOptional.isEmpty()){ - throw new NoTreeFoundException(); + return BreakAbortionCause.NO_SUCH_TREE; } var tree = treeOptional.get(); @@ -64,13 +64,35 @@ public BreakTreeResult breakTree(@NotNull ILevel level, @NotNull IPlayer player, } catch(TreeTooBigException e){ mod.notifyPlayer(player, mod.translate("chat.fallingtree.tree_too_big", mod.getConfiguration().getTrees().getMaxScanSize())); - throw new TreeBreakingException(e); + return BreakAbortionCause.TREE_TOO_BIG_SCAN; } catch(BreakTreeTooBigException e){ mod.notifyPlayer(player, mod.translate("chat.fallingtree.break_tree_too_big", mod.getConfiguration().getTrees().getMaxSize())); - throw new TreeBreakingException(e); + return BreakAbortionCause.TREE_TOO_BIG_BREAK; } } + + @Deprecated + @NotNull + public BreakTreeResult breakTree(@NotNull ILevel level, @NotNull IPlayer player, @NotNull IBlockPos blockPos) throws TreeBreakingNotEnabledException, PlayerNotInRightState, ToolUseForcedException, TreeBreakingException, NoTreeFoundException, NotServerException{ + IBreakAttemptResult result = this.attemptTreeBreaking(level, player, blockPos); + if (result instanceof BreakTreeResult ret) { + return ret; + } + if (result instanceof BreakAbortionCause cause) { + switch (cause) { + case NOT_SERVER -> throw new NotServerException(); + case TREE_TOO_BIG_SCAN -> throw new TreeBreakingException("Tree exceeded scanning limits"); + case NO_SUCH_TREE -> throw new NoTreeFoundException(); + case INVALID_PLAYER_STATE -> throw new PlayerNotInRightState(); + case NOT_ENABLED -> throw new TreeBreakingNotEnabledException(); + case TREE_TOO_BIG_BREAK -> throw new TreeBreakingException("Tree exceeded breaking limits"); + case REQUIRED_TOOL_ABSENT -> throw new ToolUseForcedException(); + } + } + + throw new TreeBreakingException("Unknown/Unsupported break result: " + result); + } @NotNull private BreakMode getBreakMode(@NotNull IItemStack itemStack){ diff --git a/common/src/main/java/fr/rakambda/fallingtree/common/tree/exception/TreeBreakingException.java b/common/src/main/java/fr/rakambda/fallingtree/common/tree/exception/TreeBreakingException.java index b85b78b9..61ad41a0 100644 --- a/common/src/main/java/fr/rakambda/fallingtree/common/tree/exception/TreeBreakingException.java +++ b/common/src/main/java/fr/rakambda/fallingtree/common/tree/exception/TreeBreakingException.java @@ -1,7 +1,11 @@ package fr.rakambda.fallingtree.common.tree.exception; public class TreeBreakingException extends Exception{ + @Deprecated public TreeBreakingException(Throwable throwable){ super(throwable); } + public TreeBreakingException(String message){ + super(message); + } } diff --git a/fabric/src/main/java/fr/rakambda/fallingtree/fabric/event/BlockBreakListener.java b/fabric/src/main/java/fr/rakambda/fallingtree/fabric/event/BlockBreakListener.java index 3a8c6a86..dd9c5f0a 100644 --- a/fabric/src/main/java/fr/rakambda/fallingtree/fabric/event/BlockBreakListener.java +++ b/fabric/src/main/java/fr/rakambda/fallingtree/fabric/event/BlockBreakListener.java @@ -1,12 +1,7 @@ package fr.rakambda.fallingtree.fabric.event; import fr.rakambda.fallingtree.common.FallingTreeCommon; -import fr.rakambda.fallingtree.common.tree.exception.NoTreeFoundException; -import fr.rakambda.fallingtree.common.tree.exception.NotServerException; -import fr.rakambda.fallingtree.common.tree.exception.PlayerNotInRightState; -import fr.rakambda.fallingtree.common.tree.exception.ToolUseForcedException; -import fr.rakambda.fallingtree.common.tree.exception.TreeBreakingException; -import fr.rakambda.fallingtree.common.tree.exception.TreeBreakingNotEnabledException; +import fr.rakambda.fallingtree.common.tree.BreakTreeResult; import fr.rakambda.fallingtree.fabric.common.wrapper.BlockPosWrapper; import fr.rakambda.fallingtree.fabric.common.wrapper.LevelWrapper; import fr.rakambda.fallingtree.fabric.common.wrapper.PlayerWrapper; @@ -32,18 +27,14 @@ public boolean beforeBlockBreak(Level level, Player player, BlockPos blockPos, B var wrappedLevel = level instanceof ServerLevel serverLevel ? new ServerLevelWrapper(serverLevel) : new LevelWrapper(level); var wrappedPos = new BlockPosWrapper(blockPos); - try{ - var result = mod.getTreeHandler().breakTree(wrappedLevel, wrappedPlayer, wrappedPos); - return switch(result.breakMode()){ + var result = mod.getTreeHandler().attemptTreeBreaking(wrappedLevel, wrappedPlayer, wrappedPos); + if (result instanceof BreakTreeResult breakTreeResult) { + return switch(breakTreeResult.breakMode()){ case INSTANTANEOUS, FALL_ITEM, FALL_BLOCK, FALL_ALL_BLOCK -> !result.shouldCancel(); case SHIFT_DOWN -> false; }; - } - catch(TreeBreakingNotEnabledException | PlayerNotInRightState | TreeBreakingException | NoTreeFoundException | NotServerException e){ - return true; - } - catch(ToolUseForcedException e){ - return false; + } else { + return !result.shouldCancel(); } } } diff --git a/forge/src/main/java/fr/rakambda/fallingtree/forge/event/BlockBreakListener.java b/forge/src/main/java/fr/rakambda/fallingtree/forge/event/BlockBreakListener.java index c405eb34..54b86411 100644 --- a/forge/src/main/java/fr/rakambda/fallingtree/forge/event/BlockBreakListener.java +++ b/forge/src/main/java/fr/rakambda/fallingtree/forge/event/BlockBreakListener.java @@ -1,7 +1,7 @@ package fr.rakambda.fallingtree.forge.event; import fr.rakambda.fallingtree.common.FallingTreeCommon; -import fr.rakambda.fallingtree.common.tree.exception.*; +import fr.rakambda.fallingtree.common.tree.BreakTreeResult; import fr.rakambda.fallingtree.forge.common.wrapper.BlockPosWrapper; import fr.rakambda.fallingtree.forge.common.wrapper.LevelWrapper; import fr.rakambda.fallingtree.forge.common.wrapper.PlayerWrapper; @@ -12,7 +12,6 @@ import net.minecraftforge.event.level.BlockEvent; import net.minecraftforge.eventbus.api.SubscribeEvent; import org.jetbrains.annotations.NotNull; - import javax.annotation.Nonnull; @RequiredArgsConstructor @@ -55,19 +54,14 @@ public void onBlockBreakEvent(@Nonnull BlockEvent.BreakEvent event){ var wrappedLevel = event.getLevel() instanceof ServerLevel serverLevel ? new ServerLevelWrapper(serverLevel) : new LevelWrapper(event.getLevel()); var wrappedPos = new BlockPosWrapper(event.getPos()); - try{ - var result = mod.getTreeHandler().breakTree(wrappedLevel, wrappedPlayer, wrappedPos); - if(event.isCancelable()){ - switch(result.breakMode()){ - case INSTANTANEOUS, FALL_ITEM, FALL_BLOCK, FALL_ALL_BLOCK -> event.setCanceled(result.shouldCancel()); - case SHIFT_DOWN -> event.setCanceled(true); - } + var result = mod.getTreeHandler().attemptTreeBreaking(wrappedLevel, wrappedPlayer, wrappedPos); + if (result instanceof BreakTreeResult breakTreeResult) { + switch(breakTreeResult.breakMode()){ + case INSTANTANEOUS, FALL_ITEM, FALL_BLOCK, FALL_ALL_BLOCK -> event.setCanceled(result.shouldCancel()); + case SHIFT_DOWN -> event.setCanceled(true); } - } - catch(TreeBreakingNotEnabledException | PlayerNotInRightState | TreeBreakingException | NoTreeFoundException | NotServerException ignored){ - } - catch(ToolUseForcedException e){ - if(event.isCancelable()){ + } else { + if(result.shouldCancel() && event.isCancelable()){ event.setCanceled(true); } }