diff --git a/binaries/data/mods/official/art/skeletons/skeletons.xml b/binaries/data/mods/official/art/skeletons/skeletons.xml new file mode 100644 index 0000000000..04e2b2ff0c --- /dev/null +++ b/binaries/data/mods/official/art/skeletons/skeletons.xml @@ -0,0 +1,265 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Bip01 + + + + root + + + pelvis + + + spine + + + spine1 + + + neck + + + head + + + + + l_clavicle + + + l_upperarm + + + l_forearm + + + l_hand + + + + + + + + + + + + + r_clavicle + + + r_upperarm + + + r_forearm + + + r_hand + + + + + + + + + + + + + + + + l_thigh + + + l_calf + + + l_foot + + + + + + + + + + + + r_thigh + + + r_calf + + + r_foot + + + + + + + + + + + + + + + + + Biped_GlobalSRT + + + + root + + + pelvis + + + spine + + + spine1 + + + + + l_clavicle + + + l_upperarm + + + l_forearm + + + + r_clavicle + + + r_upperarm + + + r_forearm + + + + neck + + + head + + + + l_thigh + + + l_calf + + + + r_thigh + + + r_calf + + + + l_hand + + + + l_foot + + + + r_hand + + + + r_foot + + + + + \ No newline at end of file diff --git a/binaries/data/tests/collada/skeletons.xml b/binaries/data/tests/collada/skeletons.xml new file mode 100644 index 0000000000..0d4a5b8156 --- /dev/null +++ b/binaries/data/tests/collada/skeletons.xml @@ -0,0 +1,3 @@ + + + \ No newline at end of file diff --git a/source/collada/CommonConvert.cpp b/source/collada/CommonConvert.cpp index b39f9d7a05..60a50cb187 100644 --- a/source/collada/CommonConvert.cpp +++ b/source/collada/CommonConvert.cpp @@ -63,7 +63,7 @@ void FColladaErrorHandler::OnError(FUError::Level errorLevel, uint32 errorCode, if (errorLevel == FUError::DEBUG) Log(LOG_INFO, "FCollada message %d: %s", errorCode, errorString); else if (errorLevel == FUError::WARNING) - Log(LOG_WARNING, "FCollada error %d: %s", errorCode, errorString); + Log(LOG_WARNING, "FCollada warning %d: %s", errorCode, errorString); else throw ColladaException(errorString); } @@ -103,6 +103,7 @@ void FColladaDocument::LoadFromText(const char *text) } else { + xmlCleanupParser(); // do it here because our error handler throws FUError::Error(FUError::ERROR, FUError::ERROR_MALFORMED_XML); status = false; } diff --git a/source/collada/DLL.cpp b/source/collada/DLL.cpp index df7835f3d0..680c9e76e9 100644 --- a/source/collada/DLL.cpp +++ b/source/collada/DLL.cpp @@ -119,17 +119,22 @@ EXPORT int convert_dae_to_psa(const char* dae, OutputFn psa_writer, void* cb_dat return convert_dae_to_whatever(dae, psa_writer, cb_data, ColladaToPSA); } -EXPORT int set_skeletons(const char* xml) +EXPORT int set_skeleton_definitions(const char* xml, int length) { + std::string xmlErrors; try { - Skeleton::LoadSkeletonDataFromXml(xml); + Skeleton::LoadSkeletonDataFromXml(xml, length, xmlErrors); } catch (const ColladaException& e) { + if (! xmlErrors.empty()) + Log(LOG_ERROR, "%s", xmlErrors.c_str()); + Log(LOG_ERROR, "%s", e.what()); - return -2; + return -1; } + return 0; } diff --git a/source/collada/DLL.h b/source/collada/DLL.h index 9016a0cb5c..e442dfebdd 100644 --- a/source/collada/DLL.h +++ b/source/collada/DLL.h @@ -25,7 +25,7 @@ typedef void (*OutputFn) (void* cb_data, const char* data, unsigned int length); #define COLLADA_CONVERTER_VERSION 1 EXPORT void set_logger(LogFn logger); -EXPORT int set_skeletons(const char* xml); +EXPORT int set_skeleton_definitions(const char* xml, int length); EXPORT int convert_dae_to_pmd(const char* dae, OutputFn pmd_writer, void* cb_data); EXPORT int convert_dae_to_psa(const char* dae, OutputFn psa_writer, void* cb_data); diff --git a/source/collada/StdSkeletons.cpp b/source/collada/StdSkeletons.cpp index 260961f7d3..d7c42c5180 100644 --- a/source/collada/StdSkeletons.cpp +++ b/source/collada/StdSkeletons.cpp @@ -106,6 +106,8 @@ namespace } else { + // No target - this is a standard skeleton + b.targetId = (int)bones.size(); b.realTargetId = b.targetId; } @@ -168,12 +170,15 @@ namespace } } -void Skeleton::LoadSkeletonDataFromXml(const char* text) +void errorHandler(void* ctx, const char* msg, ...); + +void Skeleton::LoadSkeletonDataFromXml(const char* xmlData, size_t xmlLength, std::string& xmlErrors) { xmlDoc* doc = NULL; try { - doc = xmlParseDoc(reinterpret_cast(text)); + xmlSetGenericErrorFunc(&xmlErrors, &errorHandler); + doc = xmlParseMemory(xmlData, xmlLength); if (doc) { xmlNode* root = xmlDocGetRootElement(doc); @@ -182,12 +187,17 @@ void Skeleton::LoadSkeletonDataFromXml(const char* text) doc = NULL; } xmlCleanupParser(); + xmlSetGenericErrorFunc(NULL, NULL); } catch (const ColladaException&) { if (doc) xmlFreeDoc(doc); xmlCleanupParser(); + xmlSetGenericErrorFunc(NULL, NULL); throw; } + + if (! xmlErrors.empty()) + throw ColladaException("XML parsing failed"); } diff --git a/source/collada/StdSkeletons.h b/source/collada/StdSkeletons.h index 90cfb9b557..f6b7adadfd 100644 --- a/source/collada/StdSkeletons.h +++ b/source/collada/StdSkeletons.h @@ -65,10 +65,12 @@ public: /** * Initialises the global state with skeleton data loaded from the * given XML data. Must only be called once. - * (TODO: don't do global state.) - * @throws ColladaException on failure. + * (TODO: stop using global state.) + * @param xmlErrors output - XML parser error messages; will be non-empty + * on failure (and failure will always throw) + * @throws ColladaException on failure */ - static void LoadSkeletonDataFromXml(const char* text); + static void LoadSkeletonDataFromXml(const char* xmlData, size_t xmlLength, std::string& xmlErrors); std::auto_ptr m; private: diff --git a/source/collada/tests/tests.py b/source/collada/tests/tests.py index fff63befa8..b389bd9a47 100644 --- a/source/collada/tests/tests.py +++ b/source/collada/tests/tests.py @@ -24,6 +24,8 @@ def log(severity, message): clog = CFUNCTYPE(None, c_int, c_char_p)(log) # (the CFUNCTYPE must not be GC'd, so try to keep a reference) library.set_logger(clog) +skeleton_definitions = open('%s/data/tools/collada/skeletons.xml' % binaries).read() +library.set_skeleton_definitions(skeleton_definitions, len(skeleton_definitions)) def _convert_dae(func, filename, expected_status=0): output = [] @@ -55,16 +57,22 @@ def clean_dir(path): except OSError: pass # (ignore errors if it already exists) -def create_actor(mesh, texture, anims): +def create_actor(mesh, texture, anims, props_): actor = ET.Element('actor', version='1') ET.SubElement(actor, 'castshadow') group = ET.SubElement(actor, 'group') variant = ET.SubElement(group, 'variant', frequency='100', name='Base') ET.SubElement(variant, 'mesh').text = mesh+'.pmd' ET.SubElement(variant, 'texture').text = texture+'.dds' + animations = ET.SubElement(variant, 'animations') for name, file in anims: ET.SubElement(animations, 'animation', file=file+'.psa', name=name, speed='100') + + props = ET.SubElement(variant, 'props') + for name, file in props_: + ET.SubElement(props, 'prop', actor=file+'.xml', attachpoint=name) + return ET.tostring(actor) def create_actor_static(mesh, texture): @@ -80,11 +88,10 @@ def create_actor_static(mesh, texture): # Error handling -convert_dae_to_pmd('This is not well-formed XML', expected_status=-2) - -convert_dae_to_pmd('This is not COLLADA', expected_status=-2) - -convert_dae_to_pmd('This is still not valid COLLADA', expected_status=-2) +if False: + convert_dae_to_pmd('This is not well-formed XML', expected_status=-2) + convert_dae_to_pmd('This is not COLLADA', expected_status=-2) + convert_dae_to_pmd('This is still not valid COLLADA', expected_status=-2) # Do some real conversions, so the output can be tested in the Actor Viewer @@ -95,7 +102,11 @@ clean_dir(test_mod + '/art/meshes') clean_dir(test_mod + '/art/actors') clean_dir(test_mod + '/art/animation') -for test_file in ['cube', 'jav2', 'jav2b', 'teapot_basic', 'teapot_skin', 'plane_skin', 'dude_skin', 'mergenonbone', 'densemesh']: +#for test_file in ['cube', 'jav2', 'jav2b', 'teapot_basic', 'teapot_skin', 'plane_skin', 'dude_skin', 'mergenonbone', 'densemesh']: +#for test_file in ['teapot_basic', 'jav2b', 'jav2d']: +for test_file in ['xsitest3c','xsitest3e','jav2d','jav2d2']: +#for test_file in ['xsitest3']: +#for test_file in []: print "* Converting PMD %s" % (test_file) input_filename = '%s/%s.dae' % (test_data, test_file) @@ -105,13 +116,15 @@ for test_file in ['cube', 'jav2', 'jav2b', 'teapot_basic', 'teapot_skin', 'plane output = convert_dae_to_pmd(input) open(output_filename, 'wb').write(output) - xml = create_actor(test_file, 'male', [('Idle','dudeidle'),('Corpse','dudecorpse'),('Melee','jav2b')]) + xml = create_actor(test_file, 'male', [('Idle','dudeidle'),('Corpse','dudecorpse'),('attack1',test_file),('attack2','jav2d')], [('helmet','teapot_basic_static')]) open('%s/art/actors/%s.xml' % (test_mod, test_file), 'w').write(xml) xml = create_actor_static(test_file, 'male') open('%s/art/actors/%s_static.xml' % (test_mod, test_file), 'w').write(xml) -for test_file in ['jav2b']: +#for test_file in ['jav2','jav2b', 'jav2d']: +for test_file in ['xsitest3c','xsitest3e','jav2d','jav2d2']: +#for test_file in []: print "* Converting PSA %s" % (test_file) input_filename = '%s/%s.dae' % (test_data, test_file) diff --git a/source/graphics/ColladaManager.cpp b/source/graphics/ColladaManager.cpp index 0b27265375..91fb872074 100644 --- a/source/graphics/ColladaManager.cpp +++ b/source/graphics/ColladaManager.cpp @@ -49,6 +49,7 @@ class CColladaManagerImpl DllLoader dll; void (*set_logger)(Collada::LogFn logger); + int (*set_skeleton_definitions)(const char* xml, int length); int (*convert_dae_to_pmd)(const char* dae, Collada::OutputFn pmd_writer, void* cb_data); int (*convert_dae_to_psa)(const char* dae, Collada::OutputFn psa_writer, void* cb_data); @@ -83,6 +84,7 @@ public: try { dll.LoadSymbol("set_logger", set_logger); + dll.LoadSymbol("set_skeleton_definitions", set_skeleton_definitions); dll.LoadSymbol("convert_dae_to_pmd", convert_dae_to_pmd); dll.LoadSymbol("convert_dae_to_psa", convert_dae_to_psa); } @@ -94,6 +96,26 @@ public: } set_logger(ColladaLog); + + CVFSFile skeletonFile; + if (skeletonFile.Load("art/skeletons/skeletons.xml") != PSRETURN_OK) + { + LOG(ERROR, "collada", "Failed to read skeleton definitions"); + dll.Unload(); + return false; + } + + int ok = set_skeleton_definitions((const char*)skeletonFile.GetBuffer(), (int)skeletonFile.GetBufferSize()); + if (ok < 0) + { + LOG(ERROR, "collada", "Failed to load skeleton definitions"); + dll.Unload(); + return false; + } + + // TODO: the cached PMD/PSA files should probably be invalidated when + // the skeleton definition file is changed, else people will get confused + // as to why it's not picking up their changes } // We need to null-terminate the buffer, so do it (possibly inefficiently) @@ -142,7 +164,7 @@ CColladaManager::~CColladaManager() delete m; } -CStr CColladaManager::GetLoadableFilename(const CStr &sourceName, FileType type) +CStr CColladaManager::GetLoadableFilename(const CStr& sourceName, FileType type) { const char* extn = NULL; switch (type) diff --git a/source/graphics/tests/test_MeshManager.h b/source/graphics/tests/test_MeshManager.h index 4b69a0a5af..07f054d13e 100644 --- a/source/graphics/tests/test_MeshManager.h +++ b/source/graphics/tests/test_MeshManager.h @@ -10,6 +10,8 @@ #include "graphics/MeshManager.h" #include "graphics/ModelDef.h" +#include "ps/CLogger.h" + #define MOD_PATH "mods/_test.mesh" #define CACHE_PATH "_testcache" @@ -19,6 +21,9 @@ const char* testDAE = "art/meshes/skeletal/test.dae"; const char* testPMD = "art/meshes/skeletal/test.pmd"; const char* testBase = "art/meshes/skeletal/test"; +const char* srcSkeletonDefs = "tests/collada/skeletons.xml"; +const char* testSkeletonDefs = "art/skeletons/skeletons.xml"; + class TestMeshManager : public CxxTest::TestSuite { void initVfs() @@ -79,11 +84,11 @@ class TestMeshManager : public CxxTest::TestSuite FileIOBuf buf = FILE_BUF_ALLOC; ssize_t read = file_io(&f, 0, f.size, &buf); TS_ASSERT_EQUALS(read, f.size); + file_close(&f); vfs_store(dst, buf, read, FILE_NO_AIO); file_buf_free(buf); - file_close(&f); } void buildArchive() @@ -160,7 +165,13 @@ public: void test_load_dae() { + // TODO: I get + // Assertion failed: "buf_in_cache == buf" + // Location: file_cache.cpp:1094 (file_buf_free) + // when the order of these is swapped... + copyFile(srcDAE, testDAE); + copyFile(srcSkeletonDefs, testSkeletonDefs); CModelDefPtr modeldef = meshManager->GetMesh(testDAE); TS_ASSERT(modeldef); @@ -170,13 +181,41 @@ public: void test_load_dae_caching() { copyFile(srcDAE, testDAE); + copyFile(srcSkeletonDefs, testSkeletonDefs); CStr daeName1 = colladaManager->GetLoadableFilename(testBase, CColladaManager::PMD); CStr daeName2 = colladaManager->GetLoadableFilename(testBase, CColladaManager::PMD); TS_ASSERT(daeName1.length()); TS_ASSERT_STR_EQUALS(daeName1, daeName2); - // TODO: it'd be nice to test that it isn't doing the DAE->PMD conversion - // again, but there doesn't seem to be an easy way to check that + // TODO: it'd be nice to test that it really isn't doing the DAE->PMD + // conversion a second time, but there doesn't seem to be an easy way + // to check that + } + + void test_invalid_skeletons() + { + TestLogger logger; + + copyFile(srcDAE, testDAE); + const char text[] = "Not valid XML"; + vfs_store(testSkeletonDefs, text, strlen(text), FILE_NO_AIO); + + CModelDefPtr modeldef = meshManager->GetMesh(testDAE); + TS_ASSERT(! modeldef); + TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "parser error"); + } + + void test_invalid_dae() + { + TestLogger logger; + + copyFile(srcSkeletonDefs, testSkeletonDefs); + const char text[] = "Not valid XML"; + vfs_store(testDAE, text, strlen(text), FILE_NO_AIO); + + CModelDefPtr modeldef = meshManager->GetMesh(testDAE); + TS_ASSERT(! modeldef); + TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "parser error"); } void test_load_nonexistent_pmd() diff --git a/source/lib/self_test.h b/source/lib/self_test.h index d1953bf47b..7100d6ec2a 100644 --- a/source/lib/self_test.h +++ b/source/lib/self_test.h @@ -217,6 +217,12 @@ namespace CxxTest #define TS_ASSERT_STR_EQUALS(str1, str2) TS_ASSERT_EQUALS(std::string(str1), std::string(str2)) #define TS_ASSERT_WSTR_EQUALS(str1, str2) TS_ASSERT_EQUALS(std::wstring(str1), std::wstring(str2)) +static bool ts_str_contains(const std::string& str1, const std::string& str2) +{ + return str1.find(str2) != str1.npos; +} +#define TS_ASSERT_STR_CONTAINS(str1, str2) TS_ASSERT(ts_str_contains(str1, str2)) + template std::vector ts_make_vector(T* start, size_t size_bytes) { diff --git a/source/ps/CLogger.cpp b/source/ps/CLogger.cpp index 84f4373e1a..8aa0b7150a 100644 --- a/source/ps/CLogger.cpp +++ b/source/ps/CLogger.cpp @@ -232,3 +232,21 @@ int CLogger::Interestedness(const char* category) return level; } + + +TestLogger::TestLogger() +{ + m_OldLogger = g_Logger; + g_Logger = new CLogger(&m_Stream, &blackHoleStream, false); +} + +TestLogger::~TestLogger() +{ + delete g_Logger; + g_Logger = m_OldLogger; +} + +std::string TestLogger::GetOutput() +{ + return m_Stream.str(); +} diff --git a/source/ps/CLogger.h b/source/ps/CLogger.h index 13dd0bf91a..aa98b00364 100644 --- a/source/ps/CLogger.h +++ b/source/ps/CLogger.h @@ -65,4 +65,19 @@ private: std::set m_LoggedOnce; }; +/** + * Helper class for unit tests - captures all log output while it is in scope, + * and returns it as a single string. + */ +class TestLogger +{ +public: + TestLogger(); + ~TestLogger(); + std::string GetOutput(); +private: + CLogger* m_OldLogger; + std::stringstream m_Stream; +}; + #endif diff --git a/source/tools/autobuild/build.pl b/source/tools/autobuild/build.pl index a71bb9c9e5..9fef3c57de 100644 --- a/source/tools/autobuild/build.pl +++ b/source/tools/autobuild/build.pl @@ -81,7 +81,7 @@ allow_abort(); $svn_output =~ /^(?:Updated to|At) revision (\d+)\.$/m or die; my $svn_revision = $1; -if ($svn_output =~ m~^. (source(?![/\\]tools(?![/\\]atlas[/\\]GameInterface))|build|libraries)~m) +if ($svn_output =~ m~^. (source(?![/\\](tools(?![/\\]atlas[/\\]GameInterface)|collada))|build|libraries)~m) { # The source has been updated. # ('source' means something in the source, build, or libraries directories, excluding source/tools, but including source/tools/atlas/GameInterface)