1
0
mirror of https://github.com/SquidDev-CC/CC-Tweaked synced 2026-05-15 01:52:07 +00:00

Fix handling of integer indexes in CobaltLuaTable

- Add a new LuaTable.get(int) overload to handle getting an integer key
   instead.
 - Update CobaltLuaMachine for more sensible Java->LuaValue conversion
   for looking up keys.
 - Add some tests for our two LuaTable implementations.
 - Randomly reformat two files because spotless is ???

Fixes #2430
This commit is contained in:
Jonathan Coates
2026-05-02 13:46:52 +01:00
parent 5be300dcb8
commit 39a12893e3
7 changed files with 170 additions and 13 deletions
@@ -68,7 +68,9 @@ object ManagedComputers : ILuaMachine.Factory {
val label = os.computerLabel
return when {
id != 1 -> CobaltLuaMachine(environment, bios)
label != null && label[0] != null -> KotlinMachine(environment, label[0] as String)
else -> {
LOGGER.error("Kotlin Lua machine must have a label")
CobaltLuaMachine(environment, bios)
@@ -21,6 +21,19 @@ import static dan200.computercraft.api.lua.LuaValues.*;
* @see ObjectArguments
*/
public interface LuaTable<K, V> extends Map<K, V> {
/**
* Return the value to which a specific integer key is mapped, or {@code null} if there is no mapping for the key.
* <p>
* This should be used when accessing integer keys, as numeric keys within the table are typically normalised to
* doubles.
*
* @param index The key to look up.
* @return The corresponding value, or {@code null} if not present.
*/
default @Nullable Object get(int index) {
return get((double) index);
}
/**
* Compute the length of the array part of this table.
*
@@ -42,7 +55,7 @@ public interface LuaTable<K, V> extends Map<K, V> {
* @since 1.116
*/
default double getDouble(int index) throws LuaException {
Object value = get((double) index);
var value = get(index);
if (!(value instanceof Number number)) throw badTableItem(index, "number", getType(value));
return number.doubleValue();
}
@@ -70,7 +83,7 @@ public interface LuaTable<K, V> extends Map<K, V> {
* @throws LuaException If the value is not an integer.
*/
default long getLong(int index) throws LuaException {
Object value = get((double) index);
var value = get(index);
if (!(value instanceof Number number)) throw badTableItem(index, "number", getType(value));
checkFiniteIndex(index, number.doubleValue());
return number.longValue();
@@ -145,7 +158,7 @@ public interface LuaTable<K, V> extends Map<K, V> {
* @since 1.116
*/
default boolean getBoolean(int index) throws LuaException {
Object value = get((double) index);
var value = get(index);
if (!(value instanceof Boolean bool)) throw badTableItem(index, "boolean", getType(value));
return bool;
}
@@ -173,7 +186,7 @@ public interface LuaTable<K, V> extends Map<K, V> {
* @since 1.116
*/
default String getString(int index) throws LuaException {
Object value = get((double) index);
var value = get(index);
if (!(value instanceof String string)) throw badTableItem(index, "string", getType(value));
return string;
}
@@ -204,7 +217,7 @@ public interface LuaTable<K, V> extends Map<K, V> {
* @since 1.116
*/
default Map<?, ?> getTable(int index) throws LuaException {
Object value = get((double) index);
var value = get(index);
if (!(value instanceof Map<?, ?> table)) throw badTableItem(index, "table", getType(value));
return table;
}
@@ -236,7 +249,7 @@ public interface LuaTable<K, V> extends Map<K, V> {
* @since 1.116
*/
default Optional<Double> optDouble(int index) throws LuaException {
Object value = get((double) index);
var value = get(index);
if (value == null) return Optional.empty();
if (!(value instanceof Number number)) throw badTableItem(index, "number", getType(value));
return Optional.of(number.doubleValue());
@@ -267,7 +280,7 @@ public interface LuaTable<K, V> extends Map<K, V> {
* @since 1.116
*/
default Optional<Long> optLong(int index) throws LuaException {
Object value = get((double) index);
var value = get(index);
if (value == null) return Optional.empty();
if (!(value instanceof Number number)) throw badTableItem(index, "number", getType(value));
checkFiniteIndex(index, number.doubleValue());
@@ -351,7 +364,7 @@ public interface LuaTable<K, V> extends Map<K, V> {
* @since 1.116
*/
default Optional<Boolean> optBoolean(int index) throws LuaException {
Object value = get((double) index);
var value = get(index);
if (value == null) return Optional.empty();
if (!(value instanceof Boolean bool)) throw badTableItem(index, "boolean", getType(value));
return Optional.of(bool);
@@ -381,7 +394,7 @@ public interface LuaTable<K, V> extends Map<K, V> {
* @since 1.116
*/
default Optional<String> optString(int index) throws LuaException {
Object value = get((double) index);
var value = get(index);
if (value == null) return Optional.empty();
if (!(value instanceof String string)) throw badTableItem(index, "string", getType(value));
return Optional.of(string);
@@ -414,7 +427,7 @@ public interface LuaTable<K, V> extends Map<K, V> {
* @since 1.116
*/
default Optional<Map<?, ?>> optTable(int index) throws LuaException {
Object value = get((double) index);
var value = get(index);
if (value == null) return Optional.empty();
if (!(value instanceof Map<?, ?> table)) throw badTableItem(index, "table", getType(value));
return Optional.of(table);
@@ -6,9 +6,11 @@ package dan200.computercraft.core.lua;
import dan200.computercraft.api.lua.LuaException;
import dan200.computercraft.api.lua.LuaValues;
import org.jetbrains.annotations.VisibleForTesting;
import org.jspecify.annotations.Nullable;
import org.squiddev.cobalt.*;
import java.nio.ByteBuffer;
import java.util.*;
import static dan200.computercraft.api.lua.LuaValues.badTableItem;
@@ -37,6 +39,7 @@ class TableImpl implements dan200.computercraft.api.lua.LuaTable<Object, Object>
@Override
public long getLong(int index) throws LuaException {
checkValid();
var value = table.rawget(index);
if (!(value instanceof LuaNumber)) throw LuaValues.badTableItem(index, "number", value.typeName());
if (value instanceof LuaInteger) return value.toInteger();
@@ -58,9 +61,24 @@ class TableImpl implements dan200.computercraft.api.lua.LuaTable<Object, Object>
private LuaValue getImpl(Object o) {
checkValid();
if (o instanceof String s) return table.rawget(s);
if (o instanceof Integer i) return table.rawget(i);
return Constants.NIL;
var value = convertValue(o);
return value == null ? Constants.NIL : table.rawget(value);
}
@VisibleForTesting
static @Nullable LuaValue convertValue(@Nullable Object object) {
if (object == null) return Constants.NIL;
if (object instanceof Boolean bool) return ValueFactory.valueOf(bool);
if (object instanceof Double num) return ValueFactory.valueOf(num);
if (object instanceof String str) return ValueFactory.valueOf(str);
if (object instanceof byte[] b) return ValueFactory.valueOf(Arrays.copyOf(b, b.length));
if (object instanceof ByteBuffer b) {
var bytes = new byte[b.remaining()];
b.get(bytes);
return ValueFactory.valueOf(bytes);
}
return null;
}
@Override
@@ -74,6 +92,12 @@ class TableImpl implements dan200.computercraft.api.lua.LuaTable<Object, Object>
return CobaltLuaMachine.toObject(getImpl(o), null);
}
@Override
public @Nullable Object get(int index) {
checkValid();
return CobaltLuaMachine.toObject(table.rawget(index), null);
}
private Map<Object, Object> getBackingMap() {
checkValid();
if (backingMap != null) return backingMap;
@@ -0,0 +1,35 @@
// SPDX-FileCopyrightText: 2026 The CC: Tweaked Developers
//
// SPDX-License-Identifier: MPL-2.0
package dan200.computercraft.core.lua;
import org.squiddev.cobalt.*;
import java.util.Map;
class CobaltLuaTableTest implements LuaTableContract<TableImpl> {
@Override
public TableImpl create(Map<?, ?> map) {
try {
return new TableImpl(VarargArguments.of(Constants.NONE), convertMap(map));
} catch (LuaError e) {
throw new RuntimeException(e);
}
}
private static LuaValue convert(Object object) throws LuaError {
var value = TableImpl.convertValue(object);
if (value != null) return value;
if (object instanceof Map<?, ?> x) return convertMap(x);
if (object instanceof Integer x) return ValueFactory.valueOf(x);
throw new IllegalArgumentException("Unknown value " + object);
}
private static LuaTable convertMap(Map<?, ?> map) throws LuaError {
var out = new LuaTable();
for (var entry : map.entrySet()) out.rawset(convert(entry.getKey()), convert(entry.getValue()));
return out;
}
}
@@ -0,0 +1,66 @@
// SPDX-FileCopyrightText: 2026 The CC: Tweaked Developers
//
// SPDX-License-Identifier: MPL-2.0
package dan200.computercraft.core.lua;
import dan200.computercraft.api.lua.LuaException;
import dan200.computercraft.api.lua.LuaTable;
import org.junit.jupiter.api.Test;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
/**
* Test Interface defining the behaviour of a {@link LuaTable} implementation.
*
* @param <T> The implementation of {@link LuaTable} we are testing.
*/
public interface LuaTableContract<T extends LuaTable<?, ?>> {
T create(Map<?, ?> map);
default T createList(List<?> list) {
var out = new HashMap<>();
var i = 0;
for (var elem : list) {
var idx = ++i;
// We normalise our index to be a double, to match the behaviour of CobaltLuaMachine.toValue, which converts
// all numbers to doubles.
if (elem != null) out.put((double) idx, elem);
}
return create(out);
}
@Test
default void testLength() {
assertEquals(0, createList(List.of()).length());
assertEquals(1, createList(List.of("a")).length());
assertEquals(2, createList(List.of("a", "a")).length());
assertEquals(1, createList(Arrays.asList("a", null, "a")).length());
}
@Test
default void testGetIntLikeKey() {
assertEquals("a", createList(List.of("a", "b", "c")).get(1));
assertEquals("a", createList(List.of("a", "b", "c")).get(1.0));
// This is a little dubious, but ensures we have consistent behaviour between implementations (doubles are
// the only number) and we don't need to handle double/int normalisation within ObjectLuaTable.
assertNull(createList(List.of("a", "b", "c")).get((Object) 1));
assertNull(createList(List.of("a", "b", "c")).get(1.0f));
assertNull(createList(List.of("a", "b", "c")).get((Object) (short) 1));
}
@Test
default void testGetInt() throws LuaException {
assertEquals("a", createList(List.of("a")).get(1));
assertEquals(true, createList(List.of(true)).getBoolean(1));
assertEquals(12345, createList(List.of(12345.0)).getInt(1));
assertEquals(12345L, createList(List.of(12345.0)).getLong(1));
assertEquals("abc", createList(List.of("abc")).getString(1));
}
}
@@ -0,0 +1,16 @@
// SPDX-FileCopyrightText: 2026 The CC: Tweaked Developers
//
// SPDX-License-Identifier: MPL-2.0
package dan200.computercraft.core.lua;
import dan200.computercraft.api.lua.ObjectLuaTable;
import java.util.Map;
class ObjectLuaTableTest implements LuaTableContract<ObjectLuaTable> {
@Override
public ObjectLuaTable create(Map<?, ?> map) {
return new ObjectLuaTable(map);
}
}
@@ -75,6 +75,7 @@ internal class SideProvider {
private fun getSideImpl(sym: Symbol): Optional<Side> = when (sym.getKind()) {
ElementKind.MODULE -> Optional.empty()
ElementKind.PACKAGE -> {
val pkg = sym.toString()
when {