瀏覽代碼

OgreImporter: Added brackets for all conditional etc. statements, even when there is a single line as requested by @kimkulling. I'm still not sure about the coding convention but looked for guidance in the obj importer code. Now newline before { if only one line and no else/else if after it, othewise a newline before it.

Jonne Nauha 11 年之前
父節點
當前提交
9ad74e461e
共有 5 個文件被更改,包括 233 次插入90 次删除
  1. 30 13
      code/OgreImporter.cpp
  2. 63 33
      code/OgreMaterial.cpp
  3. 54 17
      code/OgreMesh.cpp
  4. 32 2
      code/OgreParsingUtils.h
  5. 54 25
      code/OgreSkeleton.cpp

+ 30 - 13
code/OgreImporter.cpp

@@ -74,7 +74,6 @@ const aiImporterDesc* OgreImporter::GetInfo() const
 	return &desc;
 }
 
-
 void OgreImporter::SetupProperties(const Importer* pImp)
 {
 	m_userDefinedMaterialLibFile = pImp->GetPropertyString(AI_CONFIG_IMPORT_OGRE_MATERIAL_FILE, "Scene.material");
@@ -83,8 +82,9 @@ void OgreImporter::SetupProperties(const Importer* pImp)
 
 bool OgreImporter::CanRead(const std::string &pFile, Assimp::IOSystem *pIOHandler, bool checkSig) const
 {
-	if (!checkSig)
+	if (!checkSig) {
 		return EndsWith(pFile, ".mesh.xml", false);
+	}
 
 	const char* tokens[] = { "<mesh>" };
 	return SearchFileHeaderForToken(pIOHandler, pFile, tokens, 1);
@@ -96,21 +96,24 @@ void OgreImporter::InternReadFile(const std::string &pFile, aiScene *pScene, Ass
 	
 	// Open
 	boost::scoped_ptr<IOStream> file(pIOHandler->Open(pFile));
-	if (file.get() == NULL)
+	if (!file.get()) {
 		throw DeadlyImportError("Failed to open file " + pFile);
+	}
 
 	// Read
 	boost::scoped_ptr<CIrrXML_IOStreamReader> xmlStream(new CIrrXML_IOStreamReader(file.get()));
 	boost::scoped_ptr<XmlReader> reader(irr::io::createIrrXMLReader(xmlStream.get()));
-	if (!reader)
+	if (!reader) {
 		throw DeadlyImportError("Failed to create XML Reader for " + pFile);
+	}
 
 	DefaultLogger::get()->debug("Opened a XML reader for " + pFile);
 
 	// Read root node
 	NextNode(reader.get());
-	if (!CurrentNodeNameEquals(reader, "mesh"))
+	if (!CurrentNodeNameEquals(reader, "mesh")) {
 		throw DeadlyImportError("Root node is not <mesh> but <" + string(reader->getNodeName()) + "> in " + pFile);
+	}
 	
 	// Node names
 	const string nnSharedGeometry = "sharedgeometry";
@@ -130,14 +133,16 @@ void OgreImporter::InternReadFile(const std::string &pFile, aiScene *pScene, Ass
 		unsigned int NumVertices = GetAttribute<unsigned int>(reader.get(), "vertexcount");
 
 		NextNode(reader.get());
-		while(CurrentNodeNameEquals(reader, nnVertexBuffer))
+		while(CurrentNodeNameEquals(reader, nnVertexBuffer)) {
 			ReadVertexBuffer(m_SharedGeometry, reader.get(), NumVertices);
+		}
 	}
 
 	// -------------------- Sub Meshes --------------------
 
-	if (!CurrentNodeNameEquals(reader, nnSubMeshes))
+	if (!CurrentNodeNameEquals(reader, nnSubMeshes)) {
 		throw DeadlyImportError("Could not find <submeshes> node inside root <mesh> node");
+	}
 
 	vector<boost::shared_ptr<SubMesh> > subMeshes;
 	vector<aiMaterial*> materials;
@@ -162,20 +167,23 @@ void OgreImporter::InternReadFile(const std::string &pFile, aiScene *pScene, Ass
 		materials.push_back(material);
 	}
 
-	if (subMeshes.empty())
+	if (subMeshes.empty()) {
 		throw DeadlyImportError("Could not find <submeshes> node inside root <mesh> node");
+	}
 
 	// This is really a internal error if we failed to create dummy materials.
-	if (subMeshes.size() != materials.size())
+	if (subMeshes.size() != materials.size()) {
 		throw DeadlyImportError("Internal Error: Material count does not match the submesh count");
+	}
 
 	// Skip submesh names.
 	/// @todo Should these be read to scene etc. metadata?
 	if (CurrentNodeNameEquals(reader, nnSubMeshNames))
 	{
 		NextNode(reader.get());
-		while(CurrentNodeNameEquals(reader, nnSubMesh))
+		while(CurrentNodeNameEquals(reader, nnSubMesh)) {
 			NextNode(reader.get());
+		}
 	}
 
 	// -------------------- Skeleton --------------------
@@ -187,17 +195,24 @@ void OgreImporter::InternReadFile(const std::string &pFile, aiScene *pScene, Ass
 	{
 		string skeletonFile = GetAttribute<string>(reader.get(), "name");
 		if (!skeletonFile.empty())
+		{
 			ReadSkeleton(pFile, pIOHandler, pScene, skeletonFile, Bones, Animations);
+		}
 		else
+		{
 			DefaultLogger::get()->debug("Found a unusual <" + nnSkeletonLink + "> with a empty file reference");
+		}
 		NextNode(reader.get());
 	}
 	else
+	{
 		DefaultLogger::get()->debug("Mesh has no assigned skeleton with <" + nnSkeletonLink + ">");
+	}
 
 	// Now there might be <boneassignments> for the shared geometry
-	if (CurrentNodeNameEquals(reader, "boneassignments"))
+	if (CurrentNodeNameEquals(reader, "boneassignments")) {
 		ReadBoneWeights(m_SharedGeometry, reader.get());
+	}
 
 	// -------------------- Process Results --------------------
 	BOOST_FOREACH(boost::shared_ptr<SubMesh> submesh, subMeshes)
@@ -211,8 +226,9 @@ void OgreImporter::InternReadFile(const std::string &pFile, aiScene *pScene, Ass
 	pScene->mMaterials = new aiMaterial*[materials.size()];
 	pScene->mNumMaterials = materials.size();
 
-	for(size_t i=0, len=materials.size(); i<len; ++i)
+	for(size_t i=0, len=materials.size(); i<len; ++i) {
 		pScene->mMaterials[i] = materials[i];
+	}
 
 	// Meshes
 	pScene->mMeshes = new aiMesh*[subMeshes.size()];
@@ -229,8 +245,9 @@ void OgreImporter::InternReadFile(const std::string &pFile, aiScene *pScene, Ass
 	pScene->mRootNode->mMeshes = new unsigned int[subMeshes.size()];
 	pScene->mRootNode->mNumMeshes = subMeshes.size();
 	
-	for(size_t i=0, len=subMeshes.size(); i<len; ++i)
+	for(size_t i=0, len=subMeshes.size(); i<len; ++i) {
 		pScene->mRootNode->mMeshes[i] = static_cast<unsigned int>(i);
+	}
 
 	// Skeleton and animations
 	CreateAssimpSkeleton(pScene, Bones, Animations);

+ 63 - 33
code/OgreMaterial.cpp

@@ -62,8 +62,9 @@ static const string partBlockEnd   = "}";
 aiMaterial* OgreImporter::ReadMaterial(const std::string &pFile, Assimp::IOSystem *pIOHandler, const std::string materialName)
 {
 	/// @todo Should we return null ptr here or a empty material?
-	if (materialName.empty())
+	if (materialName.empty()) {
 		return new aiMaterial();
+	}
 
 	// Full reference and examples of Ogre Material Script 
 	// can be found from http://www.ogre3d.org/docs/manual/manual_14.html
@@ -109,8 +110,9 @@ aiMaterial* OgreImporter::ReadMaterial(const std::string &pFile, Assimp::IOSyste
 		for(size_t i=0; i<potentialFiles.size(); ++i)
 		{
 			materialFile = pIOHandler->Open(potentialFiles[i]);
-			if (materialFile)
+			if (materialFile) {
 				break;
+			}
 			DefaultLogger::get()->debug(Formatter::format() << "Source file for material '" << materialName << "' " << potentialFiles[i] << " does not exist");
 		}
 		if (!materialFile)
@@ -204,76 +206,74 @@ aiMaterial* OgreImporter::ReadMaterial(const std::string &pFile, Assimp::IOSyste
 				parent texture unit name in your cloned material.
 				This is not yet supported and below code is probably some hack from the original
 				author of this Ogre importer. Should be removed? */
-			if(linePart=="set")
+			if (linePart=="set")
 			{
 				ss >> linePart;
-				if(linePart=="$specular")//todo load this values:
+				if (linePart=="$specular")//todo load this values:
 				{
 				}
-				if(linePart=="$diffuse")
+				else if (linePart=="$diffuse")
 				{
 				}
-				if(linePart=="$ambient")
+				else if (linePart=="$ambient")
 				{
 				}
-				if(linePart=="$colormap")
+				else if (linePart=="$colormap")
 				{
 					ss >> linePart;
-					aiString ts(linePart.c_str());
+					aiString ts(linePart);
 					material->AddProperty(&ts, AI_MATKEY_TEXTURE(aiTextureType_DIFFUSE, 0));
 				}
-				if(linePart=="$normalmap")
+				else if (linePart=="$normalmap")
 				{
 					ss >> linePart;
-					aiString ts(linePart.c_str());
+					aiString ts(linePart);
 					material->AddProperty(&ts, AI_MATKEY_TEXTURE(aiTextureType_NORMALS, 0));
 				}
-				
-				if(linePart=="$shininess_strength")
+				else if (linePart=="$shininess_strength")
 				{
 					ss >> linePart;
-					float Shininess=fast_atof(linePart.c_str());
+					float Shininess = fast_atof(linePart.c_str());
 					material->AddProperty(&Shininess, 1, AI_MATKEY_SHININESS_STRENGTH);
 				}
-
-				if(linePart=="$shininess_exponent")
+				else if (linePart=="$shininess_exponent")
 				{
 					ss >> linePart;
-					float Shininess=fast_atof(linePart.c_str());
+					float Shininess = fast_atof(linePart.c_str());
 					material->AddProperty(&Shininess, 1, AI_MATKEY_SHININESS);
 				}
-
 				//Properties from Venetica:
-				if(linePart=="$diffuse_map")
+				else if (linePart=="$diffuse_map")
 				{
 					ss >> linePart;
-					if(linePart[0]=='"')// "file" -> file
-						linePart=linePart.substr(1, linePart.size()-2);
-					aiString ts(linePart.c_str());
+					if (linePart[0] == '"')// "file" -> file
+						linePart = linePart.substr(1, linePart.size()-2);
+					aiString ts(linePart);
 					material->AddProperty(&ts, AI_MATKEY_TEXTURE(aiTextureType_DIFFUSE, 0));
 				}
-				if(linePart=="$specular_map")
+				else if (linePart=="$specular_map")
 				{
 					ss >> linePart;
-					if(linePart[0]=='"')// "file" -> file
-						linePart=linePart.substr(1, linePart.size()-2);
-					aiString ts(linePart.c_str());
+					if (linePart[0] == '"')// "file" -> file
+						linePart = linePart.substr(1, linePart.size()-2);
+					aiString ts(linePart);
 					material->AddProperty(&ts, AI_MATKEY_TEXTURE(aiTextureType_SHININESS, 0));
 				}
-				if(linePart=="$normal_map")
+				else if (linePart=="$normal_map")
 				{
 					ss >> linePart;
-					if(linePart[0]=='"')// "file" -> file
-						linePart=linePart.substr(1, linePart.size()-2);
-					aiString ts(linePart.c_str());
+					if (linePart[0]=='"')// "file" -> file
+						linePart = linePart.substr(1, linePart.size()-2);
+					aiString ts(linePart);
 					material->AddProperty(&ts, AI_MATKEY_TEXTURE(aiTextureType_NORMALS, 0));
 				}
-				if(linePart=="$light_map")
+				else if (linePart=="$light_map")
 				{
 					ss >> linePart;
-					if(linePart[0]=='"')// "file" -> file
-						linePart=linePart.substr(1, linePart.size()-2);
-					aiString ts(linePart.c_str());
+					if (linePart[0]=='"') {
+						linePart = linePart.substr(1, linePart.size() - 2);
+					}
+					aiString ts(linePart);
 					material->AddProperty(&ts, AI_MATKEY_TEXTURE(aiTextureType_LIGHTMAP, 0));
 				}
 			}					
@@ -363,13 +363,21 @@ bool OgreImporter::ReadPass(const std::string &passName, stringstream &ss, aiMat
 			DefaultLogger::get()->debug(Formatter::format() << "   " << linePart << " " << r << " " << g << " " << b);
 			
 			if (linePart == partAmbient)
+			{
 				material->AddProperty(&color, 1, AI_MATKEY_COLOR_AMBIENT);
+			}
 			else if (linePart == partDiffuse)
+			{
 				material->AddProperty(&color, 1, AI_MATKEY_COLOR_DIFFUSE);
+			}
 			else if (linePart == partSpecular)
+			{
 				material->AddProperty(&color, 1, AI_MATKEY_COLOR_SPECULAR);
+			}
 			else if (linePart == partEmissive)
+			{
 				material->AddProperty(&color, 1, AI_MATKEY_COLOR_EMISSIVE);
+			}
 		}
 		else if (linePart == partTextureUnit)
 		{
@@ -430,18 +438,30 @@ bool OgreImporter::ReadTextureUnit(const std::string &textureUnitName, stringstr
 					DefaultLogger::get()->debug(Formatter::format() << "Detecting texture type from filename postfix '" << identifier << "'");
 
 					if (identifier == "_n" || identifier == "_nrm" || identifier == "_nrml" || identifier == "_normal" || identifier == "_normals" || identifier == "_normalmap")
+					{
 						textureType = aiTextureType_NORMALS;
+					}
 					else if (identifier == "_s" || identifier == "_spec" || identifier == "_specular" || identifier == "_specularmap")
+					{
 						textureType = aiTextureType_SPECULAR;
+					}
 					else if (identifier == "_l" || identifier == "_light" || identifier == "_lightmap" || identifier == "_occ" || identifier == "_occlusion")
+					{
 						textureType = aiTextureType_LIGHTMAP;
+					}
 					else if (identifier == "_disp" || identifier == "_displacement")
+					{
 						textureType = aiTextureType_DISPLACEMENT;
+					}
 					else
+					{
 						textureType = aiTextureType_DIFFUSE;
+					}
 				}
 				else
+				{
 					textureType = aiTextureType_DIFFUSE;
+				}
 			}
 			// Detect from texture unit name. This cannot be too broad as 
 			// authors might give names like "LightSaber" or "NormalNinja".
@@ -449,15 +469,25 @@ bool OgreImporter::ReadTextureUnit(const std::string &textureUnitName, stringstr
 			{
 				string unitNameLower = Ogre::ToLower(textureUnitName);
 				if (unitNameLower.find("normalmap") != string::npos)
+				{
 					textureType = aiTextureType_NORMALS;
+				}
 				else if (unitNameLower.find("specularmap") != string::npos)
+				{
 					textureType = aiTextureType_SPECULAR;
+				}
 				else if (unitNameLower.find("lightmap") != string::npos)
+				{
 					textureType = aiTextureType_LIGHTMAP;
+				}
 				else if (unitNameLower.find("displacementmap") != string::npos)
+				{
 					textureType = aiTextureType_DISPLACEMENT;
+				}
 				else
+				{
 					textureType = aiTextureType_DIFFUSE;
+				}
 			}
 		}
 		else if (linePart == partTextCoordSet)

+ 54 - 17
code/OgreMesh.cpp

@@ -54,12 +54,15 @@ namespace Ogre
 
 void OgreImporter::ReadSubMesh(const unsigned int submeshIndex, SubMesh &submesh, XmlReader *reader)
 {
-	if (reader->getAttributeValue("material"))
+	if (reader->getAttributeValue("material")) {
 		submesh.MaterialName = GetAttribute<string>(reader, "material");
-	if (reader->getAttributeValue("use32bitindexes"))
+	}
+	if (reader->getAttributeValue("use32bitindexes")) {
 		submesh.Use32bitIndexes = GetAttribute<bool>(reader, "use32bitindexes");
-	if (reader->getAttributeValue("usesharedvertices"))
+	}
+	if (reader->getAttributeValue("usesharedvertices")) {
 		submesh.UseSharedGeometry = GetAttribute<bool>(reader, "usesharedvertices");
+	}
 
 	DefaultLogger::get()->debug(Formatter::format() << "Reading submesh " << submeshIndex);
 	DefaultLogger::get()->debug(Formatter::format() << "  - Material '" << submesh.MaterialName << "'");
@@ -100,8 +103,9 @@ void OgreImporter::ReadSubMesh(const unsigned int submeshIndex, SubMesh &submesh
 				NewFace.VertexIndices[2] = GetAttribute<int>(reader, "v3");
 
 				/// @todo Support quads
-				if (!quadWarned && reader->getAttributeValue("v4"))
+				if (!quadWarned && reader->getAttributeValue("v4")) {
 					DefaultLogger::get()->warn("Submesh has quads, only triangles are supported at the moment!");
+				}
 
 				submesh.Faces.push_back(NewFace);
 
@@ -111,20 +115,27 @@ void OgreImporter::ReadSubMesh(const unsigned int submeshIndex, SubMesh &submesh
 			}
 
 			if (submesh.Faces.size() == numFaces)
+			{
 				DefaultLogger::get()->debug(Formatter::format() << "  - Faces " << numFaces);
+			}
 			else
+			{
 				throw DeadlyImportError(Formatter::format() << "Read only " << submesh.Faces.size() << " faces when should have read " << numFaces);
+			}
 		}
 		else if (currentNodeName == nnGeometry)
 		{	
 			unsigned int numVertices = GetAttribute<int>(reader, "vertexcount");
 
 			NextNode(reader);
-			while(string(reader->getNodeName()) == nnVertexBuffer)
+			while(string(reader->getNodeName()) == nnVertexBuffer) {
 				ReadVertexBuffer(submesh, reader, numVertices);
+			}
 		}
 		else if (reader->getNodeName() == nnBoneAssignments)
+		{
 			ReadBoneWeights(submesh, reader);
+		}
 			
 		currentNodeName = reader->getNodeName();
 	}
@@ -157,13 +168,15 @@ void OgreImporter::ReadVertexBuffer(SubMesh &submesh, XmlReader *reader, const u
 	if (reader->getAttributeValue("texture_coords"))
 	{
 		submesh.Uvs.resize(GetAttribute<unsigned int>(reader, "texture_coords"));
-		for(size_t i=0, len=submesh.Uvs.size(); i<len; ++i)
+		for(size_t i=0, len=submesh.Uvs.size(); i<len; ++i) {
 			submesh.Uvs[i].reserve(numVertices);
+		}
 		DefaultLogger::get()->debug(Formatter::format() << "  - Has " << submesh.Uvs.size() << " texture coords");
 	}
 
-	if (!submesh.HasPositions)
+	if (!submesh.HasPositions) {
 		throw DeadlyImportError("Vertex buffer does not contain positions!");
+	}
 
 	const string nnVertex        = "vertex";
 	const string nnPosition      = "position";
@@ -227,8 +240,9 @@ void OgreImporter::ReadVertexBuffer(SubMesh &submesh, XmlReader *reader, const u
 		{
 			for(size_t i=0, len=submesh.Uvs.size(); i<len; ++i)
 			{
-				if (currentNodeName != nnTexCoord)
+				if (currentNodeName != nnTexCoord) {
 					throw DeadlyImportError("Vertex buffer declared more UVs than can be found in a vertex");
+				}
 
 				aiVector3D NewUv;
 				NewUv.x = GetAttribute<float>(reader, "u");
@@ -248,26 +262,39 @@ void OgreImporter::ReadVertexBuffer(SubMesh &submesh, XmlReader *reader, const u
 			if (currentNodeName == nnBinormal)
 			{
 				if (warnBinormal)
+				{
 					warnBinormal = false;
+				}
 				else
+				{
 					warn = false;
+				}
 			}
 			else if (currentNodeName == nnColorDiffuse)
 			{
 				if (warnColorDiffuse)
+				{
 					warnColorDiffuse = false;
+				}
 				else
+				{
 					warn = false;
+				}
 			}
 			else if (currentNodeName == nnColorSpecular)
 			{
 				if (warnColorSpecular)
+				{
 					warnColorSpecular = false;
+				}
 				else
+				{
 					warn = false;
+				}
 			}
-			if (warn)
+			if (warn) {
 				DefaultLogger::get()->warn(string("Vertex buffer attribute read not implemented for element: ") + currentNodeName);
+			}
 		}
 
 		// Advance
@@ -282,15 +309,18 @@ void OgreImporter::ReadVertexBuffer(SubMesh &submesh, XmlReader *reader, const u
 		" Tangents "  << submesh.Tangents.size());
 
 	// Sanity checks
-	if (submesh.HasNormals && submesh.Normals.size() != numVertices)
+	if (submesh.HasNormals && submesh.Normals.size() != numVertices) {
 		throw DeadlyImportError(Formatter::format() << "Read only " << submesh.Normals.size() << " normals when should have read " << numVertices);
-	if (submesh.HasTangents && submesh.Tangents.size() != numVertices)
+	}
+	if (submesh.HasTangents && submesh.Tangents.size() != numVertices) {
 		throw DeadlyImportError(Formatter::format() << "Read only " << submesh.Tangents.size() << " tangents when should have read " << numVertices);
+	}
 	for(unsigned int i=0; i<submesh.Uvs.size(); ++i)
 	{
-		if (submesh.Uvs[i].size() != numVertices)
+		if (submesh.Uvs[i].size() != numVertices) {
 			throw DeadlyImportError(Formatter::format() << "Read only " << submesh.Uvs[i].size() 
 				<< " uvs for uv index " << i << " when should have read " << numVertices);
+		}
 	}
 }
 
@@ -335,8 +365,9 @@ void OgreImporter::ProcessSubMesh(SubMesh &submesh, SubMesh &sharedGeometry)
 	vector<vector<BoneWeight> > uniqueWeights(uniqueVertexCount);
 	vector<vector<aiVector3D> > uniqueUvs(submesh.UseSharedGeometry ? sharedGeometry.Uvs.size() : submesh.Uvs.size());
 
-	for(size_t uvi=0; uvi<uniqueUvs.size(); ++uvi)
+	for(size_t uvi=0; uvi<uniqueUvs.size(); ++uvi) {
 		uniqueUvs[uvi].resize(uniqueVertexCount);
+	}
 
 	/* Support for shared geometry.
 	   We can use this loop to copy vertex informations from the shared data pool. In order to do so
@@ -417,13 +448,17 @@ void OgreImporter::ProcessSubMesh(SubMesh &submesh, SubMesh &sharedGeometry)
 		std::vector<BoneWeight> &weights = submesh.Weights[vertexId];
 		
 		float sum = 0.0f;
-		for(size_t boneId=0, blen=weights.size(); boneId<blen; ++boneId)
+		for(size_t boneId=0, blen=weights.size(); boneId<blen; ++boneId) {
 			sum += weights[boneId].Value;
+		}
 		
 		//check if the sum is too far away from 1
 		if ((sum < (1.0f - 0.05f)) || (sum > (1.0f + 0.05f)))
-			for(size_t boneId=0, blen=weights.size(); boneId<blen; ++boneId)
+		{
+			for(size_t boneId=0, blen=weights.size(); boneId<blen; ++boneId) {
 				weights[boneId].Value /= sum;
+			}
+		}
 	}
 }
 
@@ -485,8 +520,9 @@ aiMesh *OgreImporter::CreateAssimpSubMesh(aiScene *pScene, const SubMesh& submes
 	for(size_t boneId=0, len=submesh.BonesUsed; boneId<len; ++boneId)
 	{
 		const vector<aiVertexWeight> &boneWeights = assimpWeights[boneId];
-		if (boneWeights.size() == 0)
+		if (boneWeights.size() == 0) {
 			continue;
+		}
 
 		// @note The bones list is sorted by id's, this was done in LoadSkeleton.
 		aiBone *assimpBone = new aiBone();
@@ -504,8 +540,9 @@ aiMesh *OgreImporter::CreateAssimpSubMesh(aiScene *pScene, const SubMesh& submes
 		dest->mBones = new aiBone*[assimpBones.size()];
 		dest->mNumBones = assimpBones.size();
 
-		for(size_t i=0, len=assimpBones.size(); i<len; ++i)
+		for(size_t i=0, len=assimpBones.size(); i<len; ++i) {
 			dest->mBones[i] = assimpBones[i];
+		}
 	}
 
 	// Faces

+ 32 - 2
code/OgreParsingUtils.h

@@ -18,9 +18,13 @@ typedef irr::io::IrrXMLReader XmlReader;
 static void ThrowAttibuteError(const XmlReader* reader, const std::string &name, const std::string &error = "")
 {
 	if (!error.empty())
+	{
 		throw DeadlyImportError(error + " in node '" + std::string(reader->getNodeName()) + "' and attribute '" + name + "'");
+	}
 	else
+	{
 		throw DeadlyImportError("Attribute '" + name + "' does not exist in node '" + std::string(reader->getNodeName()) + "'");
+	}
 }
 
 template<typename T> 
@@ -31,7 +35,9 @@ inline int GetAttribute<int>(const XmlReader* reader, const std::string &name)
 {
 	const char* value = reader->getAttributeValue(name.c_str());
 	if (value)
+	{
 		return atoi(value);
+	}
 	else
 	{
 		ThrowAttibuteError(reader, name);
@@ -44,7 +50,9 @@ inline unsigned int GetAttribute<unsigned int>(const XmlReader* reader, const st
 {
 	const char* value = reader->getAttributeValue(name.c_str());
 	if (value)
+	{
 		return static_cast<unsigned int>(atoi(value)); ///< @todo Find a better way...
+	}
 	else
 	{
 		ThrowAttibuteError(reader, name);
@@ -57,7 +65,9 @@ inline float GetAttribute<float>(const XmlReader* reader, const std::string &nam
 {
 	const char* value = reader->getAttributeValue(name.c_str());
 	if (value)
+	{
 		return fast_atof(value);
+	}
 	else
 	{
 		ThrowAttibuteError(reader, name);
@@ -70,7 +80,9 @@ inline std::string GetAttribute<std::string>(const XmlReader* reader, const std:
 {
 	const char* value = reader->getAttributeValue(name.c_str());
 	if (value)
+	{
 		return std::string(value);
+	}
 	else
 	{
 		ThrowAttibuteError(reader, name);
@@ -83,9 +95,13 @@ inline bool GetAttribute<bool>(const XmlReader* reader, const std::string &name)
 {
 	std::string value = GetAttribute<std::string>(reader, name);
 	if (ASSIMP_stricmp(value, "true") == 0)
+	{
 		return true;
+	}
 	else if (ASSIMP_stricmp(value, "false") == 0)
+	{
 		return false;
+	}
 	else
 	{
 		ThrowAttibuteError(reader, name, "Boolean value is expected to be 'true' or 'false', encountered '" + value + "'");
@@ -97,8 +113,9 @@ inline bool NextNode(XmlReader* reader)
 {
 	do
 	{
-		if (!reader->read())
+		if (!reader->read()) {
 			return false;
+		}
 	}
 	while(reader->getNodeType() != irr::io::EXN_ELEMENT);
 	return true;
@@ -137,12 +154,17 @@ static inline std::string ToLower(std::string s)
 static inline bool EndsWith(const std::string &s, const std::string &suffix, bool caseSensitive = true)
 {
 	if (s.empty() || suffix.empty())
+	{
 		return false;
+	}
 	else if (s.length() < suffix.length())
+	{
 		return false;
+	}
 
-	if (!caseSensitive)
+	if (!caseSensitive) {
 		return EndsWith(ToLower(s), ToLower(suffix), true);
+	}
 
 	size_t len = suffix.length();
 	std::string sSuffix = s.substr(s.length()-len, len);
@@ -155,9 +177,13 @@ static inline bool EndsWith(const std::string &s, const std::string &suffix, boo
 static inline std::string &TrimLeft(std::string &s, bool newlines = true)
 {
 	if (!newlines)
+	{
 		s.erase(s.begin(), std::find_if(s.begin(), s.end(), std::not1(std::ptr_fun(Assimp::IsSpace<char>))));
+	}
 	else
+	{
 		s.erase(s.begin(), std::find_if(s.begin(), s.end(), std::not1(std::ptr_fun(Assimp::IsSpaceOrNewLine<char>))));
+	}
 	return s;
 }
 
@@ -165,9 +191,13 @@ static inline std::string &TrimLeft(std::string &s, bool newlines = true)
 static inline std::string &TrimRight(std::string &s, bool newlines = true)
 {
 	if (!newlines)
+	{
 		s.erase(std::find_if(s.rbegin(), s.rend(), std::not1(std::ptr_fun(Assimp::IsSpace<char>))).base(),s.end());
+	}
 	else
+	{
 		s.erase(s.begin(), std::find_if(s.begin(), s.end(), std::not1(std::ptr_fun(Assimp::IsSpaceOrNewLine<char>))));
+	}
 	return s;
 }
 

+ 54 - 25
code/OgreSkeleton.cpp

@@ -69,53 +69,60 @@ void OgreImporter::ReadSkeleton(const std::string &pFile, Assimp::IOSystem *pIOH
 	}
 
 	boost::scoped_ptr<IOStream> file(pIOHandler->Open(filename));
-	if (!file.get())
+	if (!file.get()) {
 		throw DeadlyImportError("Failed to open skeleton file " + filename);
+	}
 
 	boost::scoped_ptr<CIrrXML_IOStreamReader> stream(new CIrrXML_IOStreamReader(file.get()));
 	XmlReader* reader = irr::io::createIrrXMLReader(stream.get());
-	if (!reader)
+	if (!reader) {
 		throw DeadlyImportError("Failed to create XML reader for skeleton file " + filename);
+	}
 
 	DefaultLogger::get()->debug("Reading skeleton '" + filename + "'");
 
 	// Root
 	NextNode(reader);
-	if (!CurrentNodeNameEquals(reader, "skeleton"))
+	if (!CurrentNodeNameEquals(reader, "skeleton")) {
 		throw DeadlyImportError("Root node is not <skeleton> but <" + string(reader->getNodeName()) + "> in " + filename);
+	}
 
 	// Bones
 	NextNode(reader);
-	if (!CurrentNodeNameEquals(reader, "bones"))
+	if (!CurrentNodeNameEquals(reader, "bones")) {
 		throw DeadlyImportError("No <bones> node in skeleton " + skeletonFile);
+	}
 
 	NextNode(reader);
 	while(CurrentNodeNameEquals(reader, "bone"))
 	{
-		//TODO: Maybe we can have bone ids for the errrors, but normaly, they should never appear, so what....
-		/// @todo What does the above mean?
+		/** @todo Fix this mandatory ordering. Some exporters might just write rotation first etc.
+			There is no technical reason this has to be so strict. */
 
 		Bone bone;
 		bone.Id = GetAttribute<int>(reader, "id");
 		bone.Name = GetAttribute<string>(reader, "name");
 
 		NextNode(reader);
-		if (!CurrentNodeNameEquals(reader, "position"))
+		if (!CurrentNodeNameEquals(reader, "position")) {
 			throw DeadlyImportError("Position is not first node in Bone!");
+		}
 
 		bone.Position.x = GetAttribute<float>(reader, "x");
 		bone.Position.y = GetAttribute<float>(reader, "y");
 		bone.Position.z = GetAttribute<float>(reader, "z");
 
 		NextNode(reader);
-		if (!CurrentNodeNameEquals(reader, "rotation"))
+		if (!CurrentNodeNameEquals(reader, "rotation")) {
 			throw DeadlyImportError("Rotation is not the second node in Bone!");
+		}
 		
 		bone.RotationAngle = GetAttribute<float>(reader, "angle");
 		
 		NextNode(reader);
-		if (!CurrentNodeNameEquals(reader, "axis"))
+		if (!CurrentNodeNameEquals(reader, "axis")) {
 			throw DeadlyImportError("No axis specified for bone rotation!");
+		}
 			
 		bone.RotationAxis.x = GetAttribute<float>(reader, "x");
 		bone.RotationAxis.y = GetAttribute<float>(reader, "y");
@@ -133,14 +140,18 @@ void OgreImporter::ReadSkeleton(const std::string &pFile, Assimp::IOSystem *pIOH
 	/** @note Left this from original authors code, but not sure if this is strictly necessary
 		as per the Ogre skeleton spec. It might be more that other (later) code in this imported does not break. */
 	for (size_t i=0, len=Bones.size(); i<len; ++i)
-		if (static_cast<int>(Bones[i].Id) != static_cast<int>(i))
+	{
+		if (static_cast<int>(Bones[i].Id) != static_cast<int>(i)) {
 			throw DeadlyImportError("Bone Ids are not in sequence in " + skeletonFile);
+		}
+	}
 
 	DefaultLogger::get()->debug(Formatter::format() << "  - Bones " << Bones.size());
 
 	// Bone hierarchy
-	if (!CurrentNodeNameEquals(reader, "bonehierarchy"))
+	if (!CurrentNodeNameEquals(reader, "bonehierarchy")) {
 		throw DeadlyImportError("No <bonehierarchy> node found after <bones> in " + skeletonFile);
+	}
 
 	NextNode(reader);
 	while(CurrentNodeNameEquals(reader, "boneparent"))
@@ -157,7 +168,9 @@ void OgreImporter::ReadSkeleton(const std::string &pFile, Assimp::IOSystem *pIOH
 			iterParent->Children.push_back(iterChild->Id);
 		}
 		else
+		{
 			DefaultLogger::get()->warn("Failed to find bones for parenting: Child " + childName + " Parent " + parentName);
+		}
 
 		NextNode(reader);
 	}
@@ -165,8 +178,9 @@ void OgreImporter::ReadSkeleton(const std::string &pFile, Assimp::IOSystem *pIOH
 	// Calculate bone matrices for root bones. Recursively does their children.
 	BOOST_FOREACH(Bone &theBone, Bones)
 	{
-		if (!theBone.IsParented())
+		if (!theBone.IsParented()) {
 			theBone.CalculateBoneToWorldSpaceMatrix(Bones);
+		}
 	}
 	
 	aiVector3D zeroVec(0.f, 0.f, 0.f);
@@ -185,8 +199,9 @@ void OgreImporter::ReadSkeleton(const std::string &pFile, Assimp::IOSystem *pIOH
 			
 			// Tracks
 			NextNode(reader);
-			if (!CurrentNodeNameEquals(reader, "tracks"))
+			if (!CurrentNodeNameEquals(reader, "tracks")) {
 				throw DeadlyImportError("No <tracks> node found in animation '" + animation.Name + "' in " + skeletonFile);
+			}
 
 			NextNode(reader);
 			while(CurrentNodeNameEquals(reader, "track"))
@@ -196,8 +211,9 @@ void OgreImporter::ReadSkeleton(const std::string &pFile, Assimp::IOSystem *pIOH
 
 				// Keyframes
 				NextNode(reader);
-				if (!CurrentNodeNameEquals(reader, "keyframes"))
+				if (!CurrentNodeNameEquals(reader, "keyframes")) {
 					throw DeadlyImportError("No <keyframes> node found in a track in animation '" + animation.Name + "' in " + skeletonFile);
+				}
 
 				NextNode(reader);
 				while(CurrentNodeNameEquals(reader, "keyframe"))
@@ -219,8 +235,9 @@ void OgreImporter::ReadSkeleton(const std::string &pFile, Assimp::IOSystem *pIOH
 							float angle = GetAttribute<float>(reader, "angle");
 
 							NextNode(reader);
-							if(string("axis")!=reader->getNodeName())
-								throw DeadlyImportError("No axis for keyframe rotation!");
+							if (!CurrentNodeNameEquals(reader, "axis")) {
+								throw DeadlyImportError("No axis for keyframe rotation in animation '" + animation.Name + "'");
+							}
 								
 							aiVector3D axis;
 							axis.x = GetAttribute<float>(reader, "x");
@@ -230,8 +247,9 @@ void OgreImporter::ReadSkeleton(const std::string &pFile, Assimp::IOSystem *pIOH
 							if (axis.Equal(zeroVec))
 							{
 								axis.x = 1.0f;
-								if (angle != 0)
+								if (angle != 0) {
 									DefaultLogger::get()->warn("Found invalid a key frame with a zero rotation axis in animation '" + animation.Name + "'");
+								}
 							}
 							keyFrame.Rotation = aiQuaternion(axis, angle);
 						}
@@ -256,20 +274,24 @@ void OgreImporter::ReadSkeleton(const std::string &pFile, Assimp::IOSystem *pIOH
 
 void OgreImporter::CreateAssimpSkeleton(aiScene *pScene, const std::vector<Bone> &bones, const std::vector<Animation> &animations)
 {
-	if (bones.empty())
+	if (bones.empty()) {
 		return;
+	}
 
-	if (!pScene->mRootNode)
+	if (!pScene->mRootNode) {
 		throw DeadlyImportError("Creating Assimp skeleton: No root node created!");
-	if (pScene->mRootNode->mNumChildren > 0)
+	}
+	if (pScene->mRootNode->mNumChildren > 0) {
 		throw DeadlyImportError("Creating Assimp skeleton: Root node already has children!");
+	}
 
 	// Bones
 	vector<aiNode*> rootBones;
 	BOOST_FOREACH(const Bone &bone, bones)
 	{
-		if (!bone.IsParented())
+		if (!bone.IsParented()) {
 			rootBones.push_back(CreateNodeFromBone(bone.Id, bones, pScene->mRootNode));
+		}
 	}
 	
 	if (!rootBones.empty())
@@ -277,8 +299,9 @@ void OgreImporter::CreateAssimpSkeleton(aiScene *pScene, const std::vector<Bone>
 		pScene->mRootNode->mChildren = new aiNode*[rootBones.size()];
 		pScene->mRootNode->mNumChildren = rootBones.size();
 
-		for(size_t i=0, len=rootBones.size(); i<len; ++i)
+		for(size_t i=0, len=rootBones.size(); i<len; ++i) {
 			pScene->mRootNode->mChildren[i] = rootBones[i];
+		}
 	}
 
 	// TODO: Auf nicht vorhandene Animationskeys achten!
@@ -315,8 +338,9 @@ void OgreImporter::CreateAssimpSkeleton(aiScene *pScene, const std::vector<Bone>
 				vector<Bone>::const_iterator boneIter = find(bones.begin(), bones.end(), tSource.BoneName);
 				if (boneIter == bones.end())
 				{
-					for(unsigned int a=0; a<ai; a++)
-						delete pScene->mAnimations[a];
+					for(size_t createdAnimationIndex=0; createdAnimationIndex<ai; createdAnimationIndex++) {
+						delete pScene->mAnimations[createdAnimationIndex];
+					}
 					delete [] pScene->mAnimations;
 					pScene->mAnimations = NULL;
 					pScene->mNumAnimations = 0;
@@ -387,8 +411,9 @@ aiNode* OgreImporter::CreateNodeFromBone(int boneId, const std::vector<Bone> &bo
 		boneNode->mChildren = new aiNode*[source.Children.size()];
 		boneNode->mNumChildren = source.Children.size();
 
-		for(size_t i=0, len=source.Children.size(); i<len; ++i)
+		for(size_t i=0, len=source.Children.size(); i<len; ++i) {
 			boneNode->mChildren[i] = CreateNodeFromBone(source.Children[i], bones, boneNode);
+		}
 	}
 
 	return boneNode;
@@ -400,9 +425,13 @@ void Bone::CalculateBoneToWorldSpaceMatrix(vector<Bone> &Bones)
 	aiMatrix4x4 transform = aiMatrix4x4::Rotation(-RotationAngle, RotationAxis, t1) * aiMatrix4x4::Translation(-Position, t0);
 
 	if (!IsParented())
+	{
 		BoneToWorldSpace = transform;
+	}
 	else
+	{
 		BoneToWorldSpace = transform * Bones[ParentId].BoneToWorldSpace;
+	}
 
 	// Recursively for all children now that the parent matrix has been calculated.
 	BOOST_FOREACH(int childId, Children)