Unify command computer permission checks

- Move the command permisssion checks to a new
   ComputerFamily.checkUsable method (from
   CommandComputerBlockEntity and ViewComputerMenu). I don't feel great
   about putting new functionality in ComputerFamily (trying to move
   away from it), but I think this is fine for now.

 - Use this method from within the computer menu and computer block, to
   check whether computers can be interacted with.

 - Remove ViewComputerMenu, as it now no longer needs any special
   is-usable logic.

projects/common/src/client/java/dan200/computercraft/client/ClientRegistry.java

@@ -25,7 +25,6 @@ import dan200.computercraft.shared.common.IColouredItem;
 import dan200.computercraft.shared.computer.core.ComputerState;
 import dan200.computercraft.shared.computer.core.ServerContext;
 import dan200.computercraft.shared.computer.inventory.AbstractComputerMenu;
-import dan200.computercraft.shared.computer.inventory.ViewComputerMenu;
 import dan200.computercraft.shared.media.items.DiskItem;
 import dan200.computercraft.shared.media.items.TreasureDiskItem;
 import net.minecraft.Util;
@@ -90,8 +89,6 @@ public final class ClientRegistry {
         MenuScreens.register(ModRegistry.Menus.DISK_DRIVE.get(), DiskDriveScreen::new);
         MenuScreens.register(ModRegistry.Menus.PRINTOUT.get(), PrintoutScreen::new);
 
-        MenuScreens.<ViewComputerMenu, ComputerScreen<ViewComputerMenu>>register(ModRegistry.Menus.VIEW_COMPUTER.get(), ComputerScreen::new);
-
         registerItemProperty(itemProperties, "state",
             new UnclampedPropertyFunction((stack, world, player, random) -> {
                 var computer = ClientPocketComputers.get(stack);

projects/common/src/main/java/dan200/computercraft/shared/ModRegistry.java

@@ -29,7 +29,6 @@ import dan200.computercraft.shared.computer.blocks.ComputerBlock;
 import dan200.computercraft.shared.computer.blocks.ComputerBlockEntity;
 import dan200.computercraft.shared.computer.core.ComputerFamily;
 import dan200.computercraft.shared.computer.inventory.ComputerMenuWithoutInventory;
-import dan200.computercraft.shared.computer.inventory.ViewComputerMenu;
 import dan200.computercraft.shared.computer.items.CommandComputerItem;
 import dan200.computercraft.shared.computer.items.ComputerItem;
 import dan200.computercraft.shared.computer.recipe.ComputerUpgradeRecipe;
@@ -310,9 +309,6 @@ public final class ModRegistry {
                 HeldItemContainerData::new,
                 (id, inventory, data) -> new HeldItemMenu(Menus.PRINTOUT.get(), id, inventory.player, data.getHand())
             ));
-
-        public static final RegistryEntry<MenuType<ViewComputerMenu>> VIEW_COMPUTER = REGISTRY.register("view_computer",
-            () -> ContainerData.toType(ComputerContainerData::new, ViewComputerMenu::new));
     }
 
     static class ArgumentTypes {

projects/common/src/main/java/dan200/computercraft/shared/command/CommandComputerCraft.java

@@ -18,7 +18,7 @@ import dan200.computercraft.shared.command.text.TableBuilder;
 import dan200.computercraft.shared.computer.core.ComputerFamily;
 import dan200.computercraft.shared.computer.core.ServerComputer;
 import dan200.computercraft.shared.computer.core.ServerContext;
-import dan200.computercraft.shared.computer.inventory.ViewComputerMenu;
+import dan200.computercraft.shared.computer.inventory.ComputerMenuWithoutInventory;
 import dan200.computercraft.shared.computer.metrics.basic.Aggregate;
 import dan200.computercraft.shared.computer.metrics.basic.AggregatedMetric;
 import dan200.computercraft.shared.computer.metrics.basic.BasicComputerMetricsObserver;
@@ -269,7 +269,7 @@ public final class CommandComputerCraft {
 
             @Override
             public AbstractContainerMenu createMenu(int id, Inventory player, Player entity) {
-                return new ViewComputerMenu(id, player, computer);
+                return new ComputerMenuWithoutInventory(ModRegistry.Menus.COMPUTER.get(), id, player, p -> true, computer);
             }
         });
         return 1;

projects/common/src/main/java/dan200/computercraft/shared/computer/blocks/AbstractComputerBlockEntity.java

@@ -81,7 +81,8 @@ public abstract class AbstractComputerBlockEntity extends BlockEntity implements
     }
 
     public boolean isUsable(Player player) {
-        return BaseContainerBlockEntity.canUnlock(player, lockCode, getDisplayName())
+        return getFamily().checkUsable(player)
+            && BaseContainerBlockEntity.canUnlock(player, lockCode, getDisplayName())
             && Container.stillValidBlockEntity(this, player, getInteractRange());
     }
 

projects/common/src/main/java/dan200/computercraft/shared/computer/blocks/CommandComputerBlockEntity.java

@@ -7,10 +7,7 @@ package dan200.computercraft.shared.computer.blocks;
 import dan200.computercraft.shared.computer.apis.CommandAPI;
 import dan200.computercraft.shared.computer.core.ComputerFamily;
 import dan200.computercraft.shared.computer.core.ServerComputer;
-import dan200.computercraft.shared.config.Config;
 import net.minecraft.core.BlockPos;
-import net.minecraft.network.chat.Component;
-import net.minecraft.world.entity.player.Player;
 import net.minecraft.world.level.block.entity.BlockEntityType;
 import net.minecraft.world.level.block.state.BlockState;
 
@@ -25,26 +22,4 @@ public class CommandComputerBlockEntity extends ComputerBlockEntity {
         computer.addAPI(new CommandAPI(computer));
         return computer;
     }
-
-    @Override
-    public boolean isUsable(Player player) {
-        return isCommandUsable(player) && super.isUsable(player);
-    }
-
-    public static boolean isCommandUsable(Player player) {
-        var server = player.getServer();
-        if (server == null || !server.isCommandBlockEnabled()) {
-            player.displayClientMessage(Component.translatable("advMode.notEnabled"), true);
-            return false;
-        } else if (!canUseCommandBlock(player)) {
-            player.displayClientMessage(Component.translatable("advMode.notAllowed"), true);
-            return false;
-        }
-
-        return true;
-    }
-
-    private static boolean canUseCommandBlock(Player player) {
-        return Config.commandRequireCreative ? player.canUseGameMasterBlocks() : player.hasPermissions(2);
-    }
 }

projects/common/src/main/java/dan200/computercraft/shared/computer/blocks/ComputerBlockEntity.java

@@ -67,7 +67,7 @@ public class ComputerBlockEntity extends AbstractComputerBlockEntity {
     @Nullable
     @Override
     public AbstractContainerMenu createMenu(int id, Inventory inventory, Player player) {
-        return new ComputerMenuWithoutInventory(ModRegistry.Menus.COMPUTER.get(), id, inventory, this::isUsableByPlayer, createServerComputer(), getFamily());
+        return new ComputerMenuWithoutInventory(ModRegistry.Menus.COMPUTER.get(), id, inventory, this::isUsableByPlayer, createServerComputer());
     }
 
     public IPeripheral peripheral() {

projects/common/src/main/java/dan200/computercraft/shared/computer/core/ComputerFamily.java

@@ -4,8 +4,45 @@
 
 package dan200.computercraft.shared.computer.core;
 
+import dan200.computercraft.shared.config.Config;
+import net.minecraft.network.chat.Component;
+import net.minecraft.world.entity.player.Player;
+
 public enum ComputerFamily {
     NORMAL,
     ADVANCED,
-    COMMAND,
+    COMMAND;
+
+    /**
+     * Check whether computers with this family can be used by the provided player.
+     * <p>
+     * This method is not pure. On failure, the method may send a message to the player telling them why they cannot
+     * interact with the computer.
+     *
+     * @param player The player trying to use a computer.
+     * @return Whether this computer family can be used.
+     */
+    public boolean checkUsable(Player player) {
+        return switch (this) {
+            case NORMAL, ADVANCED -> true;
+            case COMMAND -> checkCommandUsable(player);
+        };
+    }
+
+    private static boolean checkCommandUsable(Player player) {
+        var server = player.getServer();
+        if (server == null || !server.isCommandBlockEnabled()) {
+            player.displayClientMessage(Component.translatable("advMode.notEnabled"), true);
+            return false;
+        } else if (!canUseCommandBlock(player)) {
+            player.displayClientMessage(Component.translatable("advMode.notAllowed"), true);
+            return false;
+        }
+
+        return true;
+    }
+
+    private static boolean canUseCommandBlock(Player player) {
+        return Config.commandRequireCreative ? player.canUseGameMasterBlocks() : player.hasPermissions(2);
+    }
 }

projects/common/src/main/java/dan200/computercraft/shared/computer/core/ServerComputer.java

@@ -23,6 +23,7 @@ import dan200.computercraft.shared.network.client.ComputerTerminalClientMessage;
 import dan200.computercraft.shared.network.server.ServerNetworking;
 import net.minecraft.core.BlockPos;
 import net.minecraft.server.level.ServerLevel;
+import net.minecraft.world.entity.player.Player;
 import net.minecraft.world.inventory.AbstractContainerMenu;
 
 import javax.annotation.Nullable;
@@ -94,7 +95,6 @@ public class ServerComputer implements InputHandler, ComputerEnvironment {
         terminalChanged.set(true);
     }
 
-
     public void tickServer() {
         ticksSincePing++;
         computer.tick();
@@ -135,6 +135,17 @@ public class ServerComputer implements InputHandler, ComputerEnvironment {
         ServerContext.get(level.getServer()).registry().remove(this);
     }
 
+    /**
+     * Check whether this computer is usable by a player.
+     *
+     * @param player The player trying to use this computer.
+     * @return Whether this computer can be used.
+     */
+    public final boolean checkUsable(Player player) {
+        return ServerContext.get(level.getServer()).registry().get(instanceUUID) == this
+            && getFamily().checkUsable(player);
+    }
+
     private void sendToAllInteracting(Function<AbstractContainerMenu, NetworkMessage<ClientNetworkContext>> createPacket) {
         var server = level.getServer();
 

projects/common/src/main/java/dan200/computercraft/shared/computer/inventory/AbstractComputerMenu.java

@@ -59,7 +59,7 @@ public abstract class AbstractComputerMenu extends AbstractContainerMenu impleme
 
     @Override
     public boolean stillValid(Player player) {
-        return canUse.test(player);
+        return (computer == null || computer.checkUsable(player)) && canUse.test(player);
     }
 
     public ComputerFamily getFamily() {

projects/common/src/main/java/dan200/computercraft/shared/computer/inventory/ComputerMenuWithoutInventory.java

@@ -4,7 +4,6 @@
 
 package dan200.computercraft.shared.computer.inventory;
 
-import dan200.computercraft.shared.computer.core.ComputerFamily;
 import dan200.computercraft.shared.computer.core.ServerComputer;
 import dan200.computercraft.shared.container.InvisibleSlot;
 import dan200.computercraft.shared.network.container.ComputerContainerData;
@@ -23,9 +22,9 @@ import java.util.function.Predicate;
 public class ComputerMenuWithoutInventory extends AbstractComputerMenu {
     public ComputerMenuWithoutInventory(
         MenuType<? extends AbstractComputerMenu> type, int id, Inventory player, Predicate<Player> canUse,
-        ServerComputer computer, ComputerFamily family
+        ServerComputer computer
     ) {
-        super(type, id, canUse, family, computer, null);
+        super(type, id, canUse, computer.getFamily(), computer, null);
         addSlots(player);
     }
 

projects/common/src/main/java/dan200/computercraft/shared/computer/inventory/ViewComputerMenu.java

@@ -1,39 +0,0 @@
-// SPDX-FileCopyrightText: 2017 The CC: Tweaked Developers
-//
-// SPDX-License-Identifier: MPL-2.0
-
-package dan200.computercraft.shared.computer.inventory;
-
-import dan200.computercraft.shared.ModRegistry;
-import dan200.computercraft.shared.computer.blocks.CommandComputerBlockEntity;
-import dan200.computercraft.shared.computer.core.ComputerFamily;
-import dan200.computercraft.shared.computer.core.ServerComputer;
-import dan200.computercraft.shared.computer.core.ServerContext;
-import dan200.computercraft.shared.network.container.ComputerContainerData;
-import net.minecraft.world.entity.player.Inventory;
-import net.minecraft.world.entity.player.Player;
-
-
-public class ViewComputerMenu extends ComputerMenuWithoutInventory {
-    public ViewComputerMenu(int id, Inventory player, ServerComputer computer) {
-        super(ModRegistry.Menus.VIEW_COMPUTER.get(), id, player, p -> canInteractWith(computer, p), computer, computer.getFamily());
-    }
-
-    public ViewComputerMenu(int id, Inventory player, ComputerContainerData data) {
-        super(ModRegistry.Menus.VIEW_COMPUTER.get(), id, player, data);
-    }
-
-    private static boolean canInteractWith(ServerComputer computer, Player player) {
-        // If this computer no longer exists then discard it.
-        if (ServerContext.get(computer.getLevel().getServer()).registry().get(computer.getInstanceUUID()) != computer) {
-            return false;
-        }
-
-        // If we're a command computer then ensure we're in creative
-        if (computer.getFamily() == ComputerFamily.COMMAND && !CommandComputerBlockEntity.isCommandUsable(player)) {
-            return false;
-        }
-
-        return true;
-    }
-}

projects/common/src/main/java/dan200/computercraft/shared/pocket/inventory/PocketComputerMenuProvider.java

@@ -50,7 +50,7 @@ public class PocketComputerMenuProvider implements MenuProvider {
                 var stack = p.getItemInHand(hand);
                 return stack.getItem() == item && PocketComputerItem.getServerComputer(assertNonNull(entity.level().getServer()), stack) == computer;
             },
-            computer, item.getFamily()
+            computer
         );
     }
 }