pioneerspacesim/pioneer — pull request #2543 by johnbartholomew (2013-11-11 15:17:19+00:00)
Title: Cache Lua SystemPath objects to deduplicate them

This addresses a bug that can be most easily illustrated in the Lua console. Start a new game, open the Lua console and run the following code:

SystemPath = import("SystemPath")
a = SystemPath.New(0,0,0, 0)
b = SystemPath.New(0,0,0, 0)
print(a == b) -- should print 'true'
t = {}
t[a] = 1
t[b] = 2
for k, v in pairs(t) do print(k, ' => ', v); end

The expected (and obviously 'correct') result is that the table `t` should contain a single entry. Since `a == b`, the assignment of `t[b] = 2` should overwrite the initial assignment of `t[a] = 1`. Unfortunately, the observed result is that the table contains two entries with apparently the same key.

This is because when a userdata value is used as a key in a Lua table, only its address is actually used as the key. For SystemPath objects, we create a new userdata value each time a SystemPath is passed to Lua. On the C++ side, SystemPath is a value type -- it has no meaningful identity beyond its actual stored value (the sector, system index and body index). On the Lua side, SystemPath is an object type -- it has an identity independent of its content, and this identity is observable when you try to use the object as a table key.

I think this bug is pretty terrible, because SystemPath is a very useful table key. The most obvious way for Lua to store per-sector, per-system or per-system-body data is to put that data in a table indexed by SystemPath.

I see three possible ways of coping with this:

1. Never use SystemPath as a table key. This leaves the question of what method we should recommend for people who want this functionality. There are several options I can see, none of which I like.
2. Modify Lua itself to make this work. Probably the way to do this would be to add a new core Lua type for "immutable data" which would act like a value-type equivalent of userdata, or a string that supports attaching a metatable. Values of this form could be used for SystemPath, and also mathematical types (vectors, matrices, quaternions) if we want them. I don't know enough about Lua's internals to know how difficult this would be.
3. Cache Lua SystemPath objects and deduplicate them in `LuaObject::PushToLua`. This is the simplest option and the option I've implemented for this PR. The disadvantage of this method is its potential performance impact, but we can't really know what that is until we measure it, so I'm just going to assume that the impact is acceptable until we demonstrate otherwise.