Minecraft Version

1.19.x

Version

1.103.1

Details

When trying to serialize an empty table that had a metatable I found that it would refuse to use the __pairs metamethod. I suspected something was wrong with serialize after seeing that an enhanced for loop did print the keys and values. Sure enough, I found this:
https://github.com/cc-tweaked/CC-Tweaked/blob/mc-1.19.x/projects/core/src/main/resources/data/computercraft/lua/rom/apis/textutils.lua#L309

I think the right approach here would be to call pairs and then use the resultant function, instead of a raw next. Checking the blame for this shows that this has been a problem since CC went open source, however, so I'm not sure if this is even worth fixing for parity reasons.

Depending on the implementation, a short term workaround to this issue is to put anything into the empty table that has the metatable applied to it. Ex: setmetatable({{}=false}, mt). Using a table as a key makes it so that it overwrites nothing in a potential parent __index.

Could change that line to just

if not getmetatable(t).__pairs and next(t) == nil then

Though I think

if pairs(t)() == nil then

should work too.

Not sure which would be better.

So ideally textutils.unserialise(textutils.serialise(xyz)) should always return an equivalent value. However, this obviously doesn't happen in the presence of metatables, as those are just dropped.

There's an argument here that trying to serialise a table with a metatable should be an error (much like serialising a recursive table is), but I suspect that's too much of a breaking change. Instead, I think there's two options here:

• Ignore metatmethods entirely (so use next and rawlen/rawget instead of pairs and ipairs).
• Use pairs to detect if the table is empty.

I have a mild preference for the former as it feels more disciplined, but also open to being convinced otherwise :).

We could add a flag for it, but I think the implementation then just gets too messy.

There's an argument here that trying to serialize a table with a metatable should be an error (much like serialising a recursive table is)

I'm not sure that's a very good argument. So long as there's not recursive tables involved, serialize should represent the table as it gets represented by iterating over it using pairs. It was super confusing to me, end user, when my table was not represented in the way that I intended it to be. Outright blocking the use of metatables feels a bit too heavy-handed. Therefore, I'm in favor of option 2, or option 1 with a flag to allow for option 2.

To add on to the flag concept, may I suggest that it accept either a boolean or a number value? If it's a boolean (or nil), then allow/disallow metatables, but error if recursion. If it's a number, allow recursion up to the provided numbers depth. I'd hate to have a parameter be tacked on to serialize (even if it is optional) that doesn't provide a richer function beyond this issue.

If someone is using metatables like this then they probably know enough to write their own serialiser. It's extra work but they would have more control over how exactly it serialises and thus might be able to deserialise it with the meta table back as a meta table.

I feel like the built-in APIs should cover most use cases while staying simple to use.

@hugeblank would you be able to explain what you're using __pairs for and what you're trying to serialise? Might be useful to understand the full picture.

I think for me there's an expectation that once involve metamethods, the behaviour of things gets a lot more fuzzy. CC typically takes the conservative option here to just copy the actual data in the table (see also os.queueEvent, modem.transmit), which is why I think it makes sense to do the same in textutils.serialize.

I'd hate to have a parameter be tacked on to serialize that doesn't provide a richer function beyond this issue.

textutils.serialise already accepts a table of additional options, so it'd be added to that instead.

I was trying to make a table that saved itself to a file whenever data was modified on it. I was hoping to be able to pass this table around between other programs, programs which might serialize that table. I ended up dropping it because I ran into this problem (among others), but I might pick it back up after having a few more thoughts about its implementation due to this discussion. I'm pretty sure I could get away with just using __newindex, bypassing any shenanigans I'd have to do with __pairs.

I'm pretty sure I could get away with just using __newindex,

This will only work for setting new values in the table, but __newindex and __index are called iff the index doesn't already exist.

ie:

local tbl = setmetatable({}, {
  __newindex = function(self, idx, v)
    print("Newindex!")
    rawset(self, idx, v)
  end
})

tbl.value = 32 -- "Newindex!"
tbl.value = 64 -- nothing is printed, newindex is not ran!

You would need a proxy table, at which point you would need to override __pairs or use some other serialization system. Preferrably the latter given discussion here, or just override __tostring to call serialize on the data table.

Or assumedly you could just serialise the backing table directly? So something like:

local function saving_table(path, backing)
  return setmetatable({}, {
    __index = backing,
    __newindex = function(self, key, value)
      backing[key] = value
      local contents = textutils.serialize(backing)
      io.open(path, "w"):write(contents):close()
    end
  })
end

I totally forgot that he wanted it to be written on every change, yes that would work much better!

Or assumedly you could just serialise the backing table directly?

Exactly what I was thinking. thanks!