From 2d3540b73fed936a34b0c08dd398f2428552204b Mon Sep 17 00:00:00 2001
From: Vsevolod Kremianskii <vkremianskii@gmail.com>
Date: Sat, 15 May 2021 15:13:38 +0700
Subject: [PATCH] Exclusively use node names to bind animations to model nodes

This both makes for a cleaner API and fixes stunt animations.
---
 .../game/object/creature_appearance.cpp       | 15 +++---
 src/engine/graphics/model/animation.cpp       |  9 +---
 src/engine/graphics/model/animation.h         |  4 +-
 src/engine/graphics/model/mdlreader.cpp       |  9 ++--
 src/engine/graphics/model/mdlreader.h         |  2 +-
 .../graphics/model/mdlreader_controllers.cpp  | 49 +++++++------------
 src/engine/graphics/model/model.cpp           | 27 +++++-----
 src/engine/graphics/model/model.h             |  4 +-
 src/engine/graphics/model/modelnode.cpp       |  2 -
 src/engine/graphics/model/modelnode.h         |  7 +--
 src/engine/gui/scenebuilder.cpp               |  5 +-
 src/engine/scene/node/lightnode.cpp           |  6 +--
 src/engine/scene/node/meshnode.cpp            |  8 +--
 src/engine/scene/node/modelnode.cpp           | 34 +++++--------
 src/engine/scene/node/modelnode.h             | 12 ++---
 src/engine/scene/node/modelnode_animation.cpp | 28 +++++------
 16 files changed, 83 insertions(+), 138 deletions(-)

diff --git a/src/engine/game/object/creature_appearance.cpp b/src/engine/game/object/creature_appearance.cpp
index b6bcf6b1..e9f07013 100644
--- a/src/engine/game/object/creature_appearance.cpp
+++ b/src/engine/game/object/creature_appearance.cpp
@@ -78,15 +78,12 @@ shared_ptr<ModelSceneNode> Creature::buildModel() {
     if (!headModelName.empty()) {
         shared_ptr<Model> headModel(Models::instance().get(headModelName));
         if (headModel) {
-            shared_ptr<ModelNode> headHook(bodyModel->getNodeByName(g_headHookNode));
-            if (headHook) {
-                auto headSceneNode = make_shared<ModelSceneNode>(headModel, ModelUsage::Creature, _sceneGraph, this);
-                headSceneNode->setInanimateNodes(bodyModel->getAncestorNodes(headHook->id()));
-                bodySceneNode->attach(headHook->id(), headSceneNode);
-                if (maskModel) {
-                    auto maskSceneNode = make_shared<ModelSceneNode>(maskModel, ModelUsage::Equipment, _sceneGraph, this);
-                    headSceneNode->attach(g_maskHookNode, maskSceneNode);
-                }
+            auto headSceneNode = make_shared<ModelSceneNode>(headModel, ModelUsage::Creature, _sceneGraph, this);
+            headSceneNode->setInanimateNodes(bodyModel->getAncestorNodes(g_headHookNode));
+            bodySceneNode->attach(g_headHookNode, headSceneNode);
+            if (maskModel) {
+                auto maskSceneNode = make_shared<ModelSceneNode>(maskModel, ModelUsage::Equipment, _sceneGraph, this);
+                headSceneNode->attach(g_maskHookNode, maskSceneNode);
             }
         }
     }
diff --git a/src/engine/graphics/model/animation.cpp b/src/engine/graphics/model/animation.cpp
index f20c8e87..252b7052 100644
--- a/src/engine/graphics/model/animation.cpp
+++ b/src/engine/graphics/model/animation.cpp
@@ -40,10 +40,10 @@ Animation::Animation(
     _rootNode(move(rootNode)),
     _events(move(events)) {
 
-    fillNodeByName();
+    fillNodeLookups();
 }
 
-void Animation::fillNodeByName() {
+void Animation::fillNodeLookups() {
     stack<shared_ptr<ModelNode>> nodes;
     nodes.push(_rootNode);
 
@@ -51,7 +51,6 @@ void Animation::fillNodeByName() {
         shared_ptr<ModelNode> node(nodes.top());
         nodes.pop();
 
-        _nodeById.insert(make_pair(node->id(), node));
         _nodeByName.insert(make_pair(node->name(), node));
 
         for (auto &child : node->children()) {
@@ -60,10 +59,6 @@ void Animation::fillNodeByName() {
     }
 }
 
-shared_ptr<ModelNode> Animation::getNodeById(uint16_t nodeId) const {
-    return getFromLookupOrNull(_nodeById, nodeId);
-}
-
 shared_ptr<ModelNode> Animation::getNodeByName(const string &name) const {
     return getFromLookupOrNull(_nodeByName, name);
 }
diff --git a/src/engine/graphics/model/animation.h b/src/engine/graphics/model/animation.h
index 447b28d2..e415b994 100644
--- a/src/engine/graphics/model/animation.h
+++ b/src/engine/graphics/model/animation.h
@@ -41,7 +41,6 @@ public:
         std::shared_ptr<ModelNode> rootNode,
         std::vector<Event> events);
 
-    std::shared_ptr<ModelNode> getNodeById(uint16_t nodeId) const;
     std::shared_ptr<ModelNode> getNodeByName(const std::string &name) const;
 
     const std::string &name() const { return _name; }
@@ -57,10 +56,9 @@ private:
     std::shared_ptr<ModelNode> _rootNode;
     std::vector<Event> _events;
 
-    std::unordered_map<uint16_t, std::shared_ptr<ModelNode>> _nodeById;
     std::unordered_map<std::string, std::shared_ptr<ModelNode>> _nodeByName;
 
-    void fillNodeByName();
+    void fillNodeLookups();
 };
 
 } // namespace graphics
diff --git a/src/engine/graphics/model/mdlreader.cpp b/src/engine/graphics/model/mdlreader.cpp
index 44a7dcf2..b12b4831 100644
--- a/src/engine/graphics/model/mdlreader.cpp
+++ b/src/engine/graphics/model/mdlreader.cpp
@@ -179,15 +179,14 @@ unique_ptr<ModelNode> MdlReader::readNode(uint32_t offset, const ModelNode *pare
     if (flags & 0xf408) {
         throw runtime_error("Unsupported MDL node flags: " + to_string(flags));
     }
-    if (!anim) {
-        _nodeFlags.insert(make_pair(nodeId, flags));
-    }
     string name(_nodeNames[nameIndex]);
+    if (!anim) {
+        _nodeFlags.insert(make_pair(name, flags));
+    }
     glm::vec3 restPosition(glm::make_vec3(&positionValues[0]));
     glm::quat restOrientation(orientationValues[0], orientationValues[1], orientationValues[2], orientationValues[3]);
 
     auto node = make_unique<ModelNode>(
-        nodeId,
         move(name),
         move(restPosition),
         move(restOrientation),
@@ -649,7 +648,7 @@ shared_ptr<ModelNode::Reference> MdlReader::readReference() {
 void MdlReader::readControllers(uint32_t keyOffset, uint32_t keyCount, const vector<float> &data, bool anim, ModelNode &node) {
     uint16_t nodeFlags;
     if (anim) {
-        nodeFlags = _nodeFlags.find(node.id())->second;
+        nodeFlags = _nodeFlags.find(node.name())->second;
     } else {
         nodeFlags = node.flags();
     }
diff --git a/src/engine/graphics/model/mdlreader.h b/src/engine/graphics/model/mdlreader.h
index aa7633e3..bd8b4dc8 100644
--- a/src/engine/graphics/model/mdlreader.h
+++ b/src/engine/graphics/model/mdlreader.h
@@ -77,7 +77,7 @@ private:
     std::unique_ptr<StreamReader> _mdxReader;
     bool _tsl { false }; /**< is this a TSL model? */
     std::vector<std::string> _nodeNames;
-    std::map<uint16_t, uint16_t> _nodeFlags;
+    std::map<std::string, uint16_t> _nodeFlags;
     std::shared_ptr<graphics::Model> _model;
 
     void doLoad() override;
diff --git a/src/engine/graphics/model/mdlreader_controllers.cpp b/src/engine/graphics/model/mdlreader_controllers.cpp
index ba2d1d81..4141cf0d 100644
--- a/src/engine/graphics/model/mdlreader_controllers.cpp
+++ b/src/engine/graphics/model/mdlreader_controllers.cpp
@@ -124,19 +124,31 @@ static inline void ensureNumColumnsEquals(int type, int expected, int actual) {
     }
 }
 
-void MdlReader::readPositionController(const ControllerKey &key, const vector<float> &data, ModelNode &node) {
+void MdlReader::readFloatController(const ControllerKey &key, const vector<float> &data, AnimatedProperty<float> &prop) {
+    ensureNumColumnsEquals(key.type, 1, key.numColumns);
+    for (uint16_t i = 0; i < key.numRows; ++i) {
+        float time = data[key.timeIndex + i];
+        float value = data[key.dataIndex + i];
+        prop.addFrame(time, value);
+    }
+    prop.update();
+}
+
+void MdlReader::readVectorController(const ControllerKey &key, const vector<float> &data, AnimatedProperty<glm::vec3> &prop) {
     bool bezier = key.numColumns & kFlagBezier;
     int numColumns = key.numColumns & ~kFlagBezier;
     ensureNumColumnsEquals(key.type, 3, numColumns);
 
     for (uint16_t i = 0; i < key.numRows; ++i) {
-        int rowTimeIdx = key.timeIndex + i;
-        int rowDataIdx = key.dataIndex + (bezier ? 9 : 3) * i;
-        float time = data[rowTimeIdx];
-        glm::vec3 position(glm::make_vec3(&data[rowDataIdx]));
-        node.position().addFrame(time, move(position));
+        float time = data[key.timeIndex + i];
+        glm::vec3 value(glm::make_vec3(&data[key.dataIndex + (bezier ? 9 : 3) * i]));
+        prop.addFrame(time, value);
     }
-    node.position().update();
+    prop.update();
+}
+
+void MdlReader::readPositionController(const ControllerKey &key, const vector<float> &data, ModelNode &node) {
+    readVectorController(key, data, node.position());
 }
 
 void MdlReader::readOrientationController(const ControllerKey &key, const vector<float> &data, ModelNode &node) {
@@ -191,29 +203,6 @@ void MdlReader::readOrientationController(const ControllerKey &key, const vector
     node.orientation().update();
 }
 
-void MdlReader::readFloatController(const ControllerKey &key, const vector<float> &data, AnimatedProperty<float> &prop) {
-    ensureNumColumnsEquals(key.type, 1, key.numColumns);
-    for (uint16_t i = 0; i < key.numRows; ++i) {
-        float time = data[key.timeIndex + i];
-        float value = data[key.dataIndex + i];
-        prop.addFrame(time, value);
-    }
-    prop.update();
-}
-
-void MdlReader::readVectorController(const ControllerKey &key, const vector<float> &data, AnimatedProperty<glm::vec3> &prop) {
-    bool bezier = key.numColumns & kFlagBezier;
-    int numColumns = key.numColumns & ~kFlagBezier;
-    ensureNumColumnsEquals(key.type, 3, numColumns);
-
-    for (uint16_t i = 0; i < key.numRows; ++i) {
-        float time = data[key.timeIndex + i];
-        glm::vec3 value(glm::make_vec3(&data[key.dataIndex + (bezier ? 9 : 3) * i]));
-        prop.addFrame(time, value);
-    }
-    prop.update();
-}
-
 void MdlReader::readScaleController(const ControllerKey &key, const vector<float> &data, ModelNode &node) {
     readFloatController(key, data, node.scale());
 }
diff --git a/src/engine/graphics/model/model.cpp b/src/engine/graphics/model/model.cpp
index 89734a3f..1647a028 100644
--- a/src/engine/graphics/model/model.cpp
+++ b/src/engine/graphics/model/model.cpp
@@ -52,7 +52,6 @@ Model::Model(
 
 void Model::fillNodeLookups(const shared_ptr<ModelNode> &node) {
     _nodes.push_back(node);
-    _nodeById.insert(make_pair(node->id(), node));
     _nodeByName.insert(make_pair(node->name(), node));
 
     for (auto &child : node->children()) {
@@ -62,20 +61,20 @@ void Model::fillNodeLookups(const shared_ptr<ModelNode> &node) {
 
 void Model::fillBoneNodeId() {
     // In MDL files, bones reference node serial numbers (DFS ordering).
-    // We want them to reference node identifiers, for simplicity.
+    // We want them to reference node names instead.
 
     for (auto &node : _nodes) {
         if (!node->isSkinMesh()) continue;
 
         shared_ptr<ModelNode::TriangleMesh> mesh(node->mesh());
-        mesh->skin->boneNodeId.resize(mesh->skin->boneNodeSerial.size());
+        mesh->skin->boneNodeName.resize(mesh->skin->boneNodeSerial.size());
 
         for (size_t i = 0; i < mesh->skin->boneNodeSerial.size(); ++i) {
             uint16_t nodeSerial = mesh->skin->boneNodeSerial[i];
             if (nodeSerial < static_cast<int>(_nodes.size())) {
-                mesh->skin->boneNodeId[i] = _nodes[nodeSerial]->id();
+                mesh->skin->boneNodeName[i] = _nodes[nodeSerial]->name();
             } else {
-                mesh->skin->boneNodeId[i] = 0xffff;
+                mesh->skin->boneNodeName[i].clear();
             }
         }
     }
@@ -84,7 +83,7 @@ void Model::fillBoneNodeId() {
 void Model::computeAABB() {
     _aabb.reset();
 
-    for (auto &node : _nodeById) {
+    for (auto &node : _nodeByName) {
         shared_ptr<ModelNode::TriangleMesh> mesh(node.second->mesh());
         if (mesh) {
             _aabb.expand(mesh->mesh->aabb() * node.second->absoluteTransform());
@@ -100,10 +99,6 @@ void Model::addAnimation(shared_ptr<Animation> animation) {
     _animations.insert(make_pair(animation->name(), move(animation)));
 }
 
-shared_ptr<ModelNode> Model::getNodeById(uint16_t nodeId) const {
-    return getFromLookupOrNull(_nodeById, nodeId);
-}
-
 shared_ptr<ModelNode> Model::getNodeByName(const string &name) const {
     return getFromLookupOrNull(_nodeByName, name);
 }
@@ -117,7 +112,7 @@ shared_ptr<ModelNode> Model::getNodeByNameRecursive(const string &name) const {
 }
 
 shared_ptr<ModelNode> Model::getAABBNode() const {
-    for (auto &node : _nodeById) {
+    for (auto &node : _nodeByName) {
         if (node.second->isAABBMesh()) return node.second;
     }
     return nullptr;
@@ -151,13 +146,13 @@ vector<string> Model::getAnimationNames() const {
     return move(result);
 }
 
-set<uint16_t> Model::getAncestorNodes(uint16_t parentId) const {
-    set<uint16_t> result;
+set<string> Model::getAncestorNodes(const string &parentName) const {
+    set<string> result;
 
-    auto maybeParent = _nodeById.find(parentId);
-    if (maybeParent != _nodeById.end()) {
+    auto maybeParent = _nodeByName.find(parentName);
+    if (maybeParent != _nodeByName.end()) {
         for (const ModelNode *node = maybeParent->second->parent(); node; node = node->parent()) {
-            result.insert(node->id());
+            result.insert(node->name());
         }
     }
 
diff --git a/src/engine/graphics/model/model.h b/src/engine/graphics/model/model.h
index eb56e427..2d3e0461 100644
--- a/src/engine/graphics/model/model.h
+++ b/src/engine/graphics/model/model.h
@@ -64,13 +64,12 @@ public:
 
     bool isAffectedByFog() const { return _affectedByFog; }
 
-    std::shared_ptr<ModelNode> getNodeById(uint16_t nodeId) const;
     std::shared_ptr<ModelNode> getNodeByName(const std::string &name) const;
     std::shared_ptr<ModelNode> getNodeByNameRecursive(const std::string &name) const;
     std::shared_ptr<ModelNode> getAABBNode() const;
     std::shared_ptr<Animation> getAnimation(const std::string &name) const;
     std::vector<std::string> getAnimationNames() const;
-    std::set<uint16_t> getAncestorNodes(uint16_t parentId) const;
+    std::set<std::string> getAncestorNodes(const std::string &parentName) const;
 
     const std::string &name() const { return _name; }
     Classification classification() const { return _classification; }
@@ -91,7 +90,6 @@ private:
     bool _affectedByFog;
 
     std::vector<std::shared_ptr<ModelNode>> _nodes;
-    std::unordered_map<uint16_t, std::shared_ptr<ModelNode>> _nodeById;
     std::unordered_map<std::string, std::shared_ptr<ModelNode>> _nodeByName;
     AABB _aabb;
 
diff --git a/src/engine/graphics/model/modelnode.cpp b/src/engine/graphics/model/modelnode.cpp
index a6bbff68..8cb97faf 100644
--- a/src/engine/graphics/model/modelnode.cpp
+++ b/src/engine/graphics/model/modelnode.cpp
@@ -28,13 +28,11 @@ namespace reone {
 namespace graphics {
 
 ModelNode::ModelNode(
-    uint16_t id,
     string name,
     glm::vec3 restPosition,
     glm::quat restOrientation,
     const ModelNode *parent
 ) :
-    _id(id),
     _name(move(name)),
     _restPosition(move(restPosition)),
     _restOrientation(move(restOrientation)),
diff --git a/src/engine/graphics/model/modelnode.h b/src/engine/graphics/model/modelnode.h
index 7dc32ee4..f62ac220 100644
--- a/src/engine/graphics/model/modelnode.h
+++ b/src/engine/graphics/model/modelnode.h
@@ -45,8 +45,8 @@ class Model;
 class ModelNode : boost::noncopyable {
 public:
     struct Skin {
-        std::vector<uint16_t> boneNodeSerial; /**< temporary, used to fill boneNodeId below */
-        std::vector<uint16_t> boneNodeId; /**< used in skeletal animation */
+        std::vector<std::string> boneNodeName;
+        std::vector<uint16_t> boneNodeSerial; /**< temporary, used to fill boneNodeName above */
     };
 
     struct UVAnimation {
@@ -171,7 +171,6 @@ public:
     };
 
     ModelNode(
-        uint16_t id,
         std::string name,
         glm::vec3 restPosition,
         glm::quat restOrientation,
@@ -183,7 +182,6 @@ public:
 
     std::vector<uint32_t> getFacesByMaterial(uint32_t material) const;
 
-    uint16_t id() const { return _id; }
     const std::string &name() const { return _name; }
     uint16_t flags() const { return _flags; }
     const ModelNode *parent() const { return _parent; }
@@ -297,7 +295,6 @@ public:
     // END Keyframes
 
 private:
-    uint16_t _id; /**< node identifier, matches id within supermodel */
     std::string _name;
     const ModelNode *_parent;
 
diff --git a/src/engine/gui/scenebuilder.cpp b/src/engine/gui/scenebuilder.cpp
index ab1ee096..b8f7c2ce 100644
--- a/src/engine/gui/scenebuilder.cpp
+++ b/src/engine/gui/scenebuilder.cpp
@@ -57,10 +57,7 @@ unique_ptr<SceneGraph> SceneBuilder::build() {
     } else {
         shared_ptr<ModelNode> refModelNode(model->model()->getNodeByName(_cameraNodeName));
         if (refModelNode) {
-            shared_ptr<ModelNodeSceneNode> refSceneNode(model->getNodeById(refModelNode->id()));
-            if (refSceneNode) {
-                camera->setLocalTransform(refModelNode->absoluteTransform() * _cameraTransform);
-            }
+            camera->setLocalTransform(refModelNode->absoluteTransform() * _cameraTransform);
         }
     }
 
diff --git a/src/engine/scene/node/lightnode.cpp b/src/engine/scene/node/lightnode.cpp
index 53635fed..79c3f135 100644
--- a/src/engine/scene/node/lightnode.cpp
+++ b/src/engine/scene/node/lightnode.cpp
@@ -47,9 +47,9 @@ LightSceneNode::LightSceneNode(const ModelSceneNode *model, shared_ptr<ModelNode
     if (!model) {
         throw invalid_argument("model must not be null");
     }
-    _radius = modelNode->radius().getByFrameOrElse(0, 1.0f);
-    _multiplier = modelNode->multiplier().getByFrameOrElse(0, 1.0f);
-    _color = modelNode->color().getByFrameOrElse(0, glm::vec3(1.0f));
+    _radius = modelNode->radius().getByFrameOrElse(0, 0.0f);
+    _multiplier = modelNode->multiplier().getByFrameOrElse(0, 0.0f);
+    _color = modelNode->color().getByFrameOrElse(0, glm::vec3(0.0f));
 }
 
 void LightSceneNode::drawLensFlares(const ModelNode::LensFlare &flare) {
diff --git a/src/engine/scene/node/meshnode.cpp b/src/engine/scene/node/meshnode.cpp
index db3c6145..e458a7ff 100644
--- a/src/engine/scene/node/meshnode.cpp
+++ b/src/engine/scene/node/meshnode.cpp
@@ -293,10 +293,10 @@ void MeshSceneNode::drawSingle(bool shadowPass) {
 
             // Offset bone matrices by 1 to account for negative bone indices
             uniforms.skeletal->bones[0] = glm::mat4(1.0f);
-            for (size_t i = 1; i < 1 + mesh->skin->boneNodeId.size() && i < kMaxBones; ++i) {
-                uint16_t nodeId = mesh->skin->boneNodeId[i - 1];
-                if (nodeId != 0xffff) {
-                    shared_ptr<ModelNodeSceneNode> bone(_model->getNodeById(nodeId));
+            for (size_t i = 1; i < 1 + mesh->skin->boneNodeName.size() && i < kMaxBones; ++i) {
+                string nodeName = mesh->skin->boneNodeName[i - 1];
+                if (!nodeName.empty()) {
+                    shared_ptr<ModelNodeSceneNode> bone(_model->getNodeByName(nodeName));
                     if (bone && bone->type() == SceneNodeType::Mesh) {
                         uniforms.skeletal->bones[i] = _modelNode->absoluteTransformInverse() * bone->boneTransform() * _modelNode->absoluteTransform();
                     }
diff --git a/src/engine/scene/node/modelnode.cpp b/src/engine/scene/node/modelnode.cpp
index 5d4814a4..16aa9d9c 100644
--- a/src/engine/scene/node/modelnode.cpp
+++ b/src/engine/scene/node/modelnode.cpp
@@ -78,12 +78,11 @@ void ModelSceneNode::buildNodeTree(shared_ptr<ModelNode> node, SceneNode *parent
         sceneNode->setLocalTransform(node->localTransform());
         parent->addChild(sceneNode);
     }
-    _nodeById.insert(make_pair(node->id(), sceneNode));
     _nodeByName.insert(make_pair(node->name(), sceneNode));
 
     if (node->isReference()) {
         auto model = make_shared<ModelSceneNode>(node->reference()->model, _usage, _sceneGraph, _animEventListener);
-        attach(node->id(), move(model));
+        attach(node->name(), move(model));
     }
     for (auto &child : node->children()) {
         buildNodeTree(child, sceneNode.get());
@@ -101,14 +100,14 @@ void ModelSceneNode::update(float dt) {
 void ModelSceneNode::computeAABB() {
     _aabb.reset();
 
-    for (auto &node : _nodeById) {
+    for (auto &node : _nodeByName) {
         if (node.second->type() == SceneNodeType::Mesh) {
             shared_ptr<ModelNode> modelNode(node.second->modelNode());
             AABB modelSpaceAABB(modelNode->mesh()->mesh->aabb() * modelNode->absoluteTransform());
             _aabb.expand(modelSpaceAABB);
         }
     }
-    for (auto &attachment : _attachmentByNodeId) {
+    for (auto &attachment : _attachments) {
         if (attachment.second->type() == SceneNodeType::Model) {
             AABB modelSpaceAABB(attachment.second->aabb() * attachment.second->absoluteTransform() * _absTransformInv);
             _aabb.expand(modelSpaceAABB);
@@ -136,7 +135,7 @@ void ModelSceneNode::signalEvent(const string &name) {
     debug(boost::format("Model '%s': event '%s' signalled") % _model->name() % name, 3);
 
     if (name == "detonate") {
-        for (auto &node : _nodeById) {
+        for (auto &node : _nodeByName) {
             if (node.second->type() == SceneNodeType::Emitter) {
                 static_pointer_cast<EmitterSceneNode>(node.second)->detonate();
             }
@@ -146,36 +145,25 @@ void ModelSceneNode::signalEvent(const string &name) {
     }
 }
 
-void ModelSceneNode::attach(uint16_t parentId, shared_ptr<SceneNode> node) {
-    auto maybeParent = _nodeById.find(parentId);
-    if (maybeParent == _nodeById.end()) return;
+void ModelSceneNode::attach(const string &parentName, shared_ptr<SceneNode> node) {
+    auto maybeParent = _nodeByName.find(parentName);
+    if (maybeParent == _nodeByName.end()) return;
 
     shared_ptr<ModelNodeSceneNode> parent(maybeParent->second);
     parent->addChild(node);
 
-    _attachmentByNodeId.insert(make_pair(parentId, node));
+    _attachments.insert(make_pair(parentName, node));
 
     computeAABB();
 }
 
-void ModelSceneNode::attach(const string &parentName, shared_ptr<SceneNode> node) {
-    auto parent = _model->getNodeByName(parentName);
-    if (parent) {
-        attach(parent->id(), node);
-    }
-}
-
-shared_ptr<ModelNodeSceneNode> ModelSceneNode::getNodeById(uint16_t nodeId) const {
-    return getFromLookupOrNull(_nodeById, nodeId);
-}
-
 shared_ptr<ModelNodeSceneNode> ModelSceneNode::getNodeByName(const string &name) const {
     return getFromLookupOrNull(_nodeByName, name);
 }
 
 shared_ptr<SceneNode> ModelSceneNode::getAttachment(const string &parentName) const {
     auto parent = _model->getNodeByName(parentName);
-    return parent ? getFromLookupOrNull(_attachmentByNodeId, parent->id()) : nullptr;
+    return parent ? getFromLookupOrNull(_attachments, parent->name()) : nullptr;
 }
 
 void ModelSceneNode::setDiffuseTexture(shared_ptr<Texture> texture) {
@@ -187,12 +175,12 @@ void ModelSceneNode::setDiffuseTexture(shared_ptr<Texture> texture) {
 }
 
 void ModelSceneNode::setAppliedForce(glm::vec3 force) {
-    for (auto &node : _nodeById) {
+    for (auto &node : _nodeByName) {
         if (node.second->type() == SceneNodeType::Mesh) {
             static_pointer_cast<MeshSceneNode>(node.second)->setAppliedForce(force);
         }
     }
-    for (auto &attachment : _attachmentByNodeId) {
+    for (auto &attachment : _attachments) {
         if (attachment.second->type() == SceneNodeType::Model) {
             static_pointer_cast<ModelSceneNode>(attachment.second)->setAppliedForce(force);
         }
diff --git a/src/engine/scene/node/modelnode.h b/src/engine/scene/node/modelnode.h
index 1101d118..4d74d4da 100644
--- a/src/engine/scene/node/modelnode.h
+++ b/src/engine/scene/node/modelnode.h
@@ -53,7 +53,6 @@ public:
     void computeAABB();
     void signalEvent(const std::string &name);
 
-    std::shared_ptr<ModelNodeSceneNode> getNodeById(uint16_t nodeId) const;
     std::shared_ptr<ModelNodeSceneNode> getNodeByName(const std::string &name) const;
 
     std::shared_ptr<graphics::Model> model() const { return _model; }
@@ -71,13 +70,12 @@ public:
 
     bool isAnimationFinished() const;
 
-    void setInanimateNodes(std::set<uint16_t> nodes) { _inanimateNodes = std::move(nodes); }
+    void setInanimateNodes(std::set<std::string> nodes) { _inanimateNodes = std::move(nodes); }
 
     // END Animation
 
     // Attachments
 
-    void attach(uint16_t parentId, std::shared_ptr<SceneNode> node);
     void attach(const std::string &parentName, std::shared_ptr<SceneNode> node);
 
     std::shared_ptr<SceneNode> getAttachment(const std::string &parentName) const;
@@ -109,7 +107,7 @@ private:
         std::shared_ptr<graphics::LipAnimation> lipAnim;
         AnimationProperties properties;
         float time { 0.0f };
-        std::unordered_map<uint16_t, AnimationState> stateById;
+        std::unordered_map<std::string, AnimationState> stateByName;
 
         // Flags
 
@@ -128,10 +126,8 @@ private:
 
     // Lookups
 
-    std::unordered_map<uint16_t, std::shared_ptr<ModelNodeSceneNode>> _nodeById;
     std::unordered_map<std::string, std::shared_ptr<ModelNodeSceneNode>> _nodeByName;
-
-    std::unordered_map<uint16_t, std::shared_ptr<SceneNode>> _attachmentByNodeId;
+    std::unordered_map<std::string, std::shared_ptr<SceneNode>> _attachments;
 
     // END Lookups
 
@@ -139,7 +135,7 @@ private:
 
     AnimationChannel _animChannels[kNumAnimationChannels];
     AnimationBlendMode _animBlendMode { AnimationBlendMode::Single };
-    std::set<uint16_t> _inanimateNodes; /**< node identifiers that are not to be animated */
+    std::set<std::string> _inanimateNodes; /**< names of nodes that are not to be animated */
 
     // END Animation
 
diff --git a/src/engine/scene/node/modelnode_animation.cpp b/src/engine/scene/node/modelnode_animation.cpp
index 7e25fd19..d3f89a5b 100644
--- a/src/engine/scene/node/modelnode_animation.cpp
+++ b/src/engine/scene/node/modelnode_animation.cpp
@@ -87,7 +87,7 @@ void ModelSceneNode::playAnimation(shared_ptr<Animation> anim, shared_ptr<LipAni
 
     // Optionally propagate animation to attachments
     if (properties.flags & AnimationFlags::propagate) {
-        for (auto &attachment : _attachmentByNodeId) {
+        for (auto &attachment : _attachments) {
             if (attachment.second->type() == SceneNodeType::Model) {
                 static_pointer_cast<ModelSceneNode>(attachment.second)->playAnimation(anim, lipAnim, properties);
             }
@@ -106,7 +106,7 @@ void ModelSceneNode::resetAnimationChannel(AnimationChannel &channel, shared_ptr
     channel.lipAnim = move(lipAnim);
     channel.properties = move(properties);
     channel.time = 0.0f;
-    channel.stateById.clear();
+    channel.stateByName.clear();
     channel.finished = false;
     channel.transition = false;
     channel.freeze = false;
@@ -160,14 +160,14 @@ void ModelSceneNode::updateAnimationChannel(AnimationChannel &channel, float dt)
     // Compute animation states only when this model is not culled
     if (!_culled) {
         float time = channel.transition ? channel.anim->transitionTime() : channel.time;
-        channel.stateById.clear();
+        channel.stateByName.clear();
         computeAnimationStates(channel, time, *_model->rootNode());
     }
 }
 
 void ModelSceneNode::computeAnimationStates(AnimationChannel &channel, float time, const ModelNode &modelNode) {
-    shared_ptr<ModelNode> animNode(channel.anim->getNodeById(modelNode.id()));
-    if (animNode && _inanimateNodes.count(modelNode.id()) == 0) {
+    shared_ptr<ModelNode> animNode(channel.anim->getNodeByName(modelNode.name()));
+    if (animNode && _inanimateNodes.count(modelNode.name()) == 0) {
         AnimationState state;
         state.flags = 0;
 
@@ -231,7 +231,7 @@ void ModelSceneNode::computeAnimationStates(AnimationChannel &channel, float tim
             state.selfIllumColor = move(animSelfIllum);
         }
 
-        channel.stateById.insert(make_pair(modelNode.id(), move(state)));
+        channel.stateByName.insert(make_pair(modelNode.name(), move(state)));
     }
 
     for (auto &child : modelNode.children()) {
@@ -240,8 +240,8 @@ void ModelSceneNode::computeAnimationStates(AnimationChannel &channel, float tim
 }
 
 void ModelSceneNode::applyAnimationStates(const ModelNode &modelNode) {
-    auto maybeSceneNode = _nodeById.find(modelNode.id());
-    if (maybeSceneNode != _nodeById.end()) {
+    auto maybeSceneNode = _nodeByName.find(modelNode.name());
+    if (maybeSceneNode != _nodeByName.end()) {
         shared_ptr<SceneNode> sceneNode(maybeSceneNode->second);
         AnimationState combined;
 
@@ -249,8 +249,8 @@ void ModelSceneNode::applyAnimationStates(const ModelNode &modelNode) {
             case AnimationBlendMode::Single:
             case AnimationBlendMode::Blend: {
                 bool blend = _animBlendMode == AnimationBlendMode::Blend && _animChannels[0].transition;
-                auto state1 = _animChannels[0].stateById.count(modelNode.id()) > 0 ? _animChannels[0].stateById[modelNode.id()] : AnimationState();
-                auto state2 = _animChannels[1].stateById.count(modelNode.id()) > 0 ? _animChannels[1].stateById[modelNode.id()] : AnimationState();
+                auto state1 = _animChannels[0].stateByName.count(modelNode.name()) > 0 ? _animChannels[0].stateByName[modelNode.name()] : AnimationState();
+                auto state2 = _animChannels[1].stateByName.count(modelNode.name()) > 0 ? _animChannels[1].stateByName[modelNode.name()] : AnimationState();
                 if (blend && state1.flags & AnimationStateFlags::transform && state2.flags & AnimationStateFlags::transform) {
                     float factor = glm::min(1.0f, _animChannels[0].time / _animChannels[0].anim->transitionTime());
                     glm::vec3 scale1, scale2, translation1, translation2, skew;
@@ -278,8 +278,8 @@ void ModelSceneNode::applyAnimationStates(const ModelNode &modelNode) {
             }
             case AnimationBlendMode::Overlay:
                 for (int i = 0; i < kNumAnimationChannels; ++i) {
-                    auto maybeState = _animChannels[i].stateById.find(modelNode.id());
-                    if (maybeState != _animChannels[i].stateById.end()) {
+                    auto maybeState = _animChannels[i].stateByName.find(modelNode.name());
+                    if (maybeState != _animChannels[i].stateByName.end()) {
                         const AnimationState &state = maybeState->second;
                         if ((state.flags & AnimationStateFlags::transform) && !(combined.flags & AnimationStateFlags::transform)) {
                             combined.flags |= AnimationStateFlags::transform;
@@ -302,8 +302,6 @@ void ModelSceneNode::applyAnimationStates(const ModelNode &modelNode) {
 
         if (combined.flags & AnimationStateFlags::transform) {
             sceneNode->setLocalTransform(combined.transform);
-        } else if (!modelNode.isSkinMesh()) {
-            sceneNode->setLocalTransform(modelNode.localTransform());
         }
         if (combined.flags & AnimationStateFlags::alpha) {
             static_pointer_cast<MeshSceneNode>(sceneNode)->setAlpha(combined.alpha);
@@ -319,7 +317,7 @@ void ModelSceneNode::applyAnimationStates(const ModelNode &modelNode) {
 }
 
 void ModelSceneNode::computeBoneTransforms() {
-    for (auto &node : _nodeById) {
+    for (auto &node : _nodeByName) {
         glm::mat4 transform(1.0f);
         transform = node.second->absoluteTransform() * node.second->modelNode()->absoluteTransformInverse(); // make relative to the rest pose (world space)
         transform = _absTransformInv * transform; // world space to model space