Sort lua components during serialization

[Gliese852]

In the serialization of the Lua objects there is a system of "one definition", when all subsequent mention of the object only refer to it. It is vital that the full definition meets before the rest of the refs to the object, otherwise an error occurs.

The same objects can now meet in different lua components, and this means that we must ensure the order of processing procedure during saving and loading.

fixes #6064

[Gliese852]

So I looked again and now I saw some promise that the values inside the json object are sorted:

pioneer/contrib/json/json.hpp

Lines 11543 to 11546 in 89d503a

	- Internally, name/value pairs are stored in lexicographical order of the
	names. Objects will also be serialized (see @ref dump) in this order.
	For instance, `{"b": 1, "a": 2}` and `{"a": 2, "b": 1}` will be stored
	and serialized as `{"a": 2, "b": 1}`.

I wonder, can we trust this and not even use std::set? On the one hand, this is put into the docs, on the other hand, there is the word "Internally".

[markdascher]

I like the explicit sort, just in case that library sorts things slightly differently than std::set

[sturnclaw]

So I looked again and now I saw some promise that the values inside the json object are sorted:

pioneer/contrib/json/json.hpp

Lines 11543 to 11546 in 89d503a

	- Internally, name/value pairs are stored in lexicographical order of the
	names. Objects will also be serialized (see @ref dump) in this order.
	For instance, `{"b": 1, "a": 2}` and `{"a": 2, "b": 1}` will be stored
	and serialized as `{"a": 2, "b": 1}`.

I wonder, can we trust this and not even use std::set? On the one hand, this is put into the docs, on the other hand, there is the word "Internally".

There is a strong guarantee that the contents of a JSON object are lexicographically sorted when written to and read from disk, and that sort order is based on the container type used in the basic_json type definition. By default (which we use) all sorting of JSON keys uses std::less<std::string>.

The problem at hand is that we are potentially deserializing keys in a different order than we are serializing them, due to the instability of Lua table traversal. This requires a sorted order for keys when serializing, but as long as that sort is performed using std::less we can deserialize directly without an additional sort step.

Note that the data referred to by std::string_view key must be assumed to go out-of-scope as soon as the underlying string key is popped from the Lua stack. Lua does not provide any guarantees that string data is valid unless the string value is present on the stack. Thus, the std::set<...> componentNames should be std::set<std::string>.

[Gliese852]

@sturnclaw

Note that the data referred to by std::string_view key must be assumed to go out-of-scope as soon as the underlying string key is popped from the Lua stack. Lua does not provide any guarantees that string data is valid unless the string value is present on the stack. Thus, the std::set<...> componentNames should be std::set<std::string>.

Indeed, very specifically written, thanks! But still, maybe we can count on the fact that these strings in a sense are still on the stack, because the table containing them is on the stack? Not that I am against std::string, I just want to know your opinion on this.

[sturnclaw]

But still, maybe we can count on the fact that these strings in a sense are still on the stack, because the table containing them is on the stack?

That would be relying on undocumented behavior, and so I'd somewhat prefer to avoid doing so in the general case.

In this particular case however, because strings are uniquely interned in the global Lua string table (not actually a LUA_TTABLE) and IIRC have pointer identity, as long as the string value in question is considered reachable by the Lua GC it's probably safe to do so.

Regardless of std::string or std::string_view, I'd recommend pushing all component strings into a single std::vector and then performing a sort using the std::less comparator.