Index: ps/trunk/binaries/data/mods/public/gui/gamesetup/Pages/GameSetupPage/GameSettings/PerPlayer/Dropdowns/PlayerColor.js =================================================================== --- ps/trunk/binaries/data/mods/public/gui/gamesetup/Pages/GameSetupPage/GameSettings/PerPlayer/Dropdowns/PlayerColor.js +++ ps/trunk/binaries/data/mods/public/gui/gamesetup/Pages/GameSetupPage/GameSettings/PerPlayer/Dropdowns/PlayerColor.js @@ -48,7 +48,7 @@ smallestDistance = distance; } } - this.values.push(clone(closestColor)); + this.values.push(closestColor); } this.dropdown.list = this.values.map(color => coloredText(this.ColorIcon, rgbToGuiColor(color))); Index: ps/trunk/source/simulation2/serialization/BinarySerializer.h =================================================================== --- ps/trunk/source/simulation2/serialization/BinarySerializer.h +++ ps/trunk/source/simulation2/serialization/BinarySerializer.h @@ -22,6 +22,9 @@ #include "lib/byte_order.h" +#include "js/AllocPolicy.h" +#include "js/GCHashTable.h" + #include #include #include @@ -83,16 +86,20 @@ { public: CBinarySerializerScriptImpl(const ScriptInterface& scriptInterface, ISerializer& serializer); + ~CBinarySerializerScriptImpl(); void ScriptString(const char* name, JS::HandleString string); void HandleScriptVal(JS::HandleValue val); private: + static void Trace(JSTracer* trc, void* data); + const ScriptInterface& m_ScriptInterface; ISerializer& m_Serializer; - JS::PersistentRootedSymbol m_ScriptBackrefSymbol; - i32 m_ScriptBackrefsNext; - i32 GetScriptBackrefTag(JS::HandleObject obj); + using ObjectTagMap = JS::GCHashMap, u32, js::PointerHasher, js::SystemAllocPolicy>; + ObjectTagMap m_ScriptBackrefTags; + u32 m_ScriptBackrefsNext; + u32 GetScriptBackrefTag(JS::HandleObject obj); }; /** Index: ps/trunk/source/simulation2/serialization/BinarySerializer.cpp =================================================================== --- ps/trunk/source/simulation2/serialization/BinarySerializer.cpp +++ ps/trunk/source/simulation2/serialization/BinarySerializer.cpp @@ -59,7 +59,12 @@ { ScriptRequest rq(m_ScriptInterface); - m_ScriptBackrefSymbol.init(rq.cx, JS::NewSymbol(rq.cx, nullptr)); + JS_AddExtraGCRootsTracer(m_ScriptInterface.GetGeneralJSContext(), Trace, this); +} + +CBinarySerializerScriptImpl::~CBinarySerializerScriptImpl() +{ + JS_RemoveExtraGCRootsTracer(m_ScriptInterface.GetGeneralJSContext(), Trace, this); } void CBinarySerializerScriptImpl::HandleScriptVal(JS::HandleValue val) @@ -89,11 +94,11 @@ JS::RootedObject obj(rq.cx, &val.toObject()); // If we've already serialized this object, just output a reference to it - i32 tag = GetScriptBackrefTag(obj); - if (tag != -1) + u32 tag = GetScriptBackrefTag(obj); + if (tag != 0) { m_Serializer.NumberU8_Unbounded("type", SCRIPT_TYPE_BACKREF); - m_Serializer.NumberI32("tag", tag, 0, JSVAL_INT_MAX); + m_Serializer.NumberU32("tag", tag, 0, JSVAL_INT_MAX); break; } @@ -437,7 +442,13 @@ } } -i32 CBinarySerializerScriptImpl::GetScriptBackrefTag(JS::HandleObject obj) +void CBinarySerializerScriptImpl::Trace(JSTracer *trc, void *data) +{ + CBinarySerializerScriptImpl* serializer = static_cast(data); + serializer->m_ScriptBackrefTags.trace(trc); +} + +u32 CBinarySerializerScriptImpl::GetScriptBackrefTag(JS::HandleObject obj) { // To support non-tree structures (e.g. "var x = []; var y = [x, x];"), we need a way // to indicate multiple references to one object(/array). So every time we serialize a @@ -449,33 +460,13 @@ ScriptRequest rq(m_ScriptInterface); - JS::RootedValue symbolValue(rq.cx, JS::SymbolValue(m_ScriptBackrefSymbol)); - JS::RootedId symbolId(rq.cx); - ENSURE(JS_ValueToId(rq.cx, symbolValue, &symbolId)); - - JS::RootedValue tagValue(rq.cx); - - // If it was already there, return the tag - bool tagFound; - ENSURE(JS_HasPropertyById(rq.cx, obj, symbolId, &tagFound)); - if (tagFound) - { - ENSURE(JS_GetPropertyById(rq.cx, obj, symbolId, &tagValue)); - ENSURE(tagValue.isInt32()); - return tagValue.toInt32(); - } - - tagValue = JS::Int32Value(m_ScriptBackrefsNext); - // TODO: this fails if the object cannot be written to. - // This means we could end up in an infinite loop... - if (!JS_DefinePropertyById(rq.cx, obj, symbolId, tagValue, JSPROP_READONLY)) + ObjectTagMap::Ptr ptr = m_ScriptBackrefTags.lookup(JS::Heap(obj.get())); + if (!ptr.found()) { - // For now just warn, this should be user-fixable and may not actually error out. - JS::RootedValue objVal(rq.cx, JS::ObjectValue(*obj.get())); - LOGWARNING("Serialization symbol cannot be written on object %s", m_ScriptInterface.ToString(&objVal)); + ENSURE(m_ScriptBackrefTags.put(JS::Heap(obj.get()), ++m_ScriptBackrefsNext)); + // Return 0 to mean "you have to serialize this object"; + return 0; } - - ++m_ScriptBackrefsNext; - // Return a non-tag number so callers know they need to serialize the object - return -1; + else + return ptr->value(); } Index: ps/trunk/source/simulation2/serialization/StdDeserializer.cpp =================================================================== --- ps/trunk/source/simulation2/serialization/StdDeserializer.cpp +++ ps/trunk/source/simulation2/serialization/StdDeserializer.cpp @@ -31,6 +31,8 @@ m_ScriptInterface(scriptInterface), m_Stream(stream) { JS_AddExtraGCRootsTracer(m_ScriptInterface.GetGeneralJSContext(), CStdDeserializer::Trace, this); + // Insert a dummy object in front, as valid tags start at 1. + m_ScriptBackrefs.emplace_back(nullptr); } CStdDeserializer::~CStdDeserializer() Index: ps/trunk/source/simulation2/tests/test_Serializer.h =================================================================== --- ps/trunk/source/simulation2/tests/test_Serializer.h +++ ps/trunk/source/simulation2/tests/test_Serializer.h @@ -719,13 +719,13 @@ ); helper_script_roundtrip("Nested maps using backrefs", - "var a = new Map(); var b = new Map(); a.set(1, b); a.set(2, b); a", + "var a = new Map(); var b = new Map(); a.set(1, b); a.set(2, b); a.set(3, b); a", /* expected: */ "({})", /* expected stream: */ - 25, + 35, "\x0f" // SCRIPT_TYPE_MAP - "\x02\0\0\0" // size + "\x03\0\0\0" // size "\x05" // SCRIPT_TYPE_INT "\x01\0\0\0" // 1 @@ -735,7 +735,12 @@ "\x05" // SCRIPT_TYPE_INT "\x02\0\0\0" // 2 "\x08" // SCRIPT_TYPE_BACKREF - "\x01\0\0\0" // ref. to object #1, i.e. "b", with #0 being "a" + "\x02\0\0\0" // ref. to object #2, i.e. "b", with #1 being "a" + + "\x05" // SCRIPT_TYPE_INT + "\x03\0\0\0" // 3 + "\x08" // SCRIPT_TYPE_BACKREF + "\x02\0\0\0" // ref. to object #2, i.e. "b", with #1 being "a" ); } @@ -757,7 +762,7 @@ "\x01\0\0\0" // num props "\x01\x03\0\0\0" "bar" // "bar" "\x08" // SCRIPT_TYPE_BACKREF - "\x01\0\0\0" // ref to object #1, i.e. "b", with #0 being "a" + "\x02\0\0\0" // ref to object #2, i.e. "b", with #1 being "a" ); }