From cefa128034220d30ccd85b179597e2b9928c1c5c Mon Sep 17 00:00:00 2001 From: Glenn Maynard Date: Mon, 12 Feb 2007 01:52:18 +0000 Subject: [PATCH] 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 | 3 +-- stepmania/src/IniFile.cpp | 2 -- stepmania/src/XmlFile.cpp | 35 ++++++++++++++--------------------- stepmania/src/XmlFile.h | 10 +++++----- stepmania/src/XmlFileUtil.cpp | 22 +++++++++++++++++----- 5 files changed, 37 insertions(+), 35 deletions(-) diff --git a/stepmania/src/ActorUtil.cpp b/stepmania/src/ActorUtil.cpp index 952ca75707..754af4e2dd 100644 --- a/stepmania/src/ActorUtil.cpp +++ b/stepmania/src/ActorUtil.cpp @@ -232,8 +232,7 @@ namespace { pNode = queue.back(); queue.pop_back(); - FOREACH_Child( pNode, pChild ) - queue.push_back( pChild ); + queue.insert( queue.end(), pNode->m_childs.begin(), pNode->m_childs.end() ); /* Source file, for error messages: */ pNode->AppendAttr( "_Source", sFile ); diff --git a/stepmania/src/IniFile.cpp b/stepmania/src/IniFile.cpp index e2aaf18564..e3051b5a3a 100644 --- a/stepmania/src/IniFile.cpp +++ b/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; } diff --git a/stepmania/src/XmlFile.cpp b/stepmania/src/XmlFile.cpp index abb0134d92..30e71417b6 100644 --- a/stepmania/src/XmlFile.cpp +++ b/stepmania/src/XmlFile.cpp @@ -98,11 +98,10 @@ XNodeValue *XNode::GetAttr( const RString &attrname ) XNode *XNode::GetChild( const RString &sName ) { - multimap::iterator it = m_childs.find( sName ); - if( it != m_childs.end() ) + FOREACH_Child( this, it ) { - DEBUG_ASSERT( sName == it->second->m_sName ); - return it->second; + if( it->GetName() == sName ) + 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::const_iterator it = m_childs.find( sName ); - if( it != m_childs.end() ) + FOREACH_CONST_Child( this, it ) { - DEBUG_ASSERT( sName == it->second->m_sName ); - return it->second; + if( it->GetName() == sName ) + 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.insert( m_childs.end(), pair(node->m_sName,node) ); + m_childs.push_back( node ); return node; } // detach node and delete object bool XNode::RemoveChild( XNode *node, bool bDelete ) { - FOREACHMM( RString, XNode*, m_childs, p ) - { - if( p->second == node ) - { - if( bDelete ) - SAFE_DELETE( p->second ); - m_childs.erase( p ); - return true; - } - } - return false; + XNodes::iterator it = find( m_childs.begin(), m_childs.end(), node ); + if( it == m_childs.end() ) + return false; + + delete node; + m_childs.erase( it ); + return true; } diff --git a/stepmania/src/XmlFile.h b/stepmania/src/XmlFile.h index 70938c953d..e5d08e7d73 100644 --- a/stepmania/src/XmlFile.h +++ b/stepmania/src/XmlFile.h @@ -54,7 +54,7 @@ public: typedef map XAttrs; class XNode; -typedef multimap XNodes; +typedef vector XNodes; #define FOREACH_Attr( pNode, Var ) \ for( XAttrs::iterator Var = (pNode)->m_attrs.begin(); \ @@ -69,22 +69,22 @@ typedef multimap 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 - XNodes m_childs; // child node + RString m_sName; + XNodes m_childs; // child nodes XAttrs m_attrs; // attributes void SetName( const RString &sName ) { m_sName = sName; } diff --git a/stepmania/src/XmlFileUtil.cpp b/stepmania/src/XmlFileUtil.cpp index d3b7240b5e..7303ec0e30 100644 --- a/stepmania/src/XmlFileUtil.cpp +++ b/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 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->second; + XNode *pSectionNode = *it; + XNode *pChildNode = pTo->GetChild( pSectionNode->GetName() ); if( pChildNode == NULL ) { - pFrom->RemoveChild( pSectionNode, false ); // don't delete - pTo->AppendChild( pSectionNode ); + aToMove.push_back( it ); } 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 ); + } } /*