Fix script caching of inherited templates.

This was SVN commit r7773.

source/simulation2/components/CCmpTemplateManager.cpp

@@ -48,6 +48,8 @@ public:
 
 	virtual void Init(const CSimContext& context, const CParamNode& UNUSED(paramNode))
 	{
+		m_DisableValidation = false;
+
 		m_Validator.LoadGrammar(context.GetComponentManager().GenerateSchema());
 		// TODO: handle errors loading the grammar here?
 		// TODO: support hotloading changes to the grammar
@@ -114,6 +116,11 @@ public:
 		}
 	}
 
+	virtual void DisableValidation()
+	{
+		m_DisableValidation = true;
+	}
+
 	virtual const CParamNode* LoadTemplate(entity_id_t ent, const std::string& templateName, int playerID);
 
 	virtual const CParamNode* GetTemplate(std::string templateName);
@@ -128,6 +135,9 @@ private:
 	// Entity template XML validator
 	RelaxNGValidator m_Validator;
 
+	// Disable validation, for test cases
+	bool m_DisableValidation;
+
 	// Map from template name (XML filename or special |-separated string) to the most recently
 	// loaded non-broken template data. This includes files that will fail schema validation.
 	// (Failed loads won't remove existing entries under the same name, so we behave more nicely
@@ -186,21 +196,23 @@ const CParamNode* CCmpTemplateManager::GetTemplate(std::string templateName)
 		return NULL;
 	}
 
-	// Compute validity, if it's not computed before
-	if (m_TemplateSchemaValidity.find(templateName) == m_TemplateSchemaValidity.end())
-		m_TemplateSchemaValidity[templateName] = m_Validator.Validate(CStrW(templateName), m_TemplateFileData[templateName].ToXML());
-	// Refuse to return invalid templates
-	if (!m_TemplateSchemaValidity[templateName])
-		return NULL;
+	if (!m_DisableValidation)
+	{
+		// Compute validity, if it's not computed before
+		if (m_TemplateSchemaValidity.find(templateName) == m_TemplateSchemaValidity.end())
+			m_TemplateSchemaValidity[templateName] = m_Validator.Validate(CStrW(templateName), m_TemplateFileData[templateName].ToXML());
+		// Refuse to return invalid templates
+		if (!m_TemplateSchemaValidity[templateName])
+			return NULL;
+	}
 
 	const CParamNode& templateRoot = m_TemplateFileData[templateName].GetChild("Entity");
 	if (!templateRoot.IsOk())
 	{
+		// The validator should never let this happen
 		LOGERROR(L"Invalid root element in entity template '%hs'", templateName.c_str());
 		return NULL;
 	}
-	// TODO: the template ought to be validated with some schema, so we don't
-	// need to nicely report errors like invalid root elements here
 
 	return &templateRoot;
 }

source/simulation2/components/ICmpTemplateManager.h

@@ -83,6 +83,11 @@ public:
 	 */
 	virtual std::vector<std::wstring> FindAllTemplates() = 0;
 
+	/**
+	 * Permanently disable XML validation (intended solely for test cases).
+	 */
+	virtual void DisableValidation() = 0;
+
 	/*
 	 * TODO:
 	 * When an entity changes template (e.g. upgrades) or player ownership, it

source/simulation2/system/ParamNode.cpp

@@ -60,6 +60,8 @@ PSRETURN CParamNode::LoadXMLString(CParamNode& ret, const char* xml)
 
 void CParamNode::ApplyLayer(const XMBFile& xmb, const XMBElement& element)
 {
+	ResetScriptVal();
+
 	std::string name = xmb.GetElementString(element.GetNodeName()); // TODO: is GetElementString inefficient?
 	utf16string value = element.GetText();
 
@@ -140,6 +142,8 @@ void CParamNode::ApplyLayer(const XMBFile& xmb, const XMBElement& element)
 
 void CParamNode::CopyFilteredChildrenOfChild(const CParamNode& src, const char* name, const std::set<std::string>& permitted)
 {
+	ResetScriptVal();
+
 	ChildrenMap::iterator dstChild = m_Childs.find(name);
 	ChildrenMap::const_iterator srcChild = src.m_Childs.find(name);
 	if (dstChild == m_Childs.end() || srcChild == src.m_Childs.end())
@@ -278,3 +282,8 @@ CScriptValRooted CParamNode::GetScriptVal() const
 {
 	return m_ScriptVal;
 }
+
+void CParamNode::ResetScriptVal()
+{
+	m_ScriptVal = CScriptValRooted();
+}

source/simulation2/system/ParamNode.h

@@ -199,7 +199,8 @@ public:
 	/**
 	 * Stores the given script representation of this node, for use in cached conversions.
 	 * This must only be called once.
-	 * The node should not be modified (e.g. by LoadXML) after setting this.
+	 * This will be reset to JSVAL_VOID if *this* node is modified (e.g. by LoadXML),
+	 * but *not* if any child nodes are modified (so don't do that).
 	 * The lifetime of the script context associated with the value must be longer
 	 * than the lifetime of this node.
 	 */
@@ -213,6 +214,8 @@ public:
 private:
 	void ApplyLayer(const XMBFile& xmb, const XMBElement& element);
 
+	void ResetScriptVal();
+
 	std::wstring m_Value;
 	ChildrenMap m_Childs;
 	bool m_IsOk;

source/simulation2/tests/test_CmpTemplateManager.h

@@ -92,6 +92,43 @@ public:
 		TS_ASSERT_WSTR_EQUALS(previewactor->ToXML(), L"<Position><Altitude>0</Altitude><Anchor>upright</Anchor><Floating>false</Floating></Position><VisualActor><Actor>example2</Actor></VisualActor>");
 	}
 
+	void test_LoadTemplate_scriptcache()
+	{
+		CSimContext context;
+		CComponentManager man(context);
+		man.LoadComponentTypes();
+
+		entity_id_t ent1 = 1, ent2 = 2;
+		CParamNode noParam;
+
+		TS_ASSERT(man.AddComponent(ent1, CID_TemplateManager, noParam));
+
+		ICmpTemplateManager* tempMan = static_cast<ICmpTemplateManager*> (man.QueryInterface(ent1, IID_TemplateManager));
+		TS_ASSERT(tempMan != NULL);
+		tempMan->DisableValidation();
+
+		CScriptValRooted val;
+		JSContext* cx = man.GetScriptInterface().GetContext();
+
+		// This is testing some bugs in the template JS object caching
+
+		const CParamNode* inherit1 = tempMan->LoadTemplate(ent2, "inherit1", -1);
+		val = CScriptValRooted(cx, ScriptInterface::ToJSVal(cx, inherit1));
+		TS_ASSERT_WSTR_EQUALS(man.GetScriptInterface().ToString(val.get()), L"({Test1A:{'@a':\"a1\", '@b':\"b1\", '@c':\"c1\", d:\"d1\", e:\"e1\", f:\"f1\"}})");
+
+		const CParamNode* inherit2 = tempMan->LoadTemplate(ent2, "inherit2", -1);
+		val = CScriptValRooted(cx, ScriptInterface::ToJSVal(cx, inherit2));
+		TS_ASSERT_WSTR_EQUALS(man.GetScriptInterface().ToString(val.get()), L"({'@parent':\"inherit1\", Test1A:{'@a':\"a2\", '@b':\"b1\", '@c':\"c1\", d:\"d2\", e:\"e1\", f:\"f1\", g:\"g2\"}})");
+
+		const CParamNode* actor = tempMan->LoadTemplate(ent2, "actor|example1", -1);
+		val = CScriptValRooted(cx, ScriptInterface::ToJSVal(cx, &actor->GetChild("VisualActor")));
+		TS_ASSERT_WSTR_EQUALS(man.GetScriptInterface().ToString(val.get()), L"({Actor:\"example1\"})");
+
+		const CParamNode* foundation = tempMan->LoadTemplate(ent2, "foundation|actor|example1", -1);
+		val = CScriptValRooted(cx, ScriptInterface::ToJSVal(cx, &foundation->GetChild("VisualActor")));
+		TS_ASSERT_WSTR_EQUALS(man.GetScriptInterface().ToString(val.get()), L"({Actor:\"example1\", Foundation:(void 0)})");
+	}
+
 	void test_LoadTemplate_errors()
 	{
 		CSimContext context;