Fix creating a zero-sized pocket terminal

When the terminal data is not present, width/height are set to 0, rather
than the terminal's width/height. This meant we'd create an empty
terminal, which then crashes when we try to render it.

We now make the terminal nullable and initialise it the first time we
receive the terminal data. To prevent future mistakes, we hide
width/height, and use TerminalState.create everywhere.

Fixes #1765

projects/common/src/client/java/dan200/computercraft/client/pocket/ClientPocketComputers.java

@@ -6,7 +6,6 @@
 
 import dan200.computercraft.shared.computer.core.ComputerState;
 import dan200.computercraft.shared.computer.core.ServerComputer;
-import dan200.computercraft.shared.computer.terminal.NetworkedTerminal;
 import dan200.computercraft.shared.computer.terminal.TerminalState;
 import dan200.computercraft.shared.network.client.PocketComputerDataMessage;
 import dan200.computercraft.shared.pocket.items.PocketComputerItem;
@@ -47,13 +46,10 @@ public static void remove(UUID id) {
     public static void setState(UUID instanceId, ComputerState state, int lightColour, TerminalState terminalData) {
         var computer = instances.get(instanceId);
         if (computer == null) {
-            var terminal = new NetworkedTerminal(terminalData.width, terminalData.height, terminalData.colour);
+            instances.put(instanceId, new PocketComputerData(state, lightColour, terminalData));
-            instances.put(instanceId, computer = new PocketComputerData(state, lightColour, terminal));
         } else {
-            computer.setState(state, lightColour);
+            computer.setState(state, lightColour, terminalData);
         }
-
-        if (terminalData.hasTerminal()) terminalData.apply(computer.getTerminal());
     }
 
     public static @Nullable PocketComputerData get(ItemStack stack) {

projects/common/src/client/java/dan200/computercraft/client/pocket/PocketComputerData.java

@@ -6,8 +6,11 @@
 
 import dan200.computercraft.shared.computer.core.ComputerState;
 import dan200.computercraft.shared.computer.terminal.NetworkedTerminal;
+import dan200.computercraft.shared.computer.terminal.TerminalState;
 import dan200.computercraft.shared.pocket.core.PocketServerComputer;
 
+import javax.annotation.Nullable;
+
 /**
  * Clientside data about a pocket computer.
  * <p>
@@ -19,21 +22,21 @@
  * @see PocketServerComputer The server-side pocket computer.
  */
 public final class PocketComputerData {
-    private final NetworkedTerminal terminal;
+    private @Nullable NetworkedTerminal terminal;
     private ComputerState state;
     private int lightColour;
 
-    PocketComputerData(ComputerState state, int lightColour, NetworkedTerminal terminal) {
+    PocketComputerData(ComputerState state, int lightColour, TerminalState terminalData) {
         this.state = state;
         this.lightColour = lightColour;
-        this.terminal = terminal;
+        if (terminalData.hasTerminal()) terminal = terminalData.create();
     }
 
     public int getLightState() {
         return state != ComputerState.OFF ? lightColour : -1;
     }
 
-    public NetworkedTerminal getTerminal() {
+    public @Nullable NetworkedTerminal getTerminal() {
         return terminal;
     }
 
@@ -41,8 +44,16 @@ public ComputerState getState() {
         return state;
     }
 
-    void setState(ComputerState state, int lightColour) {
+    void setState(ComputerState state, int lightColour, TerminalState terminalData) {
         this.state = state;
         this.lightColour = lightColour;
+
+        if (terminalData.hasTerminal()) {
+            if (terminal == null) {
+                terminal = terminalData.create();
+            } else {
+                terminalData.apply(terminal);
+            }
+        }
     }
 }

projects/common/src/main/java/dan200/computercraft/shared/computer/terminal/TerminalState.java

@@ -18,10 +18,9 @@
  * states, etc...
  */
 public class TerminalState {
-    public final boolean colour;
+    private final boolean colour;
-
+    private final int width;
-    public final int width;
+    private final int height;
-    public final int height;
 
     @Nullable
     private final ByteBuf buffer;

projects/common/src/main/java/dan200/computercraft/shared/peripheral/monitor/ClientMonitor.java

@@ -56,8 +56,11 @@ public boolean pollTerminalChanged() {
 
     void read(TerminalState state) {
         if (state.hasTerminal()) {
-            if (terminal == null) terminal = new NetworkedTerminal(state.width, state.height, state.colour);
+            if (terminal == null) {
-            state.apply(terminal);
+                terminal = state.create();
+            } else {
+                state.apply(terminal);
+            }
             terminalChanged = true;
         } else {
             if (terminal != null) {

projects/common/src/test/java/dan200/computercraft/shared/computer/terminal/TerminalStateTest.java

@@ -11,7 +11,8 @@
 
 import java.util.Random;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 /**
  * Tests {@link TerminalState} round tripping works as expected.
@@ -42,6 +43,7 @@ private static NetworkedTerminal randomTerminal() {
     private static void checkEqual(Terminal expected, Terminal actual) {
         assertNotNull(expected, "Expected cannot be null");
         assertNotNull(actual, "Actual cannot be null");
+        assertEquals(expected.isColour(), actual.isColour(), "isColour must match");
         assertEquals(expected.getHeight(), actual.getHeight(), "Heights must match");
         assertEquals(expected.getWidth(), actual.getWidth(), "Widths must match");
 
@@ -51,13 +53,6 @@ private static void checkEqual(Terminal expected, Terminal actual) {
     }
 
     private static NetworkedTerminal read(FriendlyByteBuf buffer) {
-        var state = new TerminalState(buffer);
+        return new TerminalState(buffer).create();
-        assertTrue(state.colour);
-
-        if (!state.hasTerminal()) return null;
-
-        var other = new NetworkedTerminal(state.width, state.height, true);
-        state.apply(other);
-        return other;
     }
 }

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

@@ -41,7 +41,7 @@ fun Sync_state(context: GameTestHelper) = context.sequence {
             val pocketComputer = ClientPocketComputers.get(minecraft.player!!.mainHandItem)!!
             assertEquals(ComputerState.ON, pocketComputer.state)
 
-            val term = pocketComputer.terminal
+            val term = pocketComputer.terminal!!
             assertEquals("Hello, world!", term.getLine(0).toString().trim(), "Terminal contents is synced")
         }
         // Update the terminal contents again.
@@ -57,7 +57,7 @@ fun Sync_state(context: GameTestHelper) = context.sequence {
             val pocketComputer = ClientPocketComputers.get(minecraft.player!!.mainHandItem)!!
             assertEquals(ComputerState.BLINKING, pocketComputer.state)
 
-            val term = pocketComputer.terminal
+            val term = pocketComputer.terminal!!
             assertEquals("Updated text :)", term.getLine(0).toString().trim(), "Terminal contents is synced")
         }
     }