From 27f2ab364cd7a4ad74e5d7963adfe180c1335767 Mon Sep 17 00:00:00 2001 From: Jonathan Coates Date: Thu, 13 Mar 2025 21:47:14 +0000 Subject: [PATCH] Clean up disk <-> drive right clicking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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! --- .../shared/media/items/DiskItem.java | 13 ++++--- .../shared/media/items/TreasureDiskItem.java | 13 ++++--- .../peripheral/diskdrive/DiskDriveBlock.java | 31 ++++++++-------- .../computercraft/shared/ComputerCraft.java | 2 -- .../shared/FabricCommonHooks.java | 36 ------------------- 5 files changed, 29 insertions(+), 66 deletions(-) diff --git a/projects/common/src/main/java/dan200/computercraft/shared/media/items/DiskItem.java b/projects/common/src/main/java/dan200/computercraft/shared/media/items/DiskItem.java index 0cfaa6864..c70b028e6 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/media/items/DiskItem.java +++ b/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 - public boolean doesSneakBypassUse(ItemStack stack, LevelReader world, BlockPos pos, Player player) { - return true; + @Override + public InteractionResult useOn(UseOnContext context) { + return DiskDriveBlock.defaultUseItemOn(context); } public static int getDiskID(ItemStack stack) { diff --git a/projects/common/src/main/java/dan200/computercraft/shared/media/items/TreasureDiskItem.java b/projects/common/src/main/java/dan200/computercraft/shared/media/items/TreasureDiskItem.java index 31ed46d66..a17fea6a3 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/media/items/TreasureDiskItem.java +++ b/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 - public boolean doesSneakBypassUse(ItemStack stack, LevelReader world, BlockPos pos, Player player) { - return true; + @Override + public InteractionResult useOn(UseOnContext context) { + return DiskDriveBlock.defaultUseItemOn(context); } public static ItemStack create(String subPath, int colourIndex) { diff --git a/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDriveBlock.java b/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDriveBlock.java index 1a63c9a25..fdeb443c4 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDriveBlock.java +++ b/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 - public InteractionResult use(BlockState state, Level level, BlockPos pos, Player player, InteractionHand hand, BlockHitResult hit) { - if (player.isCrouching() && level.getBlockEntity(pos) instanceof DiskDriveBlockEntity drive) { - // Try to put a disk into the drive - var disk = player.getItemInHand(hand); - if (disk.isEmpty()) return InteractionResult.PASS; + /** + * A default implementation of {@link Item#useOn(UseOnContext)} for items that can be placed into a drive. + * + * @param context The context of this item usage action. + * @return Whether the item was placed or not. + */ + 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) { - drive.setDiskStack(disk.split(1)); + var level = context.getLevel(); + 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 diff --git a/projects/fabric/src/main/java/dan200/computercraft/shared/ComputerCraft.java b/projects/fabric/src/main/java/dan200/computercraft/shared/ComputerCraft.java index 4234c7ecf..a835997fb 100644 --- a/projects/fabric/src/main/java/dan200/computercraft/shared/ComputerCraft.java +++ b/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); diff --git a/projects/fabric/src/main/java/dan200/computercraft/shared/FabricCommonHooks.java b/projects/fabric/src/main/java/dan200/computercraft/shared/FabricCommonHooks.java index 62a72277b..8ef9cc632 100644 --- a/projects/fabric/src/main/java/dan200/computercraft/shared/FabricCommonHooks.java +++ b/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; - } }