change XNode children to a vector.

This means the order of children is preserved.

It also means the name of each node isn't duplicated.  This was
an opportunity for an inconsistent XNode tree.  Nodes can now be
renamed after being inserted into a tree.

Searching for children by name isn't efficient.  Use attributes for that.

stepmania/src/ActorUtil.cpp

@@ -232,8 +232,7 @@ namespace
 		{
 			pNode = queue.back();
 			queue.pop_back();
-			FOREACH_Child( pNode, pChild )
+			queue.insert( queue.end(), pNode->m_childs.begin(), pNode->m_childs.end() );
-				queue.push_back( pChild );
 
 			/* Source file, for error messages: */
 			pNode->AppendAttr( "_Source", sFile );

stepmania/src/IniFile.cpp

@@ -147,9 +147,7 @@ bool IniFile::RenameKey(const RString &from, const RString &to)
 	if( pNode == NULL )
 		return false;
 
-	RemoveChild( pNode, false );
 	pNode->SetName( to );
-	AppendChild( pNode );
 
 	return true;
 }

stepmania/src/XmlFile.cpp

@@ -98,11 +98,10 @@ XNodeValue *XNode::GetAttr( const RString &attrname )
 
 XNode *XNode::GetChild( const RString &sName )
 {
-	multimap<RString, XNode*>::iterator it = m_childs.find( sName );
+	FOREACH_Child( this, it )
-	if( it != m_childs.end() )
 	{
-		DEBUG_ASSERT( sName == it->second->m_sName );
+		if( it->GetName() == sName )
-		return it->second;
+			return it;
 	}
 	return NULL;
 }
@@ -121,11 +120,10 @@ bool XNode::PushChildValue( lua_State *L, const RString &sName ) const
 
 const XNode *XNode::GetChild( const RString &sName ) const
 {
-	multimap<RString, XNode*>::const_iterator it = m_childs.find( sName );
+	FOREACH_CONST_Child( this, it )
-	if( it != m_childs.end() )
 	{
-		DEBUG_ASSERT( sName == it->second->m_sName );
+		if( it->GetName() == sName )
-		return it->second;
+			return it;
 	}
 	return NULL;
 }
@@ -134,25 +132,20 @@ XNode *XNode::AppendChild( XNode *node )
 {
 	DEBUG_ASSERT( node->m_sName.size() );
 
-	/* Hinted insert: optimize for alphabetical inserts, for the copy ctor. */
+	m_childs.push_back( node );
-	m_childs.insert( m_childs.end(), pair<RString,XNode*>(node->m_sName,node) );
 	return node;
 }
 
 // detach node and delete object
 bool XNode::RemoveChild( XNode *node, bool bDelete )
 {
-	FOREACHMM( RString, XNode*, m_childs, p )
+	XNodes::iterator it = find( m_childs.begin(), m_childs.end(), node );
-	{
+	if( it == m_childs.end() )
-		if( p->second == node )
+		return false;
-		{
+
-			if( bDelete )
+	delete node;
-				SAFE_DELETE( p->second );
+	m_childs.erase( it );
-			m_childs.erase( p );
+	return true;
-			return true;
-		}
-	}
-	return false;
 }
 
 

stepmania/src/XmlFile.h

@@ -54,7 +54,7 @@ public:
 
 typedef map<RString,XNodeValue*> XAttrs;
 class XNode;
-typedef multimap<RString,XNode*> XNodes;
+typedef vector<XNode*> XNodes;
 
 #define FOREACH_Attr( pNode, Var ) \
 	for( XAttrs::iterator Var = (pNode)->m_attrs.begin(); \
@@ -69,22 +69,22 @@ typedef multimap<RString,XNode*> XNodes;
 #define FOREACH_Child( pNode, Var ) \
 	XNode *Var = NULL; \
 	for( XNodes::iterator Var##Iter = (pNode)->m_childs.begin(); \
-		Var = (Var##Iter != (pNode)->m_childs.end())? Var##Iter->second:NULL, \
+		Var = (Var##Iter != (pNode)->m_childs.end())? *Var##Iter:NULL, \
 		Var##Iter != (pNode)->m_childs.end(); \
 		++Var##Iter )
 
 #define FOREACH_CONST_Child( pNode, Var ) \
 	const XNode *Var = NULL; \
 	for( XNodes::const_iterator Var##Iter = (pNode)->m_childs.begin(); \
-		Var = (Var##Iter != (pNode)->m_childs.end())? Var##Iter->second:NULL, \
+		Var = (Var##Iter != (pNode)->m_childs.end())? *Var##Iter:NULL, \
 		Var##Iter != (pNode)->m_childs.end(); \
 		++Var##Iter )
 
 class XNode
 {
 public:
-	RString m_sName;	// a duplicate of the m_sName in the parent's map
+	RString m_sName;
-	XNodes	m_childs;	// child node
+	XNodes	m_childs;	// child nodes
 	XAttrs	m_attrs;	// attributes
 
 	void SetName( const RString &sName ) { m_sName = sName; }

stepmania/src/XmlFileUtil.cpp

@@ -330,7 +330,7 @@ unsigned LoadInternal( XNode *pNode, const RString &xml, RString &sErrorOut, uns
 		if( !node->GetName().empty() )
 		{
 			DEBUG_ASSERT( node->GetName().size() );
-			pNode->m_childs.insert( make_pair(node->GetName(), node) );
+			pNode->m_childs.push_back( node );
 		}
 		else
 		{
@@ -731,6 +731,10 @@ XNode *XmlFileUtil::XNodeFromTable( lua_State *L )
  * On return, the contents of pFrom will be undefined and should be deleted. */
 void XmlFileUtil::MergeIniUnder( XNode *pFrom, XNode *pTo )
 {
+	/* Batch up nodes to move, and do them all at once, to deal sanely
+	 * with the possibility of duplicate child names. */
+	vector<XNodes::iterator> aToMove;
+
 	/* Iterate over each section in pFrom. */
 	XNodes::iterator it = pFrom->m_childs.begin();
 	while( it != pFrom->m_childs.end() )
@@ -739,12 +743,11 @@ void XmlFileUtil::MergeIniUnder( XNode *pFrom, XNode *pTo )
 		++next;
 
 		/* If this node doesn't exist in pTo, just move the whole node. */
-		XNode *pChildNode = pTo->GetChild( it->first );
+		XNode *pSectionNode = *it;
-		XNode *pSectionNode = it->second;
+		XNode *pChildNode = pTo->GetChild( pSectionNode->GetName() );
 		if( pChildNode == NULL )
 		{
-			pFrom->RemoveChild( pSectionNode, false ); // don't delete
+			aToMove.push_back( it );
-			pTo->AppendChild( pSectionNode );
 		}
 		else
 		{
@@ -757,6 +760,15 @@ void XmlFileUtil::MergeIniUnder( XNode *pFrom, XNode *pTo )
 
 		it = next;
 	}
+
+	/* Iterate in reverse, since erasing iterators will invalidate the
+	 * iterators after it. */
+	for( int i = aToMove.size()-1; i >= 0; --i )
+	{
+		XNode *pNode = *aToMove[i];
+		pFrom->m_childs.erase( aToMove[i] );
+		pTo->AppendChild( pNode );
+	}
 }
 
 /*