Rethink PlatformHelper.useOn

useOn is now only responsible for firing the actual mod loader events,
and just returns the result of firing that event. The actual calling of
Block.use/Item.useOn now live in TurtlePlaceCommand.

This isn't especially useful for 1.20.1, but is more relevant on 1.21.1
when we look at #2011, as the shared code is much larger.

projects/common/src/main/java/dan200/computercraft/shared/platform/PlatformHelper.java

@@ -39,6 +39,7 @@ import net.minecraft.world.item.CreativeModeTab;
 import net.minecraft.world.item.DyeColor;
 import net.minecraft.world.item.Item;
 import net.minecraft.world.item.ItemStack;
+import net.minecraft.world.item.context.UseOnContext;
 import net.minecraft.world.item.crafting.Recipe;
 import net.minecraft.world.level.Level;
 import net.minecraft.world.level.block.Block;
@@ -53,7 +54,6 @@ import java.util.List;
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
 import java.util.function.Function;
-import java.util.function.Predicate;
 
 /**
  * This extends {@linkplain dan200.computercraft.impl.PlatformHelper the API's loader abstraction layer}, adding
@@ -375,20 +375,40 @@ public interface PlatformHelper extends dan200.computercraft.impl.PlatformHelper
     boolean interactWithEntity(ServerPlayer player, Entity entity, Vec3 hitPos);
 
     /**
-     * Place an item against a block.
+     * The result of attempting to use an item on a block.
+     */
+    sealed interface UseOnResult {
+        /**
+         * This interaction was intercepted by an event, and handled.
+         *
+         * @param result The result of using an item on a block.
+         */
+        record Handled(InteractionResult result) implements UseOnResult {
+        }
+
+        /**
+         * This result was not handled, and should be handled by the caller.
+         *
+         * @param block Whether the block may be used ({@link BlockState#use(Level, Player, InteractionHand, BlockHitResult)}).
+         * @param item  Whether the item may be used on the block ({@link ItemStack#useOn(UseOnContext)}).
+         * @see ServerPlayerGameMode#useItemOn(ServerPlayer, Level, ItemStack, InteractionHand, BlockHitResult)
+         */
+        record Continue(boolean block, boolean item) implements UseOnResult {
+        }
+    }
+
+    /**
+     * Run mod-loader specific code before placing an item against a block.
      * <p>
-     * Implementations should largely mirror {@link ServerPlayerGameMode#useItemOn(ServerPlayer, Level, ItemStack, InteractionHand, BlockHitResult)}
+     * This should dispatch any mod-loader specific events that are fired when clicking a block. It does necessarily
-     * (including any loader-specific modifications), except the call to {@link BlockState#use(Level, Player, InteractionHand, BlockHitResult)}
+     * handle the actual clicking of the block — see {@link UseOnResult.Handled} and {@link UseOnResult.Continue}.
-     * should only be evaluated when {@code canUseBlock} evaluates to true.
      *
      * @param player The player which is placing this item.
      * @param stack  The item to place.
      * @param hit    The collision with the block we're placing against.
-     * @param canUseBlock Test whether the block should be interacted with first.
      * @return Whether any interaction occurred.
-     * @see ServerPlayerGameMode#useItemOn(ServerPlayer, Level, ItemStack, InteractionHand, BlockHitResult)
      */
-    InteractionResult useOn(ServerPlayer player, ItemStack stack, BlockHitResult hit, Predicate<BlockState> canUseBlock);
+    UseOnResult useOn(ServerPlayer player, ItemStack stack, BlockHitResult hit);
 
     /**
      * Whether {@link net.minecraft.network.chat.ClickEvent.Action#RUN_COMMAND} can be used to run client commands.

projects/common/src/main/java/dan200/computercraft/shared/pocket/core/PocketHolder.java

@@ -16,7 +16,7 @@ import net.minecraft.world.phys.Vec3;
 /**
  * An object that holds a pocket computer item.
  */
-public sealed interface PocketHolder permits PocketHolder.EntityHolder {
+public sealed interface PocketHolder {
     /**
      * The level this holder is in.
      *
@@ -54,7 +54,7 @@ public sealed interface PocketHolder permits PocketHolder.EntityHolder {
     /**
      * An {@link Entity} holding a pocket computer.
      */
-    sealed interface EntityHolder extends PocketHolder permits PocketHolder.PlayerHolder, PocketHolder.ItemEntityHolder {
+    sealed interface EntityHolder extends PocketHolder {
         /**
          * Get the entity holding this pocket computer.
          *

projects/common/src/main/java/dan200/computercraft/shared/turtle/core/TurtlePlaceCommand.java

@@ -202,11 +202,22 @@ public class TurtlePlaceCommand implements TurtleCommand {
      * @return If this item was deployed.
      */
     private static InteractionResult doDeployOnBlock(ItemStack stack, TurtlePlayer turtlePlayer, BlockHitResult hit, boolean adjacent) {
-        var result = PlatformHelper.get().useOn(
+        var result = PlatformHelper.get().useOn(turtlePlayer.player(), stack, hit);
-            turtlePlayer.player(), stack, hit,
+        if (result instanceof PlatformHelper.UseOnResult.Handled handled) {
-            adjacent ? x -> x.is(ComputerCraftTags.Blocks.TURTLE_CAN_USE) : x -> false
+            if (handled.result() != InteractionResult.PASS) return handled.result();
-        );
+        } else {
-        if (result != InteractionResult.PASS) return result;
+            var canUse = (PlatformHelper.UseOnResult.Continue) result;
+
+            var player = turtlePlayer.player();
+            var block = player.level().getBlockState(hit.getBlockPos());
+            if (adjacent && canUse.block() && block.is(ComputerCraftTags.Blocks.TURTLE_CAN_USE)) {
+                var useResult = block.use(player.level(), player, InteractionHand.MAIN_HAND, hit);
+                if (useResult.consumesAction()) return useResult;
+            }
+
+            var useOnResult = stack.useOn(new UseOnContext(player, InteractionHand.MAIN_HAND, hit));
+            if (useOnResult != InteractionResult.PASS) return useOnResult;
+        }
 
         var level = turtlePlayer.player().level();
 

projects/common/src/main/java/dan200/computercraft/shared/turtle/upgrades/TurtleTool.java

@@ -29,6 +29,7 @@ import net.minecraft.world.entity.decoration.ArmorStand;
 import net.minecraft.world.entity.player.Player;
 import net.minecraft.world.item.Item;
 import net.minecraft.world.item.ItemStack;
+import net.minecraft.world.item.context.UseOnContext;
 import net.minecraft.world.item.enchantment.EnchantmentHelper;
 import net.minecraft.world.level.BlockGetter;
 import net.minecraft.world.level.Level;
@@ -343,8 +344,12 @@ public class TurtleTool extends AbstractTurtleUpgrade {
         }
 
         var hit = TurtlePlaceCommand.getHitResult(position, direction.getOpposite());
-        var result = PlatformHelper.get().useOn(turtlePlayer.player(), stack, hit, x -> false);
+        var result = PlatformHelper.get().useOn(turtlePlayer.player(), stack, hit);
-        return result.consumesAction();
+        if (result instanceof PlatformHelper.UseOnResult.Handled handled) {
+            return handled.result().consumesAction();
+        } else {
+            return ((PlatformHelper.UseOnResult.Continue) result).item() && item.useOn(new UseOnContext(turtlePlayer.player(), InteractionHand.MAIN_HAND, hit)).consumesAction();
+        }
     }
 
     private static boolean isTriviallyBreakable(BlockGetter reader, BlockPos pos, BlockState state) {

projects/common/src/test/java/dan200/computercraft/TestPlatformHelper.java

@@ -59,7 +59,6 @@ import java.util.List;
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
 import java.util.function.Function;
-import java.util.function.Predicate;
 
 @AutoService({ PlatformHelper.class, dan200.computercraft.impl.PlatformHelper.class, ComputerCraftAPIService.class })
 public class TestPlatformHelper extends AbstractComputerCraftAPI implements PlatformHelper {
@@ -223,7 +222,7 @@ public class TestPlatformHelper extends AbstractComputerCraftAPI implements Plat
     }
 
     @Override
-    public InteractionResult useOn(ServerPlayer player, ItemStack stack, BlockHitResult hit, Predicate<BlockState> canUseBlock) {
+    public UseOnResult useOn(ServerPlayer player, ItemStack stack, BlockHitResult hit) {
         throw new UnsupportedOperationException("Cannot interact with the world inside tests");
     }
 

projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Turtle_Test.kt

@@ -142,6 +142,22 @@ class Turtle_Test {
         }
     }
 
+    /**
+     * Checks that turtles cannot place items into non-adjacent blocks.
+     *
+     * See [ComputerCraftTags.Blocks.TURTLE_CAN_USE].
+     */
+    @GameTest
+    fun Place_into_composter_non_adjacent(helper: GameTestHelper) = helper.sequence {
+        thenOnComputer {
+            turtle.place(ObjectArguments()).await()
+                .assertArrayEquals(false, "Cannot place item here", message = "Failed to place item")
+        }
+        thenExecute {
+            helper.assertBlockIs(BlockPos(2, 2, 3)) { it.block == Blocks.COMPOSTER && it.getValue(ComposterBlock.LEVEL) == 0 }
+        }
+    }
+
     /**
      * Checks that turtles can place bottles into beehives.
      *

projects/common/src/testMod/resources/data/cctest/structures/turtle_test.place_into_composter_non_adjacent.snbt

@@ -0,0 +1,138 @@
+{
+    DataVersion: 3465,
+    size: [5, 5, 5],
+    data: [
+        {pos: [0, 0, 0], state: "minecraft:polished_andesite"},
+        {pos: [0, 0, 1], state: "minecraft:polished_andesite"},
+        {pos: [0, 0, 2], state: "minecraft:polished_andesite"},
+        {pos: [0, 0, 3], state: "minecraft:polished_andesite"},
+        {pos: [0, 0, 4], state: "minecraft:polished_andesite"},
+        {pos: [1, 0, 0], state: "minecraft:polished_andesite"},
+        {pos: [1, 0, 1], state: "minecraft:polished_andesite"},
+        {pos: [1, 0, 2], state: "minecraft:polished_andesite"},
+        {pos: [1, 0, 3], state: "minecraft:polished_andesite"},
+        {pos: [1, 0, 4], state: "minecraft:polished_andesite"},
+        {pos: [2, 0, 0], state: "minecraft:polished_andesite"},
+        {pos: [2, 0, 1], state: "minecraft:polished_andesite"},
+        {pos: [2, 0, 2], state: "minecraft:polished_andesite"},
+        {pos: [2, 0, 3], state: "minecraft:polished_andesite"},
+        {pos: [2, 0, 4], state: "minecraft:polished_andesite"},
+        {pos: [3, 0, 0], state: "minecraft:polished_andesite"},
+        {pos: [3, 0, 1], state: "minecraft:polished_andesite"},
+        {pos: [3, 0, 2], state: "minecraft:polished_andesite"},
+        {pos: [3, 0, 3], state: "minecraft:polished_andesite"},
+        {pos: [3, 0, 4], state: "minecraft:polished_andesite"},
+        {pos: [4, 0, 0], state: "minecraft:polished_andesite"},
+        {pos: [4, 0, 1], state: "minecraft:polished_andesite"},
+        {pos: [4, 0, 2], state: "minecraft:polished_andesite"},
+        {pos: [4, 0, 3], state: "minecraft:polished_andesite"},
+        {pos: [4, 0, 4], state: "minecraft:polished_andesite"},
+        {pos: [0, 1, 0], state: "minecraft:air"},
+        {pos: [0, 1, 1], state: "minecraft:air"},
+        {pos: [0, 1, 2], state: "minecraft:air"},
+        {pos: [0, 1, 3], state: "minecraft:air"},
+        {pos: [0, 1, 4], state: "minecraft:air"},
+        {pos: [1, 1, 0], state: "minecraft:air"},
+        {pos: [1, 1, 1], state: "minecraft:air"},
+        {pos: [1, 1, 2], state: "minecraft:air"},
+        {pos: [1, 1, 3], state: "minecraft:air"},
+        {pos: [1, 1, 4], state: "minecraft:air"},
+        {pos: [2, 1, 0], state: "minecraft:air"},
+        {pos: [2, 1, 1], state: "computercraft:turtle_normal{facing:south,waterlogged:false}", nbt: {ComputerId: 1, Fuel: 0, Items: [{Count: 1b, Slot: 0b, id: "minecraft:pumpkin_pie"}], Label: "turtle_test.place_into_composter_non_adjacent", On: 1b, Owner: {LowerId: -7298459922670553123L, Name: "Player572", UpperId: -8225029765375707172L}, Slot: 0, id: "computercraft:turtle_normal"}},
+        {pos: [2, 1, 2], state: "minecraft:air"},
+        {pos: [2, 1, 3], state: "minecraft:composter{level:0}"},
+        {pos: [2, 1, 4], state: "minecraft:air"},
+        {pos: [3, 1, 0], state: "minecraft:air"},
+        {pos: [3, 1, 1], state: "minecraft:air"},
+        {pos: [3, 1, 2], state: "minecraft:air"},
+        {pos: [3, 1, 3], state: "minecraft:air"},
+        {pos: [3, 1, 4], state: "minecraft:air"},
+        {pos: [4, 1, 0], state: "minecraft:air"},
+        {pos: [4, 1, 1], state: "minecraft:air"},
+        {pos: [4, 1, 2], state: "minecraft:air"},
+        {pos: [4, 1, 3], state: "minecraft:air"},
+        {pos: [4, 1, 4], state: "minecraft:air"},
+        {pos: [0, 2, 0], state: "minecraft:air"},
+        {pos: [0, 2, 1], state: "minecraft:air"},
+        {pos: [0, 2, 2], state: "minecraft:air"},
+        {pos: [0, 2, 3], state: "minecraft:air"},
+        {pos: [0, 2, 4], state: "minecraft:air"},
+        {pos: [1, 2, 0], state: "minecraft:air"},
+        {pos: [1, 2, 1], state: "minecraft:air"},
+        {pos: [1, 2, 2], state: "minecraft:air"},
+        {pos: [1, 2, 3], state: "minecraft:air"},
+        {pos: [1, 2, 4], state: "minecraft:air"},
+        {pos: [2, 2, 0], state: "minecraft:air"},
+        {pos: [2, 2, 1], state: "minecraft:air"},
+        {pos: [2, 2, 2], state: "minecraft:air"},
+        {pos: [2, 2, 3], state: "minecraft:air"},
+        {pos: [2, 2, 4], state: "minecraft:air"},
+        {pos: [3, 2, 0], state: "minecraft:air"},
+        {pos: [3, 2, 1], state: "minecraft:air"},
+        {pos: [3, 2, 2], state: "minecraft:air"},
+        {pos: [3, 2, 3], state: "minecraft:air"},
+        {pos: [3, 2, 4], state: "minecraft:air"},
+        {pos: [4, 2, 0], state: "minecraft:air"},
+        {pos: [4, 2, 1], state: "minecraft:air"},
+        {pos: [4, 2, 2], state: "minecraft:air"},
+        {pos: [4, 2, 3], state: "minecraft:air"},
+        {pos: [4, 2, 4], state: "minecraft:air"},
+        {pos: [0, 3, 0], state: "minecraft:air"},
+        {pos: [0, 3, 1], state: "minecraft:air"},
+        {pos: [0, 3, 2], state: "minecraft:air"},
+        {pos: [0, 3, 3], state: "minecraft:air"},
+        {pos: [0, 3, 4], state: "minecraft:air"},
+        {pos: [1, 3, 0], state: "minecraft:air"},
+        {pos: [1, 3, 1], state: "minecraft:air"},
+        {pos: [1, 3, 2], state: "minecraft:air"},
+        {pos: [1, 3, 3], state: "minecraft:air"},
+        {pos: [1, 3, 4], state: "minecraft:air"},
+        {pos: [2, 3, 0], state: "minecraft:air"},
+        {pos: [2, 3, 1], state: "minecraft:air"},
+        {pos: [2, 3, 2], state: "minecraft:air"},
+        {pos: [2, 3, 3], state: "minecraft:air"},
+        {pos: [2, 3, 4], state: "minecraft:air"},
+        {pos: [3, 3, 0], state: "minecraft:air"},
+        {pos: [3, 3, 1], state: "minecraft:air"},
+        {pos: [3, 3, 2], state: "minecraft:air"},
+        {pos: [3, 3, 3], state: "minecraft:air"},
+        {pos: [3, 3, 4], state: "minecraft:air"},
+        {pos: [4, 3, 0], state: "minecraft:air"},
+        {pos: [4, 3, 1], state: "minecraft:air"},
+        {pos: [4, 3, 2], state: "minecraft:air"},
+        {pos: [4, 3, 3], state: "minecraft:air"},
+        {pos: [4, 3, 4], state: "minecraft:air"},
+        {pos: [0, 4, 0], state: "minecraft:air"},
+        {pos: [0, 4, 1], state: "minecraft:air"},
+        {pos: [0, 4, 2], state: "minecraft:air"},
+        {pos: [0, 4, 3], state: "minecraft:air"},
+        {pos: [0, 4, 4], state: "minecraft:air"},
+        {pos: [1, 4, 0], state: "minecraft:air"},
+        {pos: [1, 4, 1], state: "minecraft:air"},
+        {pos: [1, 4, 2], state: "minecraft:air"},
+        {pos: [1, 4, 3], state: "minecraft:air"},
+        {pos: [1, 4, 4], state: "minecraft:air"},
+        {pos: [2, 4, 0], state: "minecraft:air"},
+        {pos: [2, 4, 1], state: "minecraft:air"},
+        {pos: [2, 4, 2], state: "minecraft:air"},
+        {pos: [2, 4, 3], state: "minecraft:air"},
+        {pos: [2, 4, 4], state: "minecraft:air"},
+        {pos: [3, 4, 0], state: "minecraft:air"},
+        {pos: [3, 4, 1], state: "minecraft:air"},
+        {pos: [3, 4, 2], state: "minecraft:air"},
+        {pos: [3, 4, 3], state: "minecraft:air"},
+        {pos: [3, 4, 4], state: "minecraft:air"},
+        {pos: [4, 4, 0], state: "minecraft:air"},
+        {pos: [4, 4, 1], state: "minecraft:air"},
+        {pos: [4, 4, 2], state: "minecraft:air"},
+        {pos: [4, 4, 3], state: "minecraft:air"},
+        {pos: [4, 4, 4], state: "minecraft:air"}
+    ],
+    entities: [],
+    palette: [
+        "minecraft:polished_andesite",
+        "minecraft:air",
+        "minecraft:composter{level:0}",
+        "computercraft:turtle_normal{facing:south,waterlogged:false}"
+    ]
+}

projects/fabric/src/main/java/dan200/computercraft/shared/platform/PlatformHelperImpl.java

@@ -67,7 +67,6 @@ import net.minecraft.world.inventory.AbstractContainerMenu;
 import net.minecraft.world.inventory.CraftingContainer;
 import net.minecraft.world.inventory.MenuType;
 import net.minecraft.world.item.*;
-import net.minecraft.world.item.context.UseOnContext;
 import net.minecraft.world.item.crafting.Ingredient;
 import net.minecraft.world.item.crafting.Recipe;
 import net.minecraft.world.level.Level;
@@ -84,7 +83,10 @@ import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Objects;
-import java.util.function.*;
+import java.util.function.BiFunction;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.function.Supplier;
 
 @AutoService(dan200.computercraft.impl.PlatformHelper.class)
 public class PlatformHelperImpl implements PlatformHelper {
@@ -309,17 +311,10 @@ public class PlatformHelperImpl implements PlatformHelper {
     }
 
     @Override
-    public InteractionResult useOn(ServerPlayer player, ItemStack stack, BlockHitResult hit, Predicate<BlockState> canUseBlock) {
+    public UseOnResult useOn(ServerPlayer player, ItemStack stack, BlockHitResult hit) {
         var result = UseBlockCallback.EVENT.invoker().interact(player, player.level(), InteractionHand.MAIN_HAND, hit);
-        if (result != InteractionResult.PASS) return result;
+        if (result != InteractionResult.PASS) return new UseOnResult.Handled(result);
-
+        return new UseOnResult.Continue(true, true);
-        var block = player.level().getBlockState(hit.getBlockPos());
-        if (!block.isAir() && canUseBlock.test(block)) {
-            var useResult = block.use(player.level(), player, InteractionHand.MAIN_HAND, hit);
-            if (useResult.consumesAction()) return useResult;
-        }
-
-        return stack.useOn(new UseOnContext(player, InteractionHand.MAIN_HAND, hit));
     }
 
     private record RegistryWrapperImpl<T>(

projects/forge/src/main/java/dan200/computercraft/shared/platform/PlatformHelperImpl.java

@@ -78,7 +78,10 @@ import net.minecraftforge.registries.RegistryObject;
 
 import javax.annotation.Nullable;
 import java.util.*;
-import java.util.function.*;
+import java.util.function.BiFunction;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.function.Supplier;
 
 @AutoService(dan200.computercraft.impl.PlatformHelper.class)
 public class PlatformHelperImpl implements PlatformHelper {
@@ -324,25 +327,18 @@ public class PlatformHelperImpl implements PlatformHelper {
     }
 
     @Override
-    public InteractionResult useOn(ServerPlayer player, ItemStack stack, BlockHitResult hit, Predicate<BlockState> canUseBlock) {
+    public UseOnResult useOn(ServerPlayer player, ItemStack stack, BlockHitResult hit) {
-        var level = player.level();
         var pos = hit.getBlockPos();
         var event = ForgeHooks.onRightClickBlock(player, InteractionHand.MAIN_HAND, pos, hit);
-        if (event.isCanceled()) return event.getCancellationResult();
+        if (event.isCanceled()) return new UseOnResult.Handled(event.getCancellationResult());
 
         var context = new UseOnContext(player, InteractionHand.MAIN_HAND, hit);
         if (event.getUseItem() != Event.Result.DENY) {
             var result = stack.onItemUseFirst(context);
-            if (result != InteractionResult.PASS) return result;
+            if (result != InteractionResult.PASS) return new UseOnResult.Handled(event.getCancellationResult());
         }
 
-        var block = level.getBlockState(hit.getBlockPos());
+        return new UseOnResult.Continue(event.getUseBlock() != Event.Result.DENY, event.getUseItem() != Event.Result.DENY);
-        if (event.getUseBlock() != Event.Result.DENY && !block.isAir() && canUseBlock.test(block)) {
-            var useResult = block.use(level, player, InteractionHand.MAIN_HAND, hit);
-            if (useResult.consumesAction()) return useResult;
-        }
-
-        return event.getUseItem() == Event.Result.DENY ? InteractionResult.PASS : stack.useOn(context);
     }
 
     @Override