Rewrite argument validation

This uses a new utility class ArgumentHelper, which provides convenience methods for parsing arguments from an array of Objects. The format of error messages has also changed. It now follows a format similar to Lua's native error messages - including the invalid argument index, the expected type and the type actually received.
2025-11-12 03:13:00 +00:00 · 2017-06-12 10:28:31 +01:00
parent 0f982e6199
commit bffc3c18cc
21 changed files with 451 additions and 650 deletions
--- a/src/main/java/dan200/computercraft/shared/peripheral/commandblock/CommandBlockPeripheral.java
+++ b/src/main/java/dan200/computercraft/shared/peripheral/commandblock/CommandBlockPeripheral.java
@@ -16,6 +16,8 @@ import net.minecraft.util.math.BlockPos;

 import javax.annotation.Nonnull;

+import static dan200.computercraft.core.apis.ArgumentHelper.getString;
+
 public class CommandBlockPeripheral implements IPeripheral
 {
    private final TileEntityCommandBlock m_commandBlock;
@@ -67,12 +69,7 @@ public class CommandBlockPeripheral implements IPeripheral
            case 1:
            {
                // setCommand
-                if( arguments.length < 1 || !(arguments[0] instanceof String) )
-                {
-                    throw new LuaException( "Expected string" );
-                }
-
-                final String command = (String) arguments[ 0 ];
+                final String command = getString( arguments, 0 );
                context.issueMainThreadTask( new ILuaTask()
                {
                    @Override
--- a/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDrivePeripheral.java
+++ b/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDrivePeripheral.java
@@ -18,6 +18,8 @@ import net.minecraft.item.ItemStack;

 import javax.annotation.Nonnull;

+import static dan200.computercraft.core.apis.ArgumentHelper.optString;
+
 public class DiskDrivePeripheral implements IPeripheral
 {
    private final TileDiskDrive m_diskDrive;
@@ -78,15 +80,7 @@ public class DiskDrivePeripheral implements IPeripheral
            case 2:
            {
                // setDiskLabel
-                String label = null;
-                if( arguments.length > 0 )
-                {
-                    if( arguments[0] != null && !(arguments[0] instanceof String) )
-                    {
-                        throw new LuaException( "Expected string" );
-                    }
-                    label = (String)arguments[0];
-                }
+                String label = optString( arguments, 0, null );

                IMedia media = m_diskDrive.getDiskMedia();
                if( media != null )
--- a/src/main/java/dan200/computercraft/shared/peripheral/modem/ModemPeripheral.java
+++ b/src/main/java/dan200/computercraft/shared/peripheral/modem/ModemPeripheral.java
@@ -21,6 +21,8 @@ import net.minecraft.world.World;

 import javax.annotation.Nonnull;

+import static dan200.computercraft.core.apis.ArgumentHelper.getInt;
+
 public abstract class ModemPeripheral
    implements IPeripheral, IPacketSender, IPacketReceiver
 {
@@ -147,11 +149,7 @@ public abstract class ModemPeripheral
    
    private static int parseChannel( Object[] arguments, int index ) throws LuaException
    {
-        if( arguments.length <= index || !(arguments[index] instanceof Double) )
-        {
-            throw new LuaException( "Expected number" );
-        }
-        int channel = (int)((Double)arguments[index]).doubleValue();
+        int channel = getInt( arguments, index );
        if( channel < 0 || channel > 65535 )
        {
            throw new LuaException( "Expected number in range 0-65535" );
--- a/src/main/java/dan200/computercraft/shared/peripheral/modem/TileCable.java
+++ b/src/main/java/dan200/computercraft/shared/peripheral/modem/TileCable.java
@@ -40,6 +40,8 @@ import javax.annotation.Nonnull;
 import java.io.File;
 import java.util.*;

+import static dan200.computercraft.core.apis.ArgumentHelper.getString;
+
 public class TileCable extends TileModemBase
    implements IPacketNetwork
 {
@@ -116,15 +118,6 @@ public class TileCable extends TileModemBase
            return newMethods;
        }

-        private String parseString( Object[] arguments, int index ) throws LuaException
-        {
-            if( arguments.length < (index + 1) || !(arguments[index] instanceof String) )
-            {
-                throw new LuaException( "Expected string" );
-            }
-            return (String)arguments[index];
-        }
-
        @Override
        public Object[] callMethod( @Nonnull IComputerAccess computer, @Nonnull ILuaContext context, int method, @Nonnull Object[] arguments ) throws LuaException, InterruptedException
        {
@@ -148,13 +141,13 @@ public class TileCable extends TileModemBase
                case 1:
                {
                    // isPresentRemote
-                    String type = m_entity.getTypeRemote( parseString( arguments, 0 ) );
+                    String type = m_entity.getTypeRemote( getString( arguments, 0 ) );
                    return new Object[] { type != null };
                }
                case 2:
                {
                    // getTypeRemote
-                    String type = m_entity.getTypeRemote( parseString( arguments, 0 ) );
+                    String type = m_entity.getTypeRemote( getString( arguments, 0 ) );
                    if( type != null )
                    {
                        return new Object[] { type };
@@ -164,7 +157,7 @@ public class TileCable extends TileModemBase
                case 3:
                {
                    // getMethodsRemote
-                    String[] methodNames = m_entity.getMethodNamesRemote( parseString( arguments, 0 ) );
+                    String[] methodNames = m_entity.getMethodNamesRemote( getString( arguments, 0 ) );
                    if( methodNames != null )
                    {
                        Map<Object,Object> table = new HashMap<Object,Object>();
@@ -178,8 +171,8 @@ public class TileCable extends TileModemBase
                case 4:
                {
                    // callRemote
-                    String remoteName = parseString( arguments, 0 );
-                    String methodName = parseString( arguments, 1 );
+                    String remoteName = getString( arguments, 0 );
+                    String methodName = getString( arguments, 1 );
                    Object[] methodArgs = new Object[ arguments.length - 2 ];
                    System.arraycopy( arguments, 2, methodArgs, 0, arguments.length - 2 );
                    return m_entity.callMethodRemote( remoteName, context, methodName, methodArgs );
--- a/src/main/java/dan200/computercraft/shared/peripheral/monitor/MonitorPeripheral.java
+++ b/src/main/java/dan200/computercraft/shared/peripheral/monitor/MonitorPeripheral.java
@@ -10,14 +10,15 @@ import dan200.computercraft.api.lua.ILuaContext;
 import dan200.computercraft.api.lua.LuaException;
 import dan200.computercraft.api.peripheral.IComputerAccess;
 import dan200.computercraft.api.peripheral.IPeripheral;
+import dan200.computercraft.core.apis.TermAPI;
 import dan200.computercraft.core.terminal.Terminal;
 import dan200.computercraft.shared.util.Palette;
 import org.apache.commons.lang3.ArrayUtils;

-import java.util.HashMap;
-
 import javax.annotation.Nonnull;

+import static dan200.computercraft.core.apis.ArgumentHelper.*;
+
 public class MonitorPeripheral implements IPeripheral
 {
    private final TileMonitor m_monitor;
@@ -90,23 +91,16 @@ public class MonitorPeripheral implements IPeripheral
            case 1:
            {
                // scroll
-                if( args.length < 1 || !(args[0] instanceof Number) )
-                {
-                    throw new LuaException( "Expected number" );
-                }
+                int value = getInt( args, 0 );
                Terminal terminal = m_monitor.getTerminal().getTerminal();
-                terminal.scroll( ((Number)(args[0])).intValue() );
+                terminal.scroll( value );
                return null;
            }
            case 2:
            {
                // setCursorPos
-                if( args.length < 2 || !(args[0] instanceof Number) || !(args[1] instanceof Number) )
-                {
-                    throw new LuaException( "Expected number, number" );
-                }
-                int x = ((Number)args[0]).intValue() - 1;
-                int y = ((Number)args[1]).intValue() - 1;
+                int x = getInt( args, 0 ) - 1;
+                int y = getInt( args, 1 ) - 1;
                Terminal terminal = m_monitor.getTerminal().getTerminal();
                terminal.setCursorPos( x, y );
                return null;
@@ -114,12 +108,9 @@ public class MonitorPeripheral implements IPeripheral
            case 3:
            {
                // setCursorBlink
-                if( args.length < 1 || !(args[0] instanceof Boolean) )
-                {
-                    throw new LuaException( "Expected boolean" );
-                }
+                boolean blink = getBoolean( args, 0 );
                Terminal terminal = m_monitor.getTerminal().getTerminal();
-                terminal.setCursorBlink( (Boolean) args[ 0 ] );
+                terminal.setCursorBlink( blink );
                return null;
            }
            case 4:
@@ -157,11 +148,7 @@ public class MonitorPeripheral implements IPeripheral
            case 8:
            {
                // setTextScale
-                if( args.length < 1 || !(args[0] instanceof Number) )
-                {
-                    throw new LuaException( "Expected number" );
-                }
-                int scale = (int)(((Number)args[0]).doubleValue() * 2.0);
+                int scale = (int) (getReal( args, 0 ) * 2.0);
                if( scale < 1 || scale > 10 )
                {
                    throw new LuaException( "Expected number in range 0.5-5" );
@@ -173,7 +160,7 @@ public class MonitorPeripheral implements IPeripheral
            case 10:
            {
                // setTextColour/setTextColor
-                int colour = dan200.computercraft.core.apis.TermAPI.parseColour( args );
+                int colour = TermAPI.parseColour( args );
                Terminal terminal = m_monitor.getTerminal().getTerminal();
                terminal.setTextColour( colour );
                return null;
@@ -182,7 +169,7 @@ public class MonitorPeripheral implements IPeripheral
            case 12:
            {
                // setBackgroundColour/setBackgroundColor
-                int colour = dan200.computercraft.core.apis.TermAPI.parseColour( args );
+                int colour = TermAPI.parseColour( args );
                Terminal terminal = m_monitor.getTerminal().getTerminal();
                terminal.setBackgroundColour( colour );
                return null;
@@ -200,26 +187,21 @@ public class MonitorPeripheral implements IPeripheral
            {
                // getTextColour/getTextColor
                Terminal terminal = m_monitor.getTerminal().getTerminal();
-                return dan200.computercraft.core.apis.TermAPI.encodeColour( terminal.getTextColour() );
+                return TermAPI.encodeColour( terminal.getTextColour() );
            }
            case 17:
            case 18:
            {
                // getBackgroundColour/getBackgroundColor
                Terminal terminal = m_monitor.getTerminal().getTerminal();
-                return dan200.computercraft.core.apis.TermAPI.encodeColour( terminal.getBackgroundColour() );
+                return TermAPI.encodeColour( terminal.getBackgroundColour() );
            }
            case 19:
            {
                // blit
-                if( args.length < 3 || !(args[0] instanceof String) || !(args[1] instanceof String) || !(args[2] instanceof String) )
-                {
-                    throw new LuaException( "Expected string, string, string" );
-                }
-
-                String text = (String)args[0];
-                String textColour = (String)args[1];
-                String backgroundColour = (String)args[2];
+                String text = getString( args, 0 );
+                String textColour = getString( args, 1 );
+                String backgroundColour = getString( args, 2 );
                if( textColour.length() != text.length() || backgroundColour.length() != text.length() )
                {
                    throw new LuaException( "Arguments must be the same length" );
@@ -236,26 +218,21 @@ public class MonitorPeripheral implements IPeripheral
                // setPaletteColour/setPaletteColor
                Terminal terminal = m_monitor.getTerminal().getTerminal();

-                if(args.length == 2 && args[0] instanceof Double && args[1] instanceof Double)
+                int colour = 15 - TermAPI.parseColour( args );
+                if( args.length == 2 )
                {
-                    int colour = 15 - dan200.computercraft.core.apis.TermAPI.parseColour( args );
-                    int hex = ((Double)args[1]).intValue();
+                    int hex = getInt( args, 1 );
                    double[] rgb = Palette.decodeRGB8( hex );
-                    dan200.computercraft.core.apis.TermAPI.setColour( terminal, colour, rgb[0], rgb[1], rgb[2] );
-                    return null;
+                    TermAPI.setColour( terminal, colour, rgb[ 0 ], rgb[ 1 ], rgb[ 2 ] );
                }
-
-                if (args.length >= 4 && args[0] instanceof Double && args[1] instanceof Double && args[2] instanceof Double && args[3] instanceof Double)
+                else
                {
-                    int colour = 15 - dan200.computercraft.core.apis.TermAPI.parseColour( args );
-                    double r = (Double)args[1];
-                    double g = (Double)args[2];
-                    double b = (Double)args[3];
-                    dan200.computercraft.core.apis.TermAPI.setColour( terminal, colour, r, g, b );
-                    return null;
+                    double r = getReal( args, 1 );
+                    double g = getReal( args, 2 );
+                    double b = getReal( args, 3 );
+                    TermAPI.setColour( terminal, colour, r, g, b );
                }
-
-                throw new LuaException( "Expected number, number, number, number" );
+                return null;
            }
            case 22:
            case 23:
@@ -264,7 +241,7 @@ public class MonitorPeripheral implements IPeripheral
                Terminal terminal = m_monitor.getTerminal().getTerminal();
                Palette palette = terminal.getPalette();

-                int colour = 15 - dan200.computercraft.core.apis.TermAPI.parseColour( args );
+                int colour = 15 - TermAPI.parseColour( args );

                if( palette != null )
                {
--- a/src/main/java/dan200/computercraft/shared/peripheral/printer/PrinterPeripheral.java
+++ b/src/main/java/dan200/computercraft/shared/peripheral/printer/PrinterPeripheral.java
@@ -14,6 +14,9 @@ import dan200.computercraft.core.terminal.Terminal;

 import javax.annotation.Nonnull;

+import static dan200.computercraft.core.apis.ArgumentHelper.getInt;
+import static dan200.computercraft.core.apis.ArgumentHelper.optString;
+
 public class PrinterPeripheral implements IPeripheral
 {
    private final TilePrinter m_printer;
@@ -70,13 +73,8 @@ public class PrinterPeripheral implements IPeripheral
            case 1:
            {
                // setCursorPos
-                if( args.length != 2 || args[0] == null || !(args[0] instanceof Number) || args[1] == null || !(args[1] instanceof Number) )
-                {
-                    throw new LuaException( "Expected number, number" );
-                }
-
-                int x = ((Number)args[0]).intValue() - 1;
-                int y = ((Number)args[1]).intValue() - 1;
+                int x = getInt( args, 0 ) - 1;
+                int y = getInt( args, 1 ) - 1;
                Terminal page = getCurrentPage();
                page.setCursorPos( x, y );
                return null;
@@ -116,16 +114,7 @@ public class PrinterPeripheral implements IPeripheral
            case 7:
            {
                // setPageTitle
-                String title = "";
-                if( args.length > 0 && args[0] != null )
-                {
-                    if( !(args[0] instanceof String) )
-                    {
-                        throw new LuaException( "Expected string" );
-                    }
-                    title = (String)args[0];
-                }
-
+                String title = optString( args, 0, "" );
                getCurrentPage();
                m_printer.setPageTitle( title );
                return null;
--- a/src/main/java/dan200/computercraft/shared/peripheral/speaker/SpeakerPeripheral.java
+++ b/src/main/java/dan200/computercraft/shared/peripheral/speaker/SpeakerPeripheral.java
@@ -17,9 +17,13 @@ import net.minecraft.util.SoundCategory;
 import net.minecraft.util.SoundEvent;
 import net.minecraft.util.math.BlockPos;
 import net.minecraft.world.World;
+
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;

+import static dan200.computercraft.core.apis.ArgumentHelper.getString;
+import static dan200.computercraft.core.apis.ArgumentHelper.optReal;
+
 public class SpeakerPeripheral implements IPeripheral {
    private TileSpeaker m_speaker;
    private long m_clock;
@@ -127,47 +131,20 @@ public class SpeakerPeripheral implements IPeripheral {
    @Nonnull
    private synchronized Object[] playNote( Object[] arguments, ILuaContext context ) throws LuaException
    {
-        float volume = 1.0f;
-        float pitch = 1.0f;
+        String name = getString(arguments, 0);
+        float volume = (float) optReal( arguments, 1, 1.0 );
+        float pitch = (float) optReal( arguments, 2, 1.0 );

-        // Check if arguments are correct
-        if( arguments.length == 0 ) // Too few args
-        {
-            throw new LuaException( "Expected string, number (optional), number (optional)" );
-        }
-
-        if( !(arguments[0] instanceof String) ) // Arg wrong type
-        {
-            throw new LuaException("Expected string, number (optional), number (optional)");
-        }
-
-        if ( !SoundEvent.REGISTRY.containsKey( new ResourceLocation( "block.note." + arguments[0] ) ) )
+        // Check if sound exists
+        if ( !SoundEvent.REGISTRY.containsKey( new ResourceLocation( "block.note." + name ) ) )
        {
            throw new LuaException("Invalid instrument, \"" + arguments[0] + "\"!");
        }

-        if ( arguments.length > 1 )
-        {
-            if ( arguments[1] != null && !(arguments[1] instanceof Double) )   // Arg wrong type
-            {
-                throw new LuaException( "Expected string, number (optional), number (optional)" );
-            }
-            volume = arguments[1] != null ? ((Double) arguments[1]).floatValue() : 1f;
-        }
-
-        if( arguments.length > 2 )
-        {
-            if( arguments[2] != null && !(arguments[2] instanceof Double) )  // Arg wrong type
-            {
-                throw new LuaException("Expected string, number (optional), number (optional)");
-            }
-            pitch = arguments[2] != null ? ((Double) arguments[2]).floatValue() : 1f;
-        }
-
        // If the resource location for note block notes changes, this method call will need to be updated
        Object[] returnValue = playSound(
            new Object[] {
-                "block.note." + arguments[0],
+                "block.note." + name,
                (double)Math.min( volume, 3f ),
                Math.pow( 2.0f, ( pitch - 12.0f ) / 12.0f)
            }, context, true
@@ -184,42 +161,11 @@ public class SpeakerPeripheral implements IPeripheral {
    @Nonnull
    private synchronized Object[] playSound( Object[] arguments, ILuaContext context, boolean isNote ) throws LuaException
    {
+        String name = getString(arguments, 0);
+        float volume = (float) optReal( arguments, 1, 1.0 );
+        float pitch = (float) optReal( arguments, 2, 1.0 );

-        float volume = 1.0f;
-        float pitch = 1.0f;
-
-        // Check if arguments are correct
-        if( arguments.length == 0 ) // Too few args
-        {
-            throw new LuaException( "Expected string, number (optional), number (optional)" );
-        }
-
-        if( !(arguments[0] instanceof String) ) // Arg wrong type
-        {
-            throw new LuaException( "Expected string, number (optional), number (optional)" );
-        }
-
-        if( arguments.length > 1 )
-        {
-            if( arguments[1] != null && !(arguments[1] instanceof Double) )  // Arg wrong type
-            {
-                throw new LuaException( "Expected string, number (optional), number (optional)" );
-            }
-
-            volume = arguments[1] != null ? ((Double) arguments[1]).floatValue() : 1f;
-
-        }
-
-        if( arguments.length > 2 )
-        {
-            if( arguments[2] != null && !(arguments[2] instanceof Double) )  // Arg wrong type
-            {
-                throw new LuaException( "Expected string, number (optional), number (optional)" );
-            }
-            pitch = arguments[2] != null ? ((Double) arguments[2]).floatValue() : 1f;
-        }
-
-        ResourceLocation resourceName = new ResourceLocation( (String) arguments[0] );
+        ResourceLocation resourceName = new ResourceLocation( name );

        if( m_clock - m_lastPlayTime >= TileSpeaker.MIN_TICKS_BETWEEN_SOUNDS || ( ( m_clock - m_lastPlayTime == 0 ) && ( m_notesThisTick < ComputerCraft.maxNotesPerTick ) && isNote ) )
        {