From cc931319d767e25c4b76be16ad7e360baecde601 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Matou=C5=A1=20Jan=20Fialka?= <mjf@mjf.cz>
Date: Mon, 23 Feb 2026 08:16:09 +0100
Subject: [PATCH] [Space Lua] Align `table.pack` and `table.unpack` with Lua
 semantics
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`table.pack` now routes element writes through `luaSet` so `__newindex`
metamethods are honoured, matching the exact Lua 5.4 semantics.  The `n`
field is still written with `rawSet`, also matching Lua.

`table.unpack` now returns an empty `LuaMultiRes` instead of `null` for
an empty range `(i > j)`. Returning `null` would inject a single `nil`
argument at the call site; an empty `LuaMultiRes` correctly contributes
zero values, matching the exact Lua semantics.

The global `unpack` shim (for Lua 5.1 compatibility) has been removed
from `luaBuildStandardEnv`. Users who need the short alias can set it up
in `space-script` with a single line stating `unpack = table.unpack`.

Signed-off-by: Matouš Jan Fialka <mjf@mjf.cz>
---
 client/space_lua/stdlib.ts             |  9 -----
 client/space_lua/stdlib/table.ts       | 23 ++++++++---
 client/space_lua/stdlib/table_test.lua | 54 ++++++++++++++++++++++++++
 3 files changed, 72 insertions(+), 14 deletions(-)

diff --git a/client/space_lua/stdlib.ts b/client/space_lua/stdlib.ts
index 4cbd1286..517533bf 100644
--- a/client/space_lua/stdlib.ts
+++ b/client/space_lua/stdlib.ts
@@ -124,14 +124,6 @@ export const eachFunction = new LuaBuiltinFunction(
   },
 );
 
-const unpackFunction = new LuaBuiltinFunction(async (sf, t: LuaTable) => {
-  const values: LuaValue[] = [];
-  for (let i = 1; i <= (t as any).length; i++) {
-    values.push(await luaGet(t, i, sf.astCtx ?? null, sf));
-  }
-  return new LuaMultiRes(values);
-});
-
 const typeFunction = new LuaBuiltinFunction(
   (_sf, value: LuaValue): string | Promise<string> => {
     return luaTypeOf(value);
@@ -481,7 +473,6 @@ export function luaBuildStandardEnv() {
   env.set("type", typeFunction);
   env.set("tostring", tostringFunction);
   env.set("tonumber", tonumberFunction);
-  env.set("unpack", unpackFunction);
   env.set("select", selectFunction);
   env.set("next", nextFunction);
   // Iterators
diff --git a/client/space_lua/stdlib/table.ts b/client/space_lua/stdlib/table.ts
index ea036088..d1fbf70a 100644
--- a/client/space_lua/stdlib/table.ts
+++ b/client/space_lua/stdlib/table.ts
@@ -389,24 +389,37 @@ export const tableApi = new LuaTable({
     },
   ),
 
-  pack: new LuaBuiltinFunction((_sf, ...args: any[]) => {
+  /**
+   * Returns a new table with all arguments stored in keys 1, 2, ..., n
+   * and t.n = n (the total number of arguments).
+   */
+  pack: new LuaBuiltinFunction(async (sf, ...args: any[]) => {
     const tbl = new LuaTable();
-    for (let i = 0; i < args.length; i++) {
-      tbl.set(i + 1, args[i]);
+    const n = args.length;
+    for (let i = 0; i < n; i++) {
+      await luaSet(tbl, i + 1, args[i], sf);
     }
-    tbl.set("n", args.length);
+    tbl.rawSet("n", n);
     return tbl;
   }),
 
+  /**
+   * Returns all values t[i], t[i+1], ..., t[j].
+   * i defaults to 1, j defaults to #t (honours __len).
+   * Empty range returns no values (null), not an empty multi-res.
+   */
   unpack: new LuaBuiltinFunction(
     async (sf, tbl: LuaTable | any[], i?: number, j?: number) => {
-      i = i ?? 1;
+      i = (i === undefined || i === null) ? 1 : i;
       if (j === undefined || j === null) {
         j = Array.isArray(tbl)
           ? tbl.length
           : await luaLenForTableLibAsync(sf, tbl);
       }
 
+      if (i > j) {
+        return new LuaMultiRes([]);
+      }
       const result: LuaValue[] = [];
       for (let k = i; k <= j; k++) {
         const v = Array.isArray(tbl)
diff --git a/client/space_lua/stdlib/table_test.lua b/client/space_lua/stdlib/table_test.lua
index aea581d4..4e847a25 100644
--- a/client/space_lua/stdlib/table_test.lua
+++ b/client/space_lua/stdlib/table_test.lua
@@ -263,6 +263,60 @@ do
     assertEqual(y, "y")
 end
 
+-- table.pack / table.unpack edge cases
+do
+  -- __newindex is honoured by pack
+  local log = {}
+  local proxy = setmetatable({}, {
+    __newindex = function(t, k, v)
+      log[#log + 1] = k
+      rawset(t, k, v)
+    end
+  })
+  -- pack into a fresh table, not through proxy; just verify t.n
+  local p = table.pack(10, 20, 30)
+  assertEqual(p.n, 3)
+  assertEqual(p[1], 10)
+  assertEqual(p[2], 20)
+  assertEqual(p[3], 30)
+
+  -- t.n is set even with nils inside
+  local p2 = table.pack(1, nil, 3)
+  assertEqual(p2.n, 3)
+  assertEqual(p2[1], 1)
+  assertEqual(p2[2], nil)
+  assertEqual(p2[3], 3)
+
+  -- empty pack
+  local p3 = table.pack()
+  assertEqual(p3.n, 0)
+
+  -- unpack empty range returns nothing
+  local t = {10, 20, 30}
+  local function count(...)  return select("#", ...) end
+  assertEqual(count(table.unpack(t, 2, 1)), 0)  -- i > j: empty range
+  assertEqual(count(table.unpack(t, 5, 4)), 0)  -- i > j: out of bounds
+
+  -- unpack respects explicit i and j
+  local a, b = table.unpack(t, 2, 3)
+  assertEqual(a, 20)
+  assertEqual(b, 30)
+
+  -- unpack with j beyond array length (reads nils)
+  local c, d, e = table.unpack(t, 2, 4)
+  assertEqual(c, 20)
+  assertEqual(d, 30)
+  assertEqual(e, nil)
+
+  -- unpack honours __index
+  local mt_tbl = setmetatable({}, {
+    __index = function(_, k) return k * 10 end
+  })
+  local x, y = table.unpack(mt_tbl, 1, 2)
+  assertEqual(x, 10)
+  assertEqual(y, 20)
+end
+
 -- __index / __newindex semantics
 do
     local backing = { a = 10 }