Send animations, bone overrides and attachments in entity initialization. Clients no longer have to be near an object when an animation or attachment is set to see the changes, and newly connected clients (or a client that simply renders the object for the first time) will get all of those settings. Therefore, the lua script no longer needs to run every X seconds either, just once per entity.

Finish fixing the material color code. But it won't work until MineTest has dynamic lighting... another day another feature.

Extra checks for the bone positioning / rotation code

Many checks and consistency improvements to the client attachment code

Make a separate function for checking if a client object is attached. A more in-depth change will be needed here to fix reading of invalid pointers

Use a different method of fetching the parent. Fixes the mass segmentation faults when rendering an attachment (some still happen though)

Major change to how attachments are handled. Fix the last segmentaton fault, which was due to the parent becoming invalid while being refreshed / removed which would bause the child to remain attached to nothing. Parents remove their children when being deleted themselves and add them back when re-added. Attachments are stored inside a 2D a vector which easily allows both a child to find their parent and a parent to find its children.

Remove attachment list entry when an object is being permanently removed. Also avoid duplicate entries in this list when re-attaching the same object

The "big code comments" can now go away. Client attachments almost work properly, and I know what else needs to be done

src/clientobject.cpp

@@ -36,7 +36,7 @@ ClientActiveObject::ClientActiveObject(u16 id, IGameDef *gamedef,
 
 ClientActiveObject::~ClientActiveObject()
 {
-	removeFromScene();
+	removeFromScene(true);
 }
 
 ClientActiveObject* ClientActiveObject::create(u8 type, IGameDef *gamedef,

src/clientobject.h

@@ -49,7 +49,7 @@ class ClientActiveObject : public ActiveObject
 
 	virtual void addToScene(scene::ISceneManager *smgr, ITextureSource *tsrc,
 			IrrlichtDevice *irr){}
-	virtual void removeFromScene(){}
+	virtual void removeFromScene(bool permanent){}
 	// 0 <= light_at_pos <= LIGHT_SUN
 	virtual void updateLight(u8 light_at_pos){}
 	virtual v3s16 getLightPosition(){return v3s16(0,0,0);}
@@ -61,6 +61,7 @@ class ClientActiveObject : public ActiveObject
 	virtual scene::IBillboardSceneNode *getSpriteSceneNode(){return NULL;}
 	virtual bool isPlayer(){return false;}
 	virtual bool isLocalPlayer(){return false;}
+	virtual void updateParent(){}
 	virtual bool doShowSelectionBox(){return true;}
 	
 	// Step object in time

src/content_cao.cpp

@@ -51,6 +51,8 @@ struct ToolCapabilities;
 
 core::map<u16, ClientActiveObject::Factory> ClientActiveObject::m_types;
 
+std::vector<core::vector2d<int> > attachment_list; // X is child ID, Y is parent ID
+
 /*
 	SmoothTranslator
 */
@@ -580,8 +582,7 @@ class GenericCAO : public ClientActiveObject
 	v2f m_frames;
 	int m_frame_speed;
 	int m_frame_blend;
-	std::map<std::string, core::vector2d<v3f> > m_bone_posrot;
-	ClientActiveObject* m_attachment_parent;
+	std::map<std::string, core::vector2d<v3f> > m_bone_posrot; // stores position and rotation for each bone name
 	std::string m_attachment_bone;
 	v3f m_attachment_position;
 	v3f m_attachment_rotation;
@@ -624,7 +625,6 @@ class GenericCAO : public ClientActiveObject
 		m_frame_speed(15),
 		m_frame_blend(0),
 		// Nothing to do for m_bone_posrot
-		m_attachment_parent(NULL),
 		m_attachment_bone(""),
 		m_attachment_position(v3f(0,0,0)),
 		m_attachment_rotation(v3f(0,0,0)),
@@ -693,28 +693,43 @@ class GenericCAO : public ClientActiveObject
 	}
 	core::aabbox3d<f32>* getSelectionBox()
 	{
-		if(!m_prop.is_visible || !m_is_visible || m_is_local_player)
+		if(!m_prop.is_visible || !m_is_visible || m_is_local_player || getParent() != NULL)
 			return NULL;
 		return &m_selection_box;
 	}
 	v3f getPosition()
 	{
+		if(getParent() != NULL){
+			if(m_meshnode)
+				return m_meshnode->getAbsolutePosition();
+			if(m_animated_meshnode)
+				return m_animated_meshnode->getAbsolutePosition();
+			if(m_spritenode)
+				return m_spritenode->getAbsolutePosition();
+			return v3f(0,0,0); // Just in case
+		}
 		return pos_translator.vect_show;
 	}
 
 	scene::IMeshSceneNode *getMeshSceneNode()
 	{
-		return m_meshnode;
+		if(m_meshnode)
+			return m_meshnode;
+		return NULL;
 	}
 
 	scene::IAnimatedMeshSceneNode *getAnimatedMeshSceneNode()
 	{
-		return m_animated_meshnode;
+		if(m_animated_meshnode)
+			return m_animated_meshnode;
+		return NULL;
 	}
 
 	scene::IBillboardSceneNode *getSpriteSceneNode()
 	{
-		return m_spritenode;
+		if(m_spritenode)
+			return m_spritenode;
+		return NULL;
 	}
 
 	bool isPlayer()
@@ -727,8 +742,80 @@ class GenericCAO : public ClientActiveObject
 		return m_is_local_player;
 	}
 
-	void removeFromScene()
+	void updateParent()
 	{
+		updateAttachments();
+	}
+
+	ClientActiveObject *getParent()
+	{
+		ClientActiveObject *obj = NULL;
+		for(std::vector<core::vector2d<int> >::const_iterator cii = attachment_list.begin(); cii != attachment_list.end(); cii++)
+		{
+			if(cii->X == this->getId()){ // This ID is our child
+				if(cii->Y > 0){ // A parent ID exists for our child
+					if(cii->X != cii->Y){ // The parent and child ID are not the same
+						obj = m_env->getActiveObject(cii->Y);
+					}
+				}
+				break;
+			}
+		}
+		if(obj)
+			return obj;
+		return NULL;
+	}
+
+	void removeFromScene(bool permanent)
+	{
+		// bool permanent should be true when removing the object permanently and false when it's only refreshed (and comes back in a few frames)
+
+		// If this object is being permanently removed, delete it from the attachments list
+		if(permanent)
+		{
+			for(std::vector<core::vector2d<int> >::iterator ii = attachment_list.begin(); ii != attachment_list.end(); ii++)
+			{
+				if(ii->X == this->getId()) // This is the ID of our object
+				{
+					attachment_list.erase(ii);
+					break;
+				}
+			}
+		}
+
+		// If this object is being removed, either permanently or just to refresh it, then all
+		// objects attached to it must be unparented else Irrlicht causes a segmentation fault.
+		for(std::vector<core::vector2d<int> >::iterator ii = attachment_list.begin(); ii != attachment_list.end(); ii++)
+		{
+			if(ii->Y == this->getId()) // This is a child of our parent
+			{
+				ClientActiveObject *obj = m_env->getActiveObject(ii->X); // Get the object of the child
+				if(obj)
+				{
+					if(permanent)
+					{
+						// The parent is being permanently removed, so the child stays detached
+						ii->Y = 0;
+						obj->updateParent();
+					}
+					else
+					{
+						// The parent is being refreshed, detach our child enough to avoid bad memory reads
+						// This only stays into effect for a few frames, as addToScene will parent its children back
+						scene::IMeshSceneNode *m_child_meshnode = obj->getMeshSceneNode();
+						scene::IAnimatedMeshSceneNode *m_child_animated_meshnode = obj->getAnimatedMeshSceneNode();
+						scene::IBillboardSceneNode *m_child_spritenode = obj->getSpriteSceneNode();
+						if(m_child_meshnode)
+							m_child_meshnode->setParent(m_smgr->getRootSceneNode());
+						if(m_child_animated_meshnode)
+							m_child_animated_meshnode->setParent(m_smgr->getRootSceneNode());
+						if(m_child_spritenode)
+							m_child_spritenode->setParent(m_smgr->getRootSceneNode());
+					}
+				}
+			}
+		}
+
 		if(m_meshnode){
 			m_meshnode->remove();
 			m_meshnode = NULL;
@@ -749,6 +836,18 @@ class GenericCAO : public ClientActiveObject
 		m_smgr = smgr;
 		m_irr = irr;
 
+		// If this object has attachments and is being re-added after having been refreshed, parent its children back.
+		// The parent ID for this child hasn't been changed in attachment_list, so just update its attachments.
+		for(std::vector<core::vector2d<int> >::iterator ii = attachment_list.begin(); ii != attachment_list.end(); ii++)
+		{
+			if(ii->Y == this->getId()) // This is a child of our parent
+			{
+				ClientActiveObject *obj = m_env->getActiveObject(ii->X); // Get the object of the child
+				if(obj)
+					obj->updateParent();
+			}
+		}
+
 		if(m_meshnode != NULL || m_animated_meshnode != NULL || m_spritenode != NULL)
 			return;
 		
@@ -828,7 +927,7 @@ class GenericCAO : public ClientActiveObject
 			mesh->addMeshBuffer(buf);
 			buf->drop();
 			}
-			m_meshnode = smgr->addMeshSceneNode(mesh, NULL);
+			m_meshnode = smgr->addMeshSceneNode(mesh, m_smgr->getRootSceneNode());
 			mesh->drop();
 			// Set it to use the materials of the meshbuffers directly.
 			// This is needed for changing the texture in the future
@@ -837,7 +936,7 @@ class GenericCAO : public ClientActiveObject
 		else if(m_prop.visual == "cube"){
 			infostream<<"GenericCAO::addToScene(): cube"<<std::endl;
 			scene::IMesh *mesh = createCubeMesh(v3f(BS,BS,BS));
-			m_meshnode = smgr->addMeshSceneNode(mesh, NULL);
+			m_meshnode = smgr->addMeshSceneNode(mesh, m_smgr->getRootSceneNode());
 			mesh->drop();
 			
 			m_meshnode->setScale(v3f(m_prop.visual_size.X,
@@ -851,7 +950,7 @@ class GenericCAO : public ClientActiveObject
 			scene::IAnimatedMesh *mesh = smgr->getMesh(m_prop.mesh.c_str());
 			if(mesh)
 			{
-				m_animated_meshnode = smgr->addAnimatedMeshSceneNode(mesh, NULL);
+				m_animated_meshnode = smgr->addAnimatedMeshSceneNode(mesh, m_smgr->getRootSceneNode());
 				m_animated_meshnode->animateJoints(); // Needed for some animations
 				m_animated_meshnode->setScale(v3f(m_prop.visual_size.X,
 						m_prop.visual_size.Y,
@@ -876,7 +975,7 @@ class GenericCAO : public ClientActiveObject
 						irr->getVideoDriver()->getMeshManipulator();
 				scene::IMesh *mesh = manip->createMeshUniquePrimitives(item_mesh);
 
-				m_meshnode = smgr->addMeshSceneNode(mesh, NULL);
+				m_meshnode = smgr->addMeshSceneNode(mesh, m_smgr->getRootSceneNode());
 				mesh->drop();
 				
 				m_meshnode->setScale(v3f(m_prop.visual_size.X/2,
@@ -918,7 +1017,7 @@ class GenericCAO : public ClientActiveObject
 	void updateLight(u8 light_at_pos)
 	{
 		// Objects attached to the local player should always be hidden
-		if(m_attachment_parent != NULL && m_attachment_parent->isLocalPlayer())
+		if(getParent() != NULL && getParent()->isLocalPlayer())
 			m_is_visible = false;
 		else
 			m_is_visible = (m_hp != 0);
@@ -949,7 +1048,7 @@ class GenericCAO : public ClientActiveObject
 
 	void updateNodePos()
 	{
-		if(m_attachment_parent != NULL)
+		if(getParent() != NULL)
 			return;
 
 		if(m_meshnode){
@@ -975,14 +1074,27 @@ class GenericCAO : public ClientActiveObject
 
 		if(m_visuals_expired && m_smgr && m_irr){
 			m_visuals_expired = false;
-			removeFromScene();
+			removeFromScene(false);
 			addToScene(m_smgr, m_gamedef->tsrc(), m_irr);
 			updateAnimations();
 			updateBonePosRot();
 			updateAttachments();
+			return;
 		}
 
-		if(m_attachment_parent == NULL) // Attachments should be glued to their parent by Irrlicht
+		if(getParent() != NULL) // Attachments should be glued to their parent by Irrlicht
+		{
+			// Set these for later
+			if(m_meshnode)
+				m_position = m_meshnode->getAbsolutePosition();
+			if(m_animated_meshnode)
+				m_position = m_animated_meshnode->getAbsolutePosition();
+			if(m_spritenode)
+				m_position = m_spritenode->getAbsolutePosition();
+			m_velocity = v3f(0,0,0);
+			m_acceleration = v3f(0,0,0);
+		}
+		else
 		{
 			if(m_prop.physical){
 				core::aabbox3d<f32> box = m_prop.collisionbox;
@@ -1047,17 +1159,10 @@ class GenericCAO : public ClientActiveObject
 				updateTextures("");
 			}
 		}
-		if(fabs(m_prop.automatic_rotate) > 0.001){
+		if(getParent() == NULL && fabs(m_prop.automatic_rotate) > 0.001){
 			m_yaw += dtime * m_prop.automatic_rotate * 180 / M_PI;
 			updateNodePos();
 		}
-
-		// REMAINING ATTACHMENT ISSUES:
-		// Absolute Position of attachments is printed differently here than what it's set to in the SetAttachment function.
-		// Apparently here it prints the origin of the parent, but ignores the offset it was actually set to.
-
-		//if(m_animated_meshnode != NULL && m_attachment_parent != NULL)
-		//	errorstream<<"Attachment position, step: "<<m_animated_meshnode->getAbsolutePosition().X<<","<<m_animated_meshnode->getAbsolutePosition().Y<<","<<m_animated_meshnode->getAbsolutePosition().Z<<std::endl;
 	}
 
 	void updateTexturePos()
@@ -1123,9 +1228,15 @@ class GenericCAO : public ClientActiveObject
 				m_spritenode->setMaterialTexture(0,
 						tsrc->getTextureRaw(texturestring));
 
-				// Does not work yet with the current lighting settings
-				m_meshnode->getMaterial(0).AmbientColor = m_prop.colors[0];
-				m_meshnode->getMaterial(0).DiffuseColor = m_prop.colors[0];
+				// This allows setting per-material colors. However, until a real lighting
+				// system is added, the code below will have no effect. Once MineTest
+				// has directional lighting, it should work automatically.
+				if(m_prop.colors.size() >= 1)
+				{
+					m_meshnode->getMaterial(0).AmbientColor = m_prop.colors[0];
+					m_meshnode->getMaterial(0).DiffuseColor = m_prop.colors[0];
+					m_meshnode->getMaterial(0).SpecularColor = m_prop.colors[0];
+				}
 			}
 		}
 		if(m_animated_meshnode)
@@ -1153,9 +1264,12 @@ class GenericCAO : public ClientActiveObject
 				}
 				for (u32 i = 0; i < m_prop.colors.size(); ++i)
 				{
-					// Does not work yet with the current lighting settings
+					// This allows setting per-material colors. However, until a real lighting
+					// system is added, the code below will have no effect. Once MineTest
+					// has directional lighting, it should work automatically.
 					m_animated_meshnode->getMaterial(i).AmbientColor = m_prop.colors[i];
 					m_animated_meshnode->getMaterial(i).DiffuseColor = m_prop.colors[i];
+					m_animated_meshnode->getMaterial(i).SpecularColor = m_prop.colors[i];
 				}
 			}
 		}
@@ -1184,9 +1298,15 @@ class GenericCAO : public ClientActiveObject
 					material.getTextureMatrix(0).setTextureTranslate(pos.X, pos.Y);
 					material.getTextureMatrix(0).setTextureScale(size.X, size.Y);
 
-					// Does not work yet with the current lighting settings
-					m_meshnode->getMaterial(i).AmbientColor = m_prop.colors[i];
-					m_meshnode->getMaterial(i).DiffuseColor = m_prop.colors[i];
+					// This allows setting per-material colors. However, until a real lighting
+					// system is added, the code below will have no effect. Once MineTest
+					// has directional lighting, it should work automatically.
+					if(m_prop.colors.size() > i)
+					{
+						m_meshnode->getMaterial(i).AmbientColor = m_prop.colors[i];
+						m_meshnode->getMaterial(i).DiffuseColor = m_prop.colors[i];
+						m_meshnode->getMaterial(i).SpecularColor = m_prop.colors[i];
+					}
 				}
 			}
 			else if(m_prop.visual == "upright_sprite")
@@ -1201,9 +1321,15 @@ class GenericCAO : public ClientActiveObject
 					buf->getMaterial().setTexture(0,
 							tsrc->getTextureRaw(tname));
 					
-					// Does not work yet with the current lighting settings
-					m_meshnode->getMaterial(0).AmbientColor = m_prop.colors[0];
-					m_meshnode->getMaterial(0).DiffuseColor = m_prop.colors[0];
+					// This allows setting per-material colors. However, until a real lighting
+					// system is added, the code below will have no effect. Once MineTest
+					// has directional lighting, it should work automatically.
+					if(m_prop.colors.size() >= 1)
+					{
+						buf->getMaterial().AmbientColor = m_prop.colors[0];
+						buf->getMaterial().DiffuseColor = m_prop.colors[0];
+						buf->getMaterial().SpecularColor = m_prop.colors[0];
+					}
 				}
 				{
 					std::string tname = "unknown_object.png";
@@ -1216,9 +1342,21 @@ class GenericCAO : public ClientActiveObject
 					buf->getMaterial().setTexture(0,
 							tsrc->getTextureRaw(tname));
 
-					// Does not work yet with the current lighting settings
-					m_meshnode->getMaterial(1).AmbientColor = m_prop.colors[1]; 
-					m_meshnode->getMaterial(1).DiffuseColor = m_prop.colors[1]; 
+					// This allows setting per-material colors. However, until a real lighting
+					// system is added, the code below will have no effect. Once MineTest
+					// has directional lighting, it should work automatically.
+					if(m_prop.colors.size() >= 2)
+					{
+						buf->getMaterial().AmbientColor = m_prop.colors[1];
+						buf->getMaterial().DiffuseColor = m_prop.colors[1];
+						buf->getMaterial().SpecularColor = m_prop.colors[1];
+					}
+					else if(m_prop.colors.size() >= 1)
+					{
+						buf->getMaterial().AmbientColor = m_prop.colors[0];
+						buf->getMaterial().DiffuseColor = m_prop.colors[0];
+						buf->getMaterial().SpecularColor = m_prop.colors[0];
+					}
 				}
 			}
 		}
@@ -1226,7 +1364,7 @@ class GenericCAO : public ClientActiveObject
 
 	void updateAnimations()
 	{
-		if(!m_animated_meshnode)
+		if(m_animated_meshnode == NULL)
 			return;
 
 		m_animated_meshnode->setFrameLoop((int)m_frames.X, (int)m_frames.Y);
@@ -1236,14 +1374,17 @@ class GenericCAO : public ClientActiveObject
 
 	void updateBonePosRot()
 	{
-		if(m_bone_posrot.size() > 0)
-		{
-			m_animated_meshnode->setJointMode(irr::scene::EJUOR_CONTROL); // To write positions to the mesh on render
-			for(std::map<std::string, core::vector2d<v3f> >::const_iterator ii = m_bone_posrot.begin(); ii != m_bone_posrot.end(); ++ii){
-				std::string bone_name = (*ii).first;
-				v3f bone_pos = (*ii).second.X;
-				v3f bone_rot = (*ii).second.Y;
-				irr::scene::IBoneSceneNode* bone = m_animated_meshnode->getJointNode(bone_name.c_str());
+		if(!m_bone_posrot.size() || m_animated_meshnode == NULL)
+			return;
+
+		m_animated_meshnode->setJointMode(irr::scene::EJUOR_CONTROL); // To write positions to the mesh on render
+		for(std::map<std::string, core::vector2d<v3f> >::const_iterator ii = m_bone_posrot.begin(); ii != m_bone_posrot.end(); ++ii){
+			std::string bone_name = (*ii).first;
+			v3f bone_pos = (*ii).second.X;
+			v3f bone_rot = (*ii).second.Y;
+			irr::scene::IBoneSceneNode* bone = m_animated_meshnode->getJointNode(bone_name.c_str());
+			if(bone)
+			{
 				bone->setPosition(bone_pos);
 				bone->setRotation(bone_rot);
 			}
@@ -1252,38 +1393,7 @@ class GenericCAO : public ClientActiveObject
 	
 	void updateAttachments()
 	{
-		// REMAINING ATTACHMENT ISSUES:
-		// We get to this function when the object is an attachment that needs to
-		// be attached to its parent. If a bone is set we attach it to that skeletal
-		// bone, otherwise just to the object's origin. Attachments should not copy parent
-		// position as that's laggy... instead the Irrlicht function(s) to attach should
-		// be used. If the parent object is NULL that means this object should be detached.
-		// This function is only called whenever a GENERIC_CMD_SET_ATTACHMENT message is received.
-		
-		// We already attach our entity on the server too (copy position). Reason we attach
-		// to the client as well is first of all lag. The server sends the position
-		// of the child separately than that of the parent, so even on localhost
-		// you'd see the child lagging behind. Models are also client-side, so this is
-		// needed to read bone data and attach to joints.
-		
-		// Functions:
-		// - m_attachment_parent is ClientActiveObject* for the parent entity.
-		// - m_attachment_bone is std::string of the bone, "" means none.
-		// - m_attachment_position is v3f and represents the position offset of the attachment.
-		// - m_attachment_rotation is v3f and represents the rotation offset of the attachment.
-		
-		// Implementation information:
-		// From what I know, we need to get the AnimatedMeshSceneNode of m_attachment_parent then
-		// use parent_node->addChild(m_animated_meshnode) for position attachment. For skeletal
-		// attachment I don't know yet. Same must be used to detach when a NULL parent is received.
-
-		//Useful links:
-		// http://irrlicht.sourceforge.net/forum/viewtopic.php?t=7514
-		// http://www.irrlicht3d.org/wiki/index.php?n=Main.HowToUseTheNewAnimationSystem
-		// http://gamedev.stackexchange.com/questions/27363/finding-the-endpoint-of-a-named-bone-in-irrlicht
-		// Irrlicht documentation: http://irrlicht.sourceforge.net/docu/
-
-		if(m_attachment_parent == NULL || m_attachment_parent->isLocalPlayer()) // Detach
+		if(getParent() == NULL || getParent()->isLocalPlayer()) // Detach
 		{
 			if(m_meshnode)
 			{
@@ -1315,25 +1425,21 @@ class GenericCAO : public ClientActiveObject
 		}
 		else // Attach
 		{
-			// REMAINING ATTACHMENT ISSUES:
-			// The code below should cause the child to get attached, but for some reason it's not working
-			// A debug print confirms both position and absolute position are set accordingly, but the object still doesn't show
-			// Position and Absolute Position were tested to be set properly here
-
 			scene::IMeshSceneNode *parent_mesh = NULL;
-			if(m_attachment_parent->getMeshSceneNode())
-				parent_mesh = m_attachment_parent->getMeshSceneNode();
+			if(getParent()->getMeshSceneNode())
+				parent_mesh = getParent()->getMeshSceneNode();
 			scene::IAnimatedMeshSceneNode *parent_animated_mesh = NULL;
-			if(m_attachment_parent->getAnimatedMeshSceneNode())
-				parent_animated_mesh = m_attachment_parent->getAnimatedMeshSceneNode();
+			if(getParent()->getAnimatedMeshSceneNode())
+				parent_animated_mesh = getParent()->getAnimatedMeshSceneNode();
 			scene::IBillboardSceneNode *parent_sprite = NULL;
-			if(m_attachment_parent->getSpriteSceneNode())
-				parent_sprite = m_attachment_parent->getSpriteSceneNode();
+			if(getParent()->getSpriteSceneNode())
+				parent_sprite = getParent()->getSpriteSceneNode();
 
 			scene::IBoneSceneNode *parent_bone = NULL;
 			if(parent_animated_mesh && m_attachment_bone != "")
 				parent_bone = parent_animated_mesh->getJointNode(m_attachment_bone.c_str());
 
+			// The spaghetti code below makes sure attaching works if either the parent or child is a spritenode, meshnode, or animatedmeshnode
 			// TODO: Perhaps use polymorphism here to save code duplication
 			if(m_meshnode){
 				if(parent_bone){
@@ -1452,7 +1558,6 @@ class GenericCAO : public ClientActiveObject
 		else if(cmd == GENERIC_CMD_UPDATE_POSITION)
 		{
 			// Not sent by the server if the object is an attachment
-			
 			m_position = readV3F1000(is);
 			m_velocity = readV3F1000(is);
 			m_acceleration = readV3F1000(is);
@@ -1462,6 +1567,9 @@ class GenericCAO : public ClientActiveObject
 			bool is_end_position = readU8(is);
 			float update_interval = readF1000(is);
 
+			if(getParent() != NULL) // Just in case
+				return;
+
 			// Place us a bit higher if we're physical, to not sink into
 			// the ground due to sucky collision detection...
 			if(m_prop.physical)
@@ -1500,7 +1608,7 @@ class GenericCAO : public ClientActiveObject
 			m_frame_speed = readF1000(is);
 			m_frame_blend = readF1000(is);
 
-			expireVisuals(); // Automatically calls the proper function next
+			updateAnimations();
 		}
 		else if(cmd == GENERIC_CMD_SET_BONE_POSROT)
 		{
@@ -1509,20 +1617,25 @@ class GenericCAO : public ClientActiveObject
 			v3f rotation = readV3F1000(is);
 			m_bone_posrot[bone] = core::vector2d<v3f>(position, rotation);
 
-			expireVisuals(); // Automatically calls the proper function next
+			updateBonePosRot();
 		}
 		else if(cmd == GENERIC_CMD_SET_ATTACHMENT)
 		{
-			int parent_id = readS16(is);
-			ClientActiveObject *obj = m_env->getActiveObject(parent_id);
-			if(!parent_id || !obj)
-				obj = NULL;
-			m_attachment_parent = obj;
+			// If an entry already exists for this object, delete it first to avoid duplicates
+			for(std::vector<core::vector2d<int> >::iterator ii = attachment_list.begin(); ii != attachment_list.end(); ii++)
+			{
+				if(ii->X == this->getId()) // This is the ID of our object
+				{
+					attachment_list.erase(ii);
+					break;
+				}
+			}
+			attachment_list.push_back(core::vector2d<int>(this->getId(), readS16(is)));
 			m_attachment_bone = deSerializeString(is);
 			m_attachment_position = readV3F1000(is);
 			m_attachment_rotation = readV3F1000(is);
 
-			expireVisuals(); // Automatically calls the proper function next
+			updateAttachments();
 		}
 		else if(cmd == GENERIC_CMD_PUNCHED)
 		{

src/content_sao.cpp

@@ -520,7 +520,7 @@ void LuaEntitySAO::step(float dtime, bool send_recommended)
 
 	if(m_animations_sent == false){
 		m_animations_sent = true;
-		std::string str = gob_cmd_set_animations(m_animation_frames, m_animation_speed, m_animation_blend);
+		std::string str = gob_cmd_update_animations(m_animation_frames, m_animation_speed, m_animation_blend);
 		// create message and add to list
 		ActiveObjectMessage aom(getId(), true, str);
 		m_messages_out.push_back(aom);
@@ -529,7 +529,7 @@ void LuaEntitySAO::step(float dtime, bool send_recommended)
 	if(m_animations_bone_sent == false){
 		m_animations_bone_sent = true;
 		for(std::map<std::string, core::vector2d<v3f> >::const_iterator ii = m_animation_bone.begin(); ii != m_animation_bone.end(); ++ii){
-			std::string str = gob_cmd_set_bone_posrot((*ii).first, (*ii).second.X, (*ii).second.Y);
+			std::string str = gob_cmd_update_bone_posrot((*ii).first, (*ii).second.X, (*ii).second.Y);
 			// create message and add to list
 			ActiveObjectMessage aom(getId(), true, str);
 			m_messages_out.push_back(aom);
@@ -538,7 +538,7 @@ void LuaEntitySAO::step(float dtime, bool send_recommended)
 
 	if(m_attachment_sent == false){
 		m_attachment_sent = true;
-		std::string str = gob_cmd_set_attachment(m_attachment_parent_id, m_attachment_bone, m_attachment_position, m_attachment_rotation);
+		std::string str = gob_cmd_update_attachment(m_attachment_parent_id, m_attachment_bone, m_attachment_position, m_attachment_rotation);
 		// create message and add to list
 		ActiveObjectMessage aom(getId(), true, str);
 		m_messages_out.push_back(aom);
@@ -555,9 +555,18 @@ std::string LuaEntitySAO::getClientInitializationData()
 	writeV3F1000(os, m_base_position);
 	writeF1000(os, m_yaw);
 	writeS16(os, m_hp);
-	writeU8(os, 2); // number of messages stuffed in here
+
+	writeU8(os, 4 + m_animation_bone.size()); // number of messages stuffed in here
 	os<<serializeLongString(getPropertyPacket()); // message 1
 	os<<serializeLongString(gob_cmd_update_armor_groups(m_armor_groups)); // 2
+	os<<serializeLongString(gob_cmd_update_animations(m_animation_frames, m_animation_speed, m_animation_blend)); // 3
+	if(m_animation_bone.size()){
+		for(std::map<std::string, core::vector2d<v3f> >::const_iterator ii = m_animation_bone.begin(); ii != m_animation_bone.end(); ++ii){
+			os<<serializeLongString(gob_cmd_update_bone_posrot((*ii).first, (*ii).second.X, (*ii).second.Y)); // m_animation_bone.size
+		}
+	}
+	os<<serializeLongString(gob_cmd_update_attachment(m_attachment_parent_id, m_attachment_bone, m_attachment_position, m_attachment_rotation)); // 4
+
 	// return result
 	return os.str();
 }
@@ -948,9 +957,19 @@ std::string PlayerSAO::getClientInitializationData()
 	writeV3F1000(os, m_player->getPosition() + v3f(0,BS*1,0));
 	writeF1000(os, m_player->getYaw());
 	writeS16(os, getHP());
-	writeU8(os, 2); // number of messages stuffed in here
+
+	writeU8(os, 4 + m_animation_bone.size()); // number of messages stuffed in here
 	os<<serializeLongString(getPropertyPacket()); // message 1
 	os<<serializeLongString(gob_cmd_update_armor_groups(m_armor_groups)); // 2
+	os<<serializeLongString(gob_cmd_update_animations(m_animation_frames, m_animation_speed, m_animation_blend)); // 3
+	if(m_animation_bone.size()){
+		for(std::map<std::string, core::vector2d<v3f> >::const_iterator ii = m_animation_bone.begin(); ii != m_animation_bone.end(); ++ii){
+			os<<serializeLongString(gob_cmd_update_bone_posrot((*ii).first, (*ii).second.X, (*ii).second.Y)); // m_animation_bone.size
+		}
+	}
+	os<<serializeLongString(gob_cmd_update_attachment(m_attachment_parent_id, m_attachment_bone, m_attachment_position, m_attachment_rotation)); // 4
+
+	// return result
 	return os.str();
 }
 
@@ -1085,7 +1104,7 @@ void PlayerSAO::step(float dtime, bool send_recommended)
 
 	if(m_animations_sent == false){
 		m_animations_sent = true;
-		std::string str = gob_cmd_set_animations(m_animation_frames, m_animation_speed, m_animation_blend);
+		std::string str = gob_cmd_update_animations(m_animation_frames, m_animation_speed, m_animation_blend);
 		// create message and add to list
 		ActiveObjectMessage aom(getId(), true, str);
 		m_messages_out.push_back(aom);
@@ -1094,7 +1113,7 @@ void PlayerSAO::step(float dtime, bool send_recommended)
 	if(m_animations_bone_sent == false){
 		m_animations_bone_sent = true;
 		for(std::map<std::string, core::vector2d<v3f> >::const_iterator ii = m_animation_bone.begin(); ii != m_animation_bone.end(); ++ii){
-			std::string str = gob_cmd_set_bone_posrot((*ii).first, (*ii).second.X, (*ii).second.Y);
+			std::string str = gob_cmd_update_bone_posrot((*ii).first, (*ii).second.X, (*ii).second.Y);
 			// create message and add to list
 			ActiveObjectMessage aom(getId(), true, str);
 			m_messages_out.push_back(aom);
@@ -1103,7 +1122,7 @@ void PlayerSAO::step(float dtime, bool send_recommended)
 
 	if(m_attachment_sent == false){
 		m_attachment_sent = true;
-		std::string str = gob_cmd_set_attachment(m_attachment_parent_id, m_attachment_bone, m_attachment_position, m_attachment_rotation);
+		std::string str = gob_cmd_update_attachment(m_attachment_parent_id, m_attachment_bone, m_attachment_position, m_attachment_rotation);
 		// create message and add to list
 		ActiveObjectMessage aom(getId(), true, str);
 		m_messages_out.push_back(aom);

src/content_sao.h

@@ -264,7 +264,7 @@ class PlayerSAO : public ServerActiveObject
 	float m_animation_blend;
 	bool m_animations_sent;
 
-	std::map<std::string, core::vector2d<v3f> > m_animation_bone;
+	std::map<std::string, core::vector2d<v3f> > m_animation_bone; // stores position and rotation for each bone name
 	bool m_animations_bone_sent;
 	
 	ServerActiveObject *m_parent;

src/environment.cpp

@@ -2315,7 +2315,7 @@ void ClientEnvironment::removeActiveObject(u16 id)
 				<<"id="<<id<<" not found"<<std::endl;
 		return;
 	}
-	obj->removeFromScene();
+	obj->removeFromScene(true);
 	delete obj;
 	m_active_objects.remove(id);
 }

src/genericobject.cpp

@@ -92,7 +92,7 @@ std::string gob_cmd_set_sprite(
 	return os.str();
 }
 
-std::string gob_cmd_set_animations(v2f frames, float frame_speed, float frame_blend)
+std::string gob_cmd_update_animations(v2f frames, float frame_speed, float frame_blend)
 {
 	std::ostringstream os(std::ios::binary);
 	// command 
@@ -104,7 +104,7 @@ std::string gob_cmd_set_animations(v2f frames, float frame_speed, float frame_bl
 	return os.str();
 }
 
-std::string gob_cmd_set_bone_posrot(std::string bone, v3f position, v3f rotation)
+std::string gob_cmd_update_bone_posrot(std::string bone, v3f position, v3f rotation)
 {
 	std::ostringstream os(std::ios::binary);
 	// command 
@@ -116,7 +116,7 @@ std::string gob_cmd_set_bone_posrot(std::string bone, v3f position, v3f rotation
 	return os.str();
 }
 
-std::string gob_cmd_set_attachment(int parent_id, std::string bone, v3f position, v3f rotation)
+std::string gob_cmd_update_attachment(int parent_id, std::string bone, v3f position, v3f rotation)
 {
 	std::ostringstream os(std::ios::binary);
 	// command 

src/genericobject.h

@@ -57,11 +57,11 @@ std::string gob_cmd_set_sprite(
 	bool select_horiz_by_yawpitch
 );
 
-std::string gob_cmd_set_animations(v2f frames, float frame_speed, float frame_blend);
+std::string gob_cmd_update_animations(v2f frames, float frame_speed, float frame_blend);
 
-std::string gob_cmd_set_bone_posrot(std::string bone, v3f position, v3f rotation);
+std::string gob_cmd_update_bone_posrot(std::string bone, v3f position, v3f rotation);
 
-std::string gob_cmd_set_attachment(int parent_id, std::string bone, v3f position, v3f rotation);
+std::string gob_cmd_update_attachment(int parent_id, std::string bone, v3f position, v3f rotation);
 
 std::string gob_cmd_punched(s16 damage, s16 result_hp);