2019-08-21 11:50 CEST

View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0006181Spring engineGeneralpublic2019-03-25 20:52
ReporterMaDDoX 
Assigned ToKloot 
PrioritynormalSeveritymajorReproducibilityalways
StatusresolvedResolutionfixed 
Product Version104.0 +git 
Target VersionFixed in Version104.0 +git 
Summary0006181: ASSIMP collada importer using id property instead of name
DescriptionSince a relatively recent update, Spring has broken compatibility with most 3D packages collada exporters. Instead of using the 'name' property of mesh nodes, as it did previously, it's now using the 'id' property as the object element identifier, to bind pieces in the script. In many .dae exporters these properties don't use the same name. OpenCollada, for instance, prefixes the name with underscores and the "parenting path" to that object. This way, when you try to run the animation in Spring it breaks, since it can't find the piece.
I've discussed the issue with Ivand, who pointed me to the fact that when opening the model in Blender and saving it again, the problem was resolved. Blender always copies the object name to the id property. The problem of using Blender as an extra step for that is that it flattens the pivot points of the original .dae file. There's probably a way to prevent Blender from doing it, but there's no reason why the importer shouldn't be compatible with .dae files exported from other 3D packages.
Steps To Reproduce1. Open .dae model exported from Modo or Maya's collada (check the attached file, just rename extension to .dae) with an animation script in Spring
2. Update unitdef .lua (armlab.lua in this case) to point to the new object
2. Start an action so that the script is played (pivots are wrong but the original armlab build animation should work anyways)
3. Spring errors out saying the piece wasn't found
TagsNo tags attached.
Checked infolog.txt for lua ErrorsYes
Attached Files

-Relationships
+Relationships

-Notes

~0019885

Kloot (developer)

No such change happened (https://github.com/spring/spring/blob/maintenance/rts/Rendering/Models/AssParser.cpp#L358), the assimp aiNode struct (https://github.com/spring/spring/blob/maintenance/rts/lib/assimp/include/assimp/scene.h#L78) does not even have an 'id' property for Spring to read.

Please find out which update caused this (installers going back to 902-g2b68b17 are here: https://springrts.com/dl/buildbot/default/maintenance/) and attach an infolog.

~0019887

MaDDoX (reporter)

I've talked to Ivand about it again today, and it might be more complicated than we've originally thought, or something else altogether. The commit he was thinking about was https://github.com/spring/spring/commit/7abfc54476d3379009cad56e366751d4686946fc#diff-23155ead32fea642a88d279effe0b9f8 . Anyways, I've done a simple repro scenario, should help us pinpoint what's going on. By the way, any .dae saved in Modo or Maya's OpenCollada did work in Spring 103, the issue has only arisen in 104+.

1. Clone the TAPrime repository as an .sdd from github.com/fluidplay/TAPrime
2. Start a game, enable cheats and enter /give supersimple
3. If units/other/supersimple.lua has objectname = "supersimpleblender.dae", it works fine, no error and its simple script's animation works.
4. If objectname there = "supersimplemodo.dae", it errors out. From infolog.txt:

[f=0000256] [GiveUnits] spawned 1 supersimple unit(s) for team 0
[f=0000256] [Game::ClientReadNet][LOGMSG] sender="Player" string="[Internal Lua error: Call failure] [string "scripts/supersimple.dae.lua"]:6: piece not found: base

I've tried editing and renaming the ids in Modo's .dae (the asset was originally created in Modo) and it still wouldn't find the piece. So I'm out of ideas here.. :(

~0019888

Kloot (developer)

Last edited: 2019-03-25 02:56

View 4 revisions

supersimpleblender.dae has this hierarchy:
  piece name "Scene" corresponding to <visual_scene id="Scene" name="Scene">
    piece name "base" corresponding to <geometry id="base-mesh" name="base">
      piece name "tip" corresponding to <geometry id="tip-mesh" name="tip">

supersimplemodo.dae instead has the following hierarchy:
  piece name "DefaultScene" corresponding to <visual_scene id="DefaultScene">
    piece name "polyRender006" corresponding to <node id="polyRender006" name="Render" type="NODE">
    piece name "Geometry-mesh002Node" corresponding to <geometry id="Geometry-mesh002" name="base" type="NODE">
      piece name "Geometry-mesh023Node" corresponding to <geometry id="Geometry-mesh023" name="tip" type="NODE">


In supersimpleblender.dae the entries under <library_visual_scenes> have matching id and base field names, whereas in supersimplemodo.dae they differ. Manually changing
  <node id="Geometry-mesh002Node" name="base" type="NODE">
  <node id="Geometry-mesh023Node" name="tip" type="NODE">
to
  <node id="base" name="base" type="NODE">
  <node id="tip" name="tip" type="NODE">

in modo.dae fixes the script errors, so the assimp library update (f87dae2f) between 103.0 and 104.0 would have to be responsible for names now getting extracted from id fields. assimp's ColladaLoader.cpp backs this up:


  aiNode* ColladaLoader::BuildHierarchy( const ColladaParser& pParser, const Collada::Node* pNode)
  {
    ...
    node->mName.Set( FindNameForNode( pNode));
    ...
  }

  ...

  std::string ColladaLoader::FindNameForNode( const Collada::Node* pNode)
  {
    // Now setup the name of the assimp node. The collada name might not be
    // unique, so we use the collada ID.
    if (!pNode->mID.empty())
      return pNode->mID;
    else if (!pNode->mSID.empty())
      return pNode->mSID;
    else
    {
      // No need to worry. Unnamed nodes are no problem at all, except
      // if cameras or lights need to be assigned to them.
      return format() << "$ColladaAutoName$_" << mNodeNameCounter++;
    }
  }

Your choices are to either stick with a single exporter and adjust your script piece names to the convention used by it, or simply post-process the <node> elements. The latter would be a few lines of Python code.

~0019889

MaDDoX (reporter)

Oh, you found it! True, not hard to fix through scripting, but Spring would still not be out-of-the-box compatible with a multitude of collada files, and potentially bring confusion to new modders. Would a "usenamesforids=true," option in the assimp lua config (eg.: supersimplemodo.dae.lua) be too much to ask for?

In any case, thanks for the detective work Kloot, highly appreciated!

~0019890

Kloot (developer)

Last edited: 2019-03-25 12:50

View 2 revisions

Your request, while understandable, entails hacking assimp library code (and breaking the Collada spec, note that Blender is actually in violation of it by copying names to id's) which may have... unforeseen consequences. I would much rather solve this on the exporter side somehow.

edit: preprocessing .dae's in-memory before handing them over to assimp's loader might be an option, I'll check that later.

~0019891

Kloot (developer)

done, just add "nodenamesfromids = true" to your model metadata scripts and update to 1153-gfb53422
+Notes

-Issue History
Date Modified Username Field Change
2019-03-22 20:18 MaDDoX New Issue
2019-03-22 20:18 MaDDoX File Added: armlab.daeopencollada
2019-03-22 21:20 Kloot Note Added: 0019885
2019-03-23 03:32 Kloot Status new => feedback
2019-03-25 01:08 MaDDoX Note Added: 0019887
2019-03-25 01:08 MaDDoX Status feedback => new
2019-03-25 02:36 Kloot Note Added: 0019888
2019-03-25 02:47 Kloot Note Edited: 0019888 View Revisions
2019-03-25 02:53 Kloot Note Edited: 0019888 View Revisions
2019-03-25 02:53 Kloot Status new => feedback
2019-03-25 02:56 Kloot Note Edited: 0019888 View Revisions
2019-03-25 05:49 MaDDoX Note Added: 0019889
2019-03-25 05:49 MaDDoX Status feedback => new
2019-03-25 12:43 Kloot Note Added: 0019890
2019-03-25 12:50 Kloot Note Edited: 0019890 View Revisions
2019-03-25 20:52 Kloot Assigned To => Kloot
2019-03-25 20:52 Kloot Status new => resolved
2019-03-25 20:52 Kloot Resolution open => fixed
2019-03-25 20:52 Kloot Fixed in Version => 104.0 +git
2019-03-25 20:52 Kloot Note Added: 0019891
+Issue History