Rework ScriptInterface EnumeratePropertyNames to support non-enumerable properties.

This lets it support the JSClass syntax, introduced for components in
5fb88172a2/D2492, which required a workaround before.
As cleanup, remove the prefix logic which was too specific to be in
ScriptInterface, and use an upstream-compatible API that also does more
of what we want.

Based on a patch by: elexis
Differential Revision: https://code.wildfiregames.com/D2644
This was SVN commit r23771.

binaries/data/mods/public/simulation/components/AutoBuildable.js

@@ -2,22 +2,11 @@ class AutoBuildable
 {
 	Init()
 	{
-		this.rate = ApplyValueModificationsToEntity("AutoBuildable/Rate", +this.template.Rate , this.entity);
+		this.rate = ApplyValueModificationsToEntity("AutoBuildable/Rate", +this.template.Rate, this.entity);
 		if (this.rate)
 			this.StartTimer();
 	}
 
-	get Schema()
-	{
-		return  "<a:help>Defines whether the entity can be built by itself.</a:help>" +
-			"<a:example>" +
-				"<Rate>1.0</Rate>" +
-			"</a:example>" +
-			"<element name='Rate' a:help='The rate at which the building autobuilds.'>" +
-				"<ref name='nonNegativeDecimal'/>" +
-			"</element>";
-	}
-
 	/**
 	 * @return {number} - The rate with technologies and aura modification applied.
 	 */
@@ -28,7 +17,7 @@ class AutoBuildable
 
 	UpdateRate()
 	{
-		this.rate = ApplyValueModificationsToEntity("AutoBuildable/Rate", +this.template.Rate , this.entity);
+		this.rate = ApplyValueModificationsToEntity("AutoBuildable/Rate", +this.template.Rate, this.entity);
 
 		if (this.rate)
 			this.StartTimer();
@@ -78,22 +67,31 @@ class AutoBuildable
 
 		cmpFoundation.Build(this.entity, this.rate);
 	}
+
+	OnValueModification(msg)
+	{
+		if (msg.component != "AutoBuildable")
+			return;
+
+		this.UpdateRate();
+	}
+
+	OnOwnershipChanged(msg)
+	{
+		if (msg.to == INVALID_PLAYER)
+			return;
+
+		this.UpdateRate();
+	}
 }
 
-AutoBuildable.prototype.OnValueModification = function(msg)
-{
-	if (msg.component != "AutoBuildable")
-		return;
-
-	this.UpdateRate();
-};
-
-AutoBuildable.prototype.OnOwnershipChanged = function(msg)
-{
-	if (msg.to == INVALID_PLAYER)
-		return;
-
-	this.UpdateRate();
-}
+AutoBuildable.prototype.Schema =
+	"<a:help>Defines whether the entity can be built by itself.</a:help>" +
+	"<a:example>" +
+		"<Rate>1.0</Rate>" +
+	"</a:example>" +
+	"<element name='Rate' a:help='The rate at which the building autobuilds.'>" +
+		"<ref name='nonNegativeDecimal'/>" +
+	"</element>";
 
 Engine.RegisterComponentType(IID_AutoBuildable, "AutoBuildable", AutoBuildable);

source/lobby/XmppClient.cpp

@@ -373,7 +373,7 @@ void XmppClient::SendIqGameReport(const ScriptInterface& scriptInterface, JS::Ha
 
 	// Iterate through all the properties reported and add them to the stanza.
 	std::vector<std::string> properties;
-	scriptInterface.EnumeratePropertyNamesWithPrefix(data, "", properties);
+	scriptInterface.EnumeratePropertyNames(data, true, properties);
 	for (const std::string& p : properties)
 	{
 		std::wstring value;
@@ -409,7 +409,7 @@ void XmppClient::SendIqRegisterGame(const ScriptInterface& scriptInterface, JS::
 
 	// Iterate through all the properties reported and add them to the stanza.
 	std::vector<std::string> properties;
-	scriptInterface.EnumeratePropertyNamesWithPrefix(data, "", properties);
+	scriptInterface.EnumeratePropertyNames(data, true, properties);
 	for (const std::string& p : properties)
 	{
 		std::wstring value;

source/scriptinterface/ScriptInterface.cpp

@@ -1,4 +1,4 @@
-/* Copyright (C) 2019 Wildfire Games.
+/* Copyright (C) 2020 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -761,24 +761,23 @@ bool ScriptInterface::HasProperty(JS::HandleValue obj, const char* name) const
 	return found;
 }
 
-bool ScriptInterface::EnumeratePropertyNamesWithPrefix(JS::HandleValue objVal, const char* prefix, std::vector<std::string>& out) const
+bool ScriptInterface::EnumeratePropertyNames(JS::HandleValue objVal, bool enumerableOnly, std::vector<std::string>& out) const
 {
 	JSAutoRequest rq(m->m_cx);
 
 	if (!objVal.isObjectOrNull())
 	{
-		LOGERROR("EnumeratePropertyNamesWithPrefix expected object type!");
+		LOGERROR("EnumeratePropertyNames expected object type!");
 		return false;
 	}
 
-	if (objVal.isNull())
-		return true; // reached the end of the prototype chain
-
 	JS::RootedObject obj(m->m_cx, &objVal.toObject());
-	JS::Rooted<JS::IdVector> props(m->m_cx, JS::IdVector(m->m_cx));
-	if (!JS_Enumerate(m->m_cx, obj, &props))
+	JS::AutoIdVector props(m->m_cx);
+	// This recurses up the prototype chain on its own.
+	if (!js::GetPropertyKeys(m->m_cx, obj, enumerableOnly? 0 : JSITER_HIDDEN, &props))
 		return false;
 
+	out.reserve(out.size() + props.length());
 	for (size_t i = 0; i < props.length(); ++i)
 	{
 		JS::RootedId id(m->m_cx, props[i]);
@@ -786,43 +785,16 @@ bool ScriptInterface::EnumeratePropertyNamesWithPrefix(JS::HandleValue objVal, c
 		if (!JS_IdToValue(m->m_cx, id, &val))
 			return false;
 
+		// Ignore integer properties for now.
+		// TODO: is this actually a thing in ECMAScript 6?
 		if (!val.isString())
-			continue; // ignore integer properties
+			continue;
 
-		JS::RootedString name(m->m_cx, val.toString());
-		size_t len = strlen(prefix)+1;
-		std::vector<char> buf(len);
-		size_t prefixLen = strlen(prefix) * sizeof(char);
-		JS_EncodeStringToBuffer(m->m_cx, name, &buf[0], prefixLen);
-		buf[len-1]= '\0';
-		if (0 == strcmp(&buf[0], prefix))
-		{
-			if (JS_StringHasLatin1Chars(name))
-			{
-				size_t length;
-				JS::AutoCheckCannotGC nogc;
-				const JS::Latin1Char* chars = JS_GetLatin1StringCharsAndLength(m->m_cx, nogc, name, &length);
-				if (chars)
-					out.push_back(std::string(chars, chars+length));
-			}
-			else
-			{
-				size_t length;
-				JS::AutoCheckCannotGC nogc;
-				const char16_t* chars = JS_GetTwoByteStringCharsAndLength(m->m_cx, nogc, name, &length);
-				if (chars)
-					out.push_back(std::string(chars, chars+length));
-			}
-		}
-	}
-
-	// Recurse up the prototype chain
-	JS::RootedObject prototype(m->m_cx);
-	if (JS_GetPrototype(m->m_cx, obj, &prototype))
-	{
-		JS::RootedValue prototypeVal(m->m_cx, JS::ObjectOrNullValue(prototype));
-		if (!EnumeratePropertyNamesWithPrefix(prototypeVal, prefix, out))
+		std::string propName;
+		if (!FromJSVal(m->m_cx, val, propName))
 			return false;
+
+		out.emplace_back(std::move(propName));
 	}
 
 	return true;

source/scriptinterface/ScriptInterface.h

@@ -212,7 +212,15 @@ public:
 	 */
 	bool HasProperty(JS::HandleValue obj, const char* name) const;
 
-	bool EnumeratePropertyNamesWithPrefix(JS::HandleValue objVal, const char* prefix, std::vector<std::string>& out) const;
+	/**
+	 * Returns all properties of the object, both own properties and inherited.
+	 * This is essentially equivalent to calling Object.getOwnPropertyNames()
+	 * and recursing up the prototype chain.
+	 * NB: this does not return properties with symbol or numeric keys, as that would
+	 * require a variant in the vector, and it's not useful for now.
+	 * @param enumerableOnly - only return enumerable properties.
+	 */
+	bool EnumeratePropertyNames(JS::HandleValue objVal, bool enumerableOnly, std::vector<std::string>& out) const;
 
 	bool SetPrototype(JS::HandleValue obj, JS::HandleValue proto);
 

source/simulation2/system/ComponentManager.cpp

@@ -264,19 +264,23 @@ void CComponentManager::Script_RegisterComponentType_Common(ScriptInterface::CxP
 	// Find all the ctor prototype's On* methods, and subscribe to the appropriate messages:
 	std::vector<std::string> methods;
 
-	if (!componentManager->m_ScriptInterface.EnumeratePropertyNamesWithPrefix(protoVal, "On", methods))
+	if (!componentManager->m_ScriptInterface.EnumeratePropertyNames(protoVal, false, methods))
 	{
-		componentManager->m_ScriptInterface.ReportError("Failed to enumerate 'On' messages");
+		componentManager->m_ScriptInterface.ReportError("Failed to enumerate component properties.");
 		return;
 	}
 
 	for (std::vector<std::string>::const_iterator it = methods.begin(); it != methods.end(); ++it)
 	{
+		// TODO C++17: string_view
+		if (strncmp((it->c_str()), "On", 2) != 0)
+			continue;
+
 		std::string name = (*it).substr(2); // strip the "On" prefix
 
 		// Handle "OnGlobalFoo" functions specially
 		bool isGlobal = false;
-		if (name.substr(0, 6) == "Global")
+		if (strncmp(name.c_str(), "Global", 6) == 0)
 		{
 			isGlobal = true;
 			name = name.substr(6);