Do not use Exceptions for code control flow for BlockBreakListener

RakambdaOrg · Oct 21, 2023 · 7188a20 · 7188a20
1 parent 89aeeda
commit 7188a20
Show file tree

Hide file tree

Showing 7 changed files with 97 additions and 39 deletions.
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
@@ -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
@@ -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
@@ -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
@@ -34,27 +34,27 @@ public class TreeHandler{
 	private final Map<UUID, CacheSpeed> 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/...on/src/main/java/fr/rakambda/fallingtree/common/tree/exception/TreeBreakingException.java b/...on/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
@@ -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
@@ -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);
 			}
 		}