Update attachments at the ending of the addToScene function for parents. And with this... *drum roll* Client-side attachments are at last functional and stick visibly.

Fix the last segmentation fault (apparently). So far attachments seem to be fully functional, although removing the parent causes children to go to origin 0,0,0 and possibly still cause such a fault (though this should already be addressed)

Fix a bug in falling code where entities get stuck

Also check if the parent has been removed server-side, and detach the child if so. Fixes children going to origin 0,0,0 when their parent is removed.

Unset all attachment properties when permanently detaching (on both the client and server). Also store less data we don't need

Create a separate function for detaching, and also update lua api documentation

When a child is detached, update its position from the server to clients. This WILL cause it to get positioned slightly differently client side, as the server attachment system only copies parent origin and knows not about mesh / bone transformation. This prevents different clients seeing the object detached in different spots which is most correct

Update the position of attached players to clients. An attached player will see himself move, but this is currently VERY ugly and laggy as it is done by the server (it probably must stay this way too)

Use a different approach for locally attached players. This allows for smooth positio transitions to work, as well at the player turning around freely. Still buggy however

1 files changed, 68 insertions, 55 deletions

diff --git a/src/content_cao.cpp b/src/content_cao.cpp
index c2cce3d58..821862c9b 100644
--- a/src/content_cao.cpp
+++ b/src/content_cao.cpp
@@ -768,50 +768,26 @@ public:
 
 	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)
+		if(permanent) // Should be true when removing the object permanently and false when refreshing (eg: updating visuals)
 		{
+			// Detach this object's children
 			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
+				if(ii->Y == this->getId()) // Is a child of our object
 				{
-					attachment_list.erase(ii);
-					break;
+					ii->Y = 0;
+					ClientActiveObject *obj = m_env->getActiveObject(ii->X); // Get the object of the child
+					if(obj)
+						obj->updateParent();
 				}
 			}
-		}
-
-		// 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
+			// Delete this object from the attachments list
+			for(std::vector<core::vector2d<int> >::iterator ii = attachment_list.begin(); ii != attachment_list.end(); ii++)
 			{
-				ClientActiveObject *obj = m_env->getActiveObject(ii->X); // Get the object of the child
-				if(obj)
+				if(ii->X == this->getId()) // Is our object
 				{
-					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());
-					}
+					attachment_list.erase(ii);
+					break;
 				}
 			}
 		}
@@ -836,18 +812,6 @@ public:
 		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;
 		
@@ -1074,14 +1038,45 @@ public:
 
 		if(m_visuals_expired && m_smgr && m_irr){
 			m_visuals_expired = false;
+
+			// Attachments, part 1: All attached objects must be unparented first, or 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)
+					{
+						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());
+					}
+				}
+			}
+
 			removeFromScene(false);
 			addToScene(m_smgr, m_gamedef->tsrc(), m_irr);
 			updateAnimations();
 			updateBonePosRot();
 			updateAttachments();
-			return;
-		}
 
+			// Attachments, part 2: Now that the parent has been refreshed, put its attachments back
+			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(getParent() != NULL) // Attachments should be glued to their parent by Irrlicht
 		{
 			// Set these for later
@@ -1093,6 +1088,12 @@ public:
 				m_position = m_spritenode->getAbsolutePosition();
 			m_velocity = v3f(0,0,0);
 			m_acceleration = v3f(0,0,0);
+
+			if(m_is_local_player) // Update local player attachment position
+			{
+				LocalPlayer *player = m_env->getLocalPlayer();
+				player->overridePosition = getParent()->getPosition();
+			}
 		}
 		else
 		{
@@ -1422,6 +1423,11 @@ public:
 				m_spritenode->setRotation(old_rotation);
 				m_spritenode->updateAbsolutePosition();
 			}
+			if(m_is_local_player)
+			{
+				LocalPlayer *player = m_env->getLocalPlayer();
+				player->isAttached = false;
+			}
 		}
 		else // Attach
 		{
@@ -1528,6 +1534,12 @@ public:
 					}
 				}
 			}
+			if(m_is_local_player)
+			{
+				LocalPlayer *player = m_env->getLocalPlayer();
+				player->isAttached = true;
+				player->overridePosition = m_attachment_position;
+			}
 		}
 	}
 
@@ -1557,7 +1569,8 @@ public:
 		}
 		else if(cmd == GENERIC_CMD_UPDATE_POSITION)
 		{
-			// Not sent by the server if the object is an attachment
+			// Not sent by the server if this object is an attachment.
+			// We might however get here if the server notices the object being detached before the client.
 			m_position = readV3F1000(is);
 			m_velocity = readV3F1000(is);
 			m_acceleration = readV3F1000(is);
@@ -1567,14 +1580,14 @@ public:
 			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)
 				m_position += v3f(0,0.002,0);
-				
+
+			if(getParent() != NULL) // Just in case
+				return;
+
 			if(do_interpolate){
 				if(!m_prop.physical)
 					pos_translator.update(m_position, is_end_position, update_interval);