Clean up disk <-> drive right clicking

Oh dear. I'd originally set out to *remove* logic from DiskItem — we're
so close to being able to remove this item in 1.21! However, while
looking at this code, I realised I could remove the whole Forge-specific
doesSneakBypassUse.

We now remove the use hook on the block, and override useOn on the item.
Obvious in retrospect!

projects/common/src/main/java/dan200/computercraft/shared/media/items/DiskItem.java

@@ -4,19 +4,18 @@
 
 package dan200.computercraft.shared.media.items;
 
-import dan200.computercraft.annotations.ForgeOverride;
 import dan200.computercraft.core.util.Colour;
 import dan200.computercraft.shared.ModRegistry;
 import dan200.computercraft.shared.common.IColouredItem;
+import dan200.computercraft.shared.peripheral.diskdrive.DiskDriveBlock;
 import net.minecraft.ChatFormatting;
-import net.minecraft.core.BlockPos;
 import net.minecraft.network.chat.Component;
-import net.minecraft.world.entity.player.Player;
+import net.minecraft.world.InteractionResult;
 import net.minecraft.world.item.Item;
 import net.minecraft.world.item.ItemStack;
 import net.minecraft.world.item.TooltipFlag;
+import net.minecraft.world.item.context.UseOnContext;
 import net.minecraft.world.level.Level;
-import net.minecraft.world.level.LevelReader;
 import org.jspecify.annotations.Nullable;
 
 import java.util.List;
@@ -47,9 +46,9 @@ public class DiskItem extends Item implements IColouredItem {
         }
     }
 
-    @ForgeOverride
+    @Override
-    public boolean doesSneakBypassUse(ItemStack stack, LevelReader world, BlockPos pos, Player player) {
+    public InteractionResult useOn(UseOnContext context) {
-        return true;
+        return DiskDriveBlock.defaultUseItemOn(context);
     }
 
     public static int getDiskID(ItemStack stack) {

projects/common/src/main/java/dan200/computercraft/shared/media/items/TreasureDiskItem.java

@@ -4,17 +4,16 @@
 
 package dan200.computercraft.shared.media.items;
 
-import dan200.computercraft.annotations.ForgeOverride;
 import dan200.computercraft.core.util.Colour;
 import dan200.computercraft.shared.ModRegistry;
-import net.minecraft.core.BlockPos;
+import dan200.computercraft.shared.peripheral.diskdrive.DiskDriveBlock;
 import net.minecraft.network.chat.Component;
-import net.minecraft.world.entity.player.Player;
+import net.minecraft.world.InteractionResult;
 import net.minecraft.world.item.Item;
 import net.minecraft.world.item.ItemStack;
 import net.minecraft.world.item.TooltipFlag;
+import net.minecraft.world.item.context.UseOnContext;
 import net.minecraft.world.level.Level;
-import net.minecraft.world.level.LevelReader;
 import org.jspecify.annotations.Nullable;
 
 import java.util.List;
@@ -34,9 +33,9 @@ public class TreasureDiskItem extends Item {
         if (!label.isEmpty()) list.add(Component.literal(label));
     }
 
-    @ForgeOverride
+    @Override
-    public boolean doesSneakBypassUse(ItemStack stack, LevelReader world, BlockPos pos, Player player) {
+    public InteractionResult useOn(UseOnContext context) {
-        return true;
+        return DiskDriveBlock.defaultUseItemOn(context);
     }
 
     public static ItemStack create(String subPath, int colourIndex) {

projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDriveBlock.java

@@ -6,12 +6,11 @@ package dan200.computercraft.shared.peripheral.diskdrive;
 
 import dan200.computercraft.shared.ModRegistry;
 import dan200.computercraft.shared.common.HorizontalContainerBlock;
-import dan200.computercraft.shared.platform.PlatformHelper;
 import net.minecraft.core.BlockPos;
 import net.minecraft.core.Direction;
-import net.minecraft.world.InteractionHand;
 import net.minecraft.world.InteractionResult;
-import net.minecraft.world.entity.player.Player;
+import net.minecraft.world.item.Item;
+import net.minecraft.world.item.context.UseOnContext;
 import net.minecraft.world.level.Level;
 import net.minecraft.world.level.block.BaseEntityBlock;
 import net.minecraft.world.level.block.Block;
@@ -21,7 +20,6 @@ import net.minecraft.world.level.block.entity.BlockEntityType;
 import net.minecraft.world.level.block.state.BlockState;
 import net.minecraft.world.level.block.state.StateDefinition;
 import net.minecraft.world.level.block.state.properties.EnumProperty;
-import net.minecraft.world.phys.BlockHitResult;
 import org.jspecify.annotations.Nullable;
 
 public class DiskDriveBlock extends HorizontalContainerBlock {
@@ -42,21 +40,26 @@ public class DiskDriveBlock extends HorizontalContainerBlock {
         properties.add(FACING, STATE);
     }
 
-    @Override
+    /**
-    @Deprecated
+     * A default implementation of {@link Item#useOn(UseOnContext)} for items that can be placed into a drive.
-    public InteractionResult use(BlockState state, Level level, BlockPos pos, Player player, InteractionHand hand, BlockHitResult hit) {
+     *
-        if (player.isCrouching() && level.getBlockEntity(pos) instanceof DiskDriveBlockEntity drive) {
+     * @param context The context of this item usage action.
-            // Try to put a disk into the drive
+     * @return Whether the item was placed or not.
-            var disk = player.getItemInHand(hand);
+     */
-            if (disk.isEmpty()) return InteractionResult.PASS;
+    public static InteractionResult defaultUseItemOn(UseOnContext context) {
+        if (context.getPlayer() == null || !context.getPlayer().isSecondaryUseActive()) return InteractionResult.PASS;
 
-            if (!level.isClientSide && drive.getDiskStack().isEmpty() && PlatformHelper.get().getMedia(disk) != null) {
+        var level = context.getLevel();
-                drive.setDiskStack(disk.split(1));
+        var blockPos = context.getClickedPos();
+        var blockState = level.getBlockState(blockPos);
+        if (blockState.is(ModRegistry.Blocks.DISK_DRIVE.get()) && blockState.getValue(STATE) == DiskDriveState.EMPTY) {
+            if (!level.isClientSide && level.getBlockEntity(blockPos) instanceof DiskDriveBlockEntity drive && drive.getDiskStack().isEmpty()) {
+                drive.setDiskStack(context.getItemInHand().split(1));
             }
             return InteractionResult.sidedSuccess(level.isClientSide);
         }
 
-        return super.use(state, level, pos, player, hand, hit);
+        return InteractionResult.PASS;
     }
 
     @Nullable

projects/fabric/src/main/java/dan200/computercraft/shared/ComputerCraft.java

@@ -25,7 +25,6 @@ import net.fabricmc.fabric.api.event.lifecycle.v1.ServerChunkEvents;
 import net.fabricmc.fabric.api.event.lifecycle.v1.ServerLifecycleEvents;
 import net.fabricmc.fabric.api.event.lifecycle.v1.ServerTickEvents;
 import net.fabricmc.fabric.api.event.player.PlayerBlockBreakEvents;
-import net.fabricmc.fabric.api.event.player.UseBlockCallback;
 import net.fabricmc.fabric.api.itemgroup.v1.ItemGroupEvents;
 import net.fabricmc.fabric.api.lookup.v1.block.BlockApiLookup;
 import net.fabricmc.fabric.api.lookup.v1.item.ItemApiLookup;
@@ -87,7 +86,6 @@ public class ComputerCraft {
         ServerChunkEvents.CHUNK_UNLOAD.register((l, c) -> CommonHooks.onServerChunkUnload(c));
 
         PlayerBlockBreakEvents.BEFORE.register(FabricCommonHooks::onBlockDestroy);
-        UseBlockCallback.EVENT.register(FabricCommonHooks::useOnBlock);
 
         LootTableEvents.MODIFY.register((resourceManager, lootManager, id, tableBuilder, source) -> {
             var pool = CommonHooks.getExtraLootPool(id);

projects/fabric/src/main/java/dan200/computercraft/shared/FabricCommonHooks.java

@@ -4,52 +4,16 @@
 
 package dan200.computercraft.shared;
 
-import dan200.computercraft.shared.media.items.DiskItem;
-import dan200.computercraft.shared.media.items.TreasureDiskItem;
 import dan200.computercraft.shared.peripheral.modem.wired.CableBlock;
 import net.minecraft.core.BlockPos;
-import net.minecraft.server.level.ServerPlayer;
-import net.minecraft.server.level.ServerPlayerGameMode;
-import net.minecraft.world.InteractionHand;
-import net.minecraft.world.InteractionResult;
 import net.minecraft.world.entity.player.Player;
-import net.minecraft.world.item.ItemStack;
 import net.minecraft.world.level.Level;
 import net.minecraft.world.level.block.entity.BlockEntity;
 import net.minecraft.world.level.block.state.BlockState;
-import net.minecraft.world.phys.BlockHitResult;
 import org.jspecify.annotations.Nullable;
 
 public class FabricCommonHooks {
     public static boolean onBlockDestroy(Level level, Player player, BlockPos pos, BlockState state, @Nullable BlockEntity blockEntity) {
         return !(state.getBlock() instanceof CableBlock cable) || !cable.onCustomDestroyBlock(state, level, pos, player);
     }
-
-    /**
-     * Allow placing disks/treasure disks into disk drives by clicking on them.
-     *
-     * @param player    The player placing the block.
-     * @param level     The current level.
-     * @param hand      The player's hand.
-     * @param hitResult The hit collision.
-     * @return Whether this interaction succeeded.
-     * @see ServerPlayerGameMode#useItemOn(ServerPlayer, Level, ItemStack, InteractionHand, BlockHitResult) The original source of this logic.
-     */
-    public static InteractionResult useOnBlock(Player player, Level level, InteractionHand hand, BlockHitResult hitResult) {
-        if (player.isSpectator()) return InteractionResult.PASS;
-
-        var block = level.getBlockState(hitResult.getBlockPos());
-        if (block.getBlock() != ModRegistry.Blocks.DISK_DRIVE.get()) return InteractionResult.PASS;
-
-        if (player.isSecondaryUseActive() && doesSneakBypassUse(player.getMainHandItem()) && doesSneakBypassUse(player.getOffhandItem())) {
-            var result = block.use(level, player, hand, hitResult);
-            if (result.consumesAction()) return result;
-        }
-
-        return InteractionResult.PASS;
-    }
-
-    private static boolean doesSneakBypassUse(ItemStack stack) {
-        return stack.isEmpty() || stack.getItem() instanceof DiskItem || stack.getItem() instanceof TreasureDiskItem;
-    }
 }