Complete the attachment framework.

The child ID can now be checked against the parent ID in content_cao.cpp so the parent can be detected. Actual attachment code to come

Divide attachment system between server attachments and client attachments, neither coded right now. As explained in the code comment:

// Attachments need to be handled on both the server and client.
// If we attach only on the server, models (which are client-side)
// can't be read so we don't know the origin and orientation of bones.
// If we attach only on the client, the real position of attachments is
// not updated and you can't click them for example.

6 files changed, 263 insertions, 145 deletions

diff --git a/src/content_cao.cpp b/src/content_cao.cpp
index 3e18337d2..613ec6153 100644
--- a/src/content_cao.cpp
+++ b/src/content_cao.cpp
@@ -581,6 +581,10 @@ private:
 	int m_frame_speed;
 	int m_frame_blend;
 	std::map<std::string, core::vector2d<v3f> > m_bone_posrot;
+	ClientActiveObject* m_attachment_parent;
+	std::string m_attachment_bone;
+	v3f m_attacmhent_position;
+	v3f m_attachment_rotation;
 	int m_anim_frame;
 	int m_anim_num_frames;
 	float m_anim_framelength;
@@ -615,6 +619,14 @@ public:
 		m_tx_basepos(0,0),
 		m_initial_tx_basepos_set(false),
 		m_tx_select_horiz_by_yawpitch(false),
+		m_frames(v2f(0,0)),
+		m_frame_speed(15),
+		m_frame_blend(0),
+		// Nothing to do for m_bone_posrot
+		m_attachment_parent(NULL),
+		m_attachment_bone(""),
+		m_attacmhent_position(v3f(0,0,0)),
+		m_attachment_rotation(v3f(0,0,0)),
 		m_anim_frame(0),
 		m_anim_num_frames(1),
 		m_anim_framelength(0.2),
@@ -906,6 +918,9 @@ public:
 
 	void updateNodePos()
 	{
+		if(m_attachment_parent != NULL)
+			return;
+
 		if(m_meshnode){
 			m_meshnode->setPosition(pos_translator.vect_show);
 			v3f rot = m_meshnode->getRotation();
@@ -933,51 +948,53 @@ public:
 			addToScene(m_smgr, m_gamedef->tsrc(), m_irr);
 			updateAnimations();
 			updateBonePosRot();
-			updateAttachment();
 		}
 
-		if(m_prop.physical){
-			core::aabbox3d<f32> box = m_prop.collisionbox;
-			box.MinEdge *= BS;
-			box.MaxEdge *= BS;
-			collisionMoveResult moveresult;
-			f32 pos_max_d = BS*0.125; // Distance per iteration
-			f32 stepheight = 0;
-			v3f p_pos = m_position;
-			v3f p_velocity = m_velocity;
-			v3f p_acceleration = m_acceleration;
-			IGameDef *gamedef = env->getGameDef();
-			moveresult = collisionMoveSimple(&env->getMap(), gamedef,
-					pos_max_d, box, stepheight, dtime,
-					p_pos, p_velocity, p_acceleration);
-			// Apply results
-			m_position = p_pos;
-			m_velocity = p_velocity;
-			m_acceleration = p_acceleration;
-			
-			bool is_end_position = moveresult.collides;
-			pos_translator.update(m_position, is_end_position, dtime);
-			pos_translator.translate(dtime);
-			updateNodePos();
-		} else {
-			m_position += dtime * m_velocity + 0.5 * dtime * dtime * m_acceleration;
-			m_velocity += dtime * m_acceleration;
-			pos_translator.update(m_position, pos_translator.aim_is_end, pos_translator.anim_time);
-			pos_translator.translate(dtime);
-			updateNodePos();
-		}
+		if(m_attachment_parent == NULL) // Attachments should be glued to their parent by Irrlicht
+		{
+			if(m_prop.physical){
+				core::aabbox3d<f32> box = m_prop.collisionbox;
+				box.MinEdge *= BS;
+				box.MaxEdge *= BS;
+				collisionMoveResult moveresult;
+				f32 pos_max_d = BS*0.125; // Distance per iteration
+				f32 stepheight = 0;
+				v3f p_pos = m_position;
+				v3f p_velocity = m_velocity;
+				v3f p_acceleration = m_acceleration;
+				IGameDef *gamedef = env->getGameDef();
+				moveresult = collisionMoveSimple(&env->getMap(), gamedef,
+						pos_max_d, box, stepheight, dtime,
+						p_pos, p_velocity, p_acceleration);
+				// Apply results
+				m_position = p_pos;
+				m_velocity = p_velocity;
+				m_acceleration = p_acceleration;
+				
+				bool is_end_position = moveresult.collides;
+				pos_translator.update(m_position, is_end_position, dtime);
+				pos_translator.translate(dtime);
+				updateNodePos();
+			} else {
+				m_position += dtime * m_velocity + 0.5 * dtime * dtime * m_acceleration;
+				m_velocity += dtime * m_acceleration;
+				pos_translator.update(m_position, pos_translator.aim_is_end, pos_translator.anim_time);
+				pos_translator.translate(dtime);
+				updateNodePos();
+			}
 
-		float moved = lastpos.getDistanceFrom(pos_translator.vect_show);
-		m_step_distance_counter += moved;
-		if(m_step_distance_counter > 1.5*BS){
-			m_step_distance_counter = 0;
-			if(!m_is_local_player && m_prop.makes_footstep_sound){
-				INodeDefManager *ndef = m_gamedef->ndef();
-				v3s16 p = floatToInt(getPosition() + v3f(0,
-						(m_prop.collisionbox.MinEdge.Y-0.5)*BS, 0), BS);
-				MapNode n = m_env->getMap().getNodeNoEx(p);
-				SimpleSoundSpec spec = ndef->get(n).sound_footstep;
-				m_gamedef->sound()->playSoundAt(spec, false, getPosition());
+			float moved = lastpos.getDistanceFrom(pos_translator.vect_show);
+			m_step_distance_counter += moved;
+			if(m_step_distance_counter > 1.5*BS){
+				m_step_distance_counter = 0;
+				if(!m_is_local_player && m_prop.makes_footstep_sound){
+					INodeDefManager *ndef = m_gamedef->ndef();
+					v3s16 p = floatToInt(getPosition() + v3f(0,
+							(m_prop.collisionbox.MinEdge.Y-0.5)*BS, 0), BS);
+					MapNode n = m_env->getMap().getNodeNoEx(p);
+					SimpleSoundSpec spec = ndef->get(n).sound_footstep;
+					m_gamedef->sound()->playSoundAt(spec, false, getPosition());
+				}
 			}
 		}
 
@@ -1193,10 +1210,38 @@ public:
 			}
 		}
 	}
-
-	void updateAttachment()
+	
+	void updateAttachments()
 	{
-		// Code for attachments goes here
+		// 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
+		// Irrlicht documentation: http://irrlicht.sourceforge.net/docu/
 	}
 
 	void processMessage(const std::string &data)
@@ -1225,6 +1270,8 @@ public:
 		}
 		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);
@@ -1238,7 +1285,7 @@ public:
 			// the ground due to sucky collision detection...
 			if(m_prop.physical)
 				m_position += v3f(0,0.002,0);
-			
+				
 			if(do_interpolate){
 				if(!m_prop.physical)
 					pos_translator.update(m_position, is_end_position, update_interval);
@@ -1287,12 +1334,18 @@ public:
 		}
 		else if(cmd == GENERIC_CMD_SET_ATTACHMENT)
 		{
-			// Part of the attachment structure, not used yet!
-
-			// Get properties here.
+			ClientActiveObject *obj;
+			int parent_id = readS16(is);
+			if(parent_id > 0)
+				obj = m_env->getActiveObject(parent_id);
+			else
+				obj = NULL;
+			m_attachment_parent = obj;
+			m_attachment_bone = deSerializeString(is);
+			m_attacmhent_position = readV3F1000(is);
+			m_attachment_rotation = readV3F1000(is);
 
-			updateAttachment();
-			expireVisuals();
+			updateAttachments();
 		}
 		else if(cmd == GENERIC_CMD_PUNCHED)
 		{
diff --git a/src/content_sao.cpp b/src/content_sao.cpp
index b23d287f4..7787e33fb 100644
--- a/src/content_sao.cpp
+++ b/src/content_sao.cpp
@@ -218,6 +218,7 @@ public:
 
 		if(pos_f.getDistanceFrom(m_last_sent_position) > 0.05*BS)
 		{
+			// TODO: We shouldn't be sending this when the object is attached, but we can't check m_parent here
 			setBasePosition(pos_f);
 			m_last_sent_position = pos_f;
 
@@ -383,6 +384,7 @@ void LuaEntitySAO::addedToEnvironment(u32 dtime_s)
 	// Create entity from name
 	lua_State *L = m_env->getLua();
 	m_registered = scriptapi_luaentity_add(L, m_id, m_init_name.c_str());
+	m_parent = NULL;
 	
 	if(m_registered){
 		// Get properties
@@ -442,28 +444,39 @@ void LuaEntitySAO::step(float dtime, bool send_recommended)
 
 	m_last_sent_position_timer += dtime;
 	
-	if(m_prop.physical){
-		core::aabbox3d<f32> box = m_prop.collisionbox;
-		box.MinEdge *= BS;
-		box.MaxEdge *= BS;
-		collisionMoveResult moveresult;
-		f32 pos_max_d = BS*0.25; // Distance per iteration
-		f32 stepheight = 0; // Maximum climbable step height
-		v3f p_pos = m_base_position;
-		v3f p_velocity = m_velocity;
-		v3f p_acceleration = m_acceleration;
-		IGameDef *gamedef = m_env->getGameDef();
-		moveresult = collisionMoveSimple(&m_env->getMap(), gamedef,
-				pos_max_d, box, stepheight, dtime,
-				p_pos, p_velocity, p_acceleration);
-		// Apply results
-		m_base_position = p_pos;
-		m_velocity = p_velocity;
-		m_acceleration = p_acceleration;
-	} else {
-		m_base_position += dtime * m_velocity + 0.5 * dtime
-				* dtime * m_acceleration;
-		m_velocity += dtime * m_acceleration;
+	if(m_parent != NULL)
+	{
+		// REMAINING ATTACHMENT ISSUES:
+		// This is causing a segmentation fault, investigate why!
+		//m_base_position = m_parent->getBasePosition();
+		m_velocity = v3f(0,0,0);
+		m_acceleration = v3f(0,0,0);
+	}
+	else
+	{
+		if(m_prop.physical){
+			core::aabbox3d<f32> box = m_prop.collisionbox;
+			box.MinEdge *= BS;
+			box.MaxEdge *= BS;
+			collisionMoveResult moveresult;
+			f32 pos_max_d = BS*0.25; // Distance per iteration
+			f32 stepheight = 0; // Maximum climbable step height
+			v3f p_pos = m_base_position;
+			v3f p_velocity = m_velocity;
+			v3f p_acceleration = m_acceleration;
+			IGameDef *gamedef = m_env->getGameDef();
+			moveresult = collisionMoveSimple(&m_env->getMap(), gamedef,
+					pos_max_d, box, stepheight, dtime,
+					p_pos, p_velocity, p_acceleration);
+			// Apply results
+			m_base_position = p_pos;
+			m_velocity = p_velocity;
+			m_acceleration = p_acceleration;
+		} else {
+			m_base_position += dtime * m_velocity + 0.5 * dtime
+					* dtime * m_acceleration;
+			m_velocity += dtime * m_acceleration;
+		}
 	}
 
 	if(m_registered){
@@ -551,6 +564,10 @@ int LuaEntitySAO::punch(v3f dir,
 		m_removed = true;
 		return 0;
 	}
+
+	// It's best that attachments cannot be punched 
+	if(m_parent != NULL)
+		return 0;
 	
 	ItemStack *punchitem = NULL;
 	ItemStack punchitem_static;
@@ -601,12 +618,16 @@ void LuaEntitySAO::rightClick(ServerActiveObject *clicker)
 
 void LuaEntitySAO::setPos(v3f pos)
 {
+	if(m_parent != NULL)
+		return;
 	m_base_position = pos;
 	sendPosition(false, true);
 }
 
 void LuaEntitySAO::moveTo(v3f pos, bool continuous)
 {
+	if(m_parent != NULL)
+		return;
 	m_base_position = pos;
 	if(!continuous)
 		sendPosition(true, true);
@@ -661,14 +682,21 @@ void LuaEntitySAO::setBonePosRot(std::string bone, v3f position, v3f rotation)
 	m_messages_out.push_back(aom);
 }
 
-// Part of the attachment structure, not used yet!
 void LuaEntitySAO::setAttachment(ServerActiveObject *parent, std::string bone, v3f position, v3f rotation)
 {
-	// Parent should be translated from a ServerActiveObject into something
-	// the client will recognize (as a ClientActiveObject) then sent in
-	// gob_cmd_set_attachment that way.
+	// Attachments need to be handled on both the server and client.
+	// If we just attach on the server, we can only copy the position of the parent. Attachments
+	// are still sent to clients at an interval so players would see them following the parent
+	// instead of sticking to it, plus we can't read and attach to skeletal bones.
+	// If we just attach on the client, the server still sees the child at its original location.
+	// This can break some things, so we also give the server the most accurate representation
+	// even if players will only see the client changes since they override server-sent position.
 
-	std::string str = gob_cmd_set_attachment(); // <- parameters here
+	// Server attachment:
+	m_parent = parent;
+
+	// Client attachment:
+	std::string str = gob_cmd_set_attachment(parent->getId(), bone, position, rotation);
 	// create message and add to list
 	ActiveObjectMessage aom(getId(), true, str);
 	m_messages_out.push_back(aom);
@@ -748,6 +776,9 @@ std::string LuaEntitySAO::getPropertyPacket()
 
 void LuaEntitySAO::sendPosition(bool do_interpolate, bool is_movement_end)
 {
+	if(m_parent != NULL)
+		return;
+	
 	m_last_sent_move_precision = m_base_position.getDistanceFrom(
 			m_last_sent_position);
 	m_last_sent_position_timer = 0;
@@ -843,6 +874,7 @@ void PlayerSAO::addedToEnvironment(u32 dtime_s)
 {
 	ServerActiveObject::addedToEnvironment(dtime_s);
 	ServerActiveObject::setBasePosition(m_player->getPosition());
+	m_parent = NULL;
 	m_player->setPlayerSAO(this);
 	m_player->peer_id = m_peer_id;
 	m_last_good_position = m_player->getPosition();
@@ -893,7 +925,7 @@ std::string PlayerSAO::getStaticData()
 }
 
 void PlayerSAO::step(float dtime, bool send_recommended)
-{
+{	
 	if(!m_properties_sent)
 	{
 		m_properties_sent = true;
@@ -905,71 +937,82 @@ void PlayerSAO::step(float dtime, bool send_recommended)
 
 	m_time_from_last_punch += dtime;
 	m_nocheat_dig_time += dtime;
-	
-	if(m_is_singleplayer || g_settings->getBool("disable_anticheat"))
-	{
-		m_last_good_position = m_player->getPosition();
-		m_last_good_position_age = 0;
-	}
-	else
+
+	if(m_parent == NULL)
 	{
-		/*
-			Check player movements
-
-			NOTE: Actually the server should handle player physics like the
-			client does and compare player's position to what is calculated
-			on our side. This is required when eg. players fly due to an
-			explosion. Altough a node-based alternative might be possible
-			too, and much more lightweight.
-		*/
-
-		float player_max_speed = 0;
-		float player_max_speed_up = 0;
-		if(m_privs.count("fast") != 0){
-			// Fast speed
-			player_max_speed = BS * 20;
-			player_max_speed_up = BS * 20;
-		} else {
-			// Normal speed
-			player_max_speed = BS * 4.0;
-			player_max_speed_up = BS * 4.0;
+		if(m_is_singleplayer || g_settings->getBool("disable_anticheat"))
+		{
+			m_last_good_position = m_player->getPosition();
+			m_last_good_position_age = 0;
 		}
-		// Tolerance
-		player_max_speed *= 2.5;
-		player_max_speed_up *= 2.5;
-
-		m_last_good_position_age += dtime;
-		if(m_last_good_position_age >= 1.0){
-			float age = m_last_good_position_age;
-			v3f diff = (m_player->getPosition() - m_last_good_position);
-			float d_vert = diff.Y;
-			diff.Y = 0;
-			float d_horiz = diff.getLength();
-			/*infostream<<m_player->getName()<<"'s horizontal speed is "
-					<<(d_horiz/age)<<std::endl;*/
-			if(d_horiz <= age * player_max_speed &&
-					(d_vert < 0 || d_vert < age * player_max_speed_up)){
-				m_last_good_position = m_player->getPosition();
+		else
+		{
+			/*
+				Check player movements
+
+				NOTE: Actually the server should handle player physics like the
+				client does and compare player's position to what is calculated
+				on our side. This is required when eg. players fly due to an
+				explosion. Altough a node-based alternative might be possible
+				too, and much more lightweight.
+			*/
+
+			float player_max_speed = 0;
+			float player_max_speed_up = 0;
+			if(m_privs.count("fast") != 0){
+				// Fast speed
+				player_max_speed = BS * 20;
+				player_max_speed_up = BS * 20;
 			} else {
-				actionstream<<"Player "<<m_player->getName()
-						<<" moved too fast; resetting position"
-						<<std::endl;
-				m_player->setPosition(m_last_good_position);
-				m_teleported = true;
+				// Normal speed
+				player_max_speed = BS * 4.0;
+				player_max_speed_up = BS * 4.0;
+			}
+			// Tolerance
+			player_max_speed *= 2.5;
+			player_max_speed_up *= 2.5;
+
+			m_last_good_position_age += dtime;
+			if(m_last_good_position_age >= 1.0){
+				float age = m_last_good_position_age;
+				v3f diff = (m_player->getPosition() - m_last_good_position);
+				float d_vert = diff.Y;
+				diff.Y = 0;
+				float d_horiz = diff.getLength();
+				/*infostream<<m_player->getName()<<"'s horizontal speed is "
+						<<(d_horiz/age)<<std::endl;*/
+				if(d_horiz <= age * player_max_speed &&
+						(d_vert < 0 || d_vert < age * player_max_speed_up)){
+					m_last_good_position = m_player->getPosition();
+				} else {
+					actionstream<<"Player "<<m_player->getName()
+							<<" moved too fast; resetting position"
+							<<std::endl;
+					m_player->setPosition(m_last_good_position);
+					m_teleported = true;
+				}
+				m_last_good_position_age = 0;
 			}
-			m_last_good_position_age = 0;
 		}
 	}
 
 	if(send_recommended == false)
 		return;
 
-	if(m_position_not_sent)
+	// If the object is attached client-side, don't waste bandwidth and send its position to clients
+	if(m_position_not_sent && m_parent == NULL)
 	{
 		m_position_not_sent = false;
 		float update_interval = m_env->getSendRecommendedInterval();
+		v3f pos;
+		// REMAINING ATTACHMENT ISSUES:
+		// This is causing a segmentation fault, investigate why!
+		if(m_parent != NULL)
+			pos = m_parent->getBasePosition();
+		else
+			pos = m_player->getPosition() + v3f(0,BS*1,0);
 		std::string str = gob_cmd_update_position(
-			m_player->getPosition() + v3f(0,BS*1,0),
+			pos,
 			v3f(0,0,0),
 			v3f(0,0,0),
 			m_player->getYaw(),
@@ -1000,12 +1043,16 @@ void PlayerSAO::step(float dtime, bool send_recommended)
 
 void PlayerSAO::setBasePosition(const v3f &position)
 {
+	if(m_parent != NULL)
+		return;
 	ServerActiveObject::setBasePosition(position);
 	m_position_not_sent = true;
 }
 
 void PlayerSAO::setPos(v3f pos)
 {
+	if(m_parent != NULL)
+		return;
 	m_player->setPosition(pos);
 	// Movement caused by this command is always valid
 	m_last_good_position = pos;
@@ -1016,6 +1063,8 @@ void PlayerSAO::setPos(v3f pos)
 
 void PlayerSAO::moveTo(v3f pos, bool continuous)
 {
+	if(m_parent != NULL)
+		return;
 	m_player->setPosition(pos);
 	// Movement caused by this command is always valid
 	m_last_good_position = pos;
@@ -1029,6 +1078,10 @@ int PlayerSAO::punch(v3f dir,
 	ServerActiveObject *puncher,
 	float time_from_last_punch)
 {
+	// It's best that attachments cannot be punched 
+	if(m_parent != NULL)
+		return 0;
+
 	if(!toolcap)
 		return 0;
 
@@ -1126,10 +1179,21 @@ void PlayerSAO::setBonePosRot(std::string bone, v3f position, v3f rotation)
 	m_messages_out.push_back(aom);
 }
 
-// Part of the attachment structure, not used yet!
 void PlayerSAO::setAttachment(ServerActiveObject *parent, std::string bone, v3f position, v3f rotation)
-{	
-	std::string str = gob_cmd_set_attachment(); // <- parameters here
+{
+	// Attachments need to be handled on both the server and client.
+	// If we just attach on the server, we can only copy the position of the parent. Attachments
+	// are still sent to clients at an interval so players would see them following the parent
+	// instead of sticking to it, plus we can't read and attach to skeletal bones.
+	// If we just attach on the client, the server still sees the child at its original location.
+	// This can break some things, so we also give the server the most accurate representation
+	// even if players will only see the client changes since they override server-sent position.
+
+	// Server attachment:
+	m_parent = parent;
+
+	// Client attachment:
+	std::string str = gob_cmd_set_attachment(parent->getId(), bone, position, rotation);
 	// create message and add to list
 	ActiveObjectMessage aom(getId(), true, str);
 	m_messages_out.push_back(aom);
diff --git a/src/content_sao.h b/src/content_sao.h
index a89f2ddd4..e4d81cd2d 100644
--- a/src/content_sao.h
+++ b/src/content_sao.h
@@ -99,6 +99,7 @@ private:
 	float m_last_sent_position_timer;
 	float m_last_sent_move_precision;
 	bool m_armor_groups_sent;
+	ServerActiveObject *m_parent;
 };
 
 /*
@@ -235,6 +236,7 @@ private:
 	bool m_position_not_sent;
 	ItemGroupList m_armor_groups;
 	bool m_armor_groups_sent;
+	ServerActiveObject *m_parent;
 	bool m_properties_sent;
 	struct ObjectProperties m_prop;
 	// Cached privileges for enforcement
diff --git a/src/genericobject.cpp b/src/genericobject.cpp
index 482dbbc78..480c4209d 100644
--- a/src/genericobject.cpp
+++ b/src/genericobject.cpp
@@ -104,23 +104,25 @@ std::string gob_cmd_set_animations(v2f frames, float frame_speed, float frame_bl
 	return os.str();
 }
 
-// Part of the attachment structure, not used yet!
-std::string gob_cmd_set_attachment() // <- parameters here
+std::string gob_cmd_set_bone_posrot(std::string bone, v3f position, v3f rotation)
 {
 	std::ostringstream os(std::ios::binary);
 	// command 
-	writeU8(os, GENERIC_CMD_SET_ATTACHMENT);
+	writeU8(os, GENERIC_CMD_SET_BONE_POSROT);
 	// parameters
-	// Parameters go here
+	os<<serializeString(bone);
+	writeV3F1000(os, position);
+	writeV3F1000(os, rotation);
 	return os.str();
 }
 
-std::string gob_cmd_set_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::ostringstream os(std::ios::binary);
 	// command 
-	writeU8(os, GENERIC_CMD_SET_BONE_POSROT);
+	writeU8(os, GENERIC_CMD_SET_ATTACHMENT);
 	// parameters
+	writeS16(os, parent_id);
 	os<<serializeString(bone);
 	writeV3F1000(os, position);
 	writeV3F1000(os, rotation);
diff --git a/src/genericobject.h b/src/genericobject.h
index d20e7b3d0..3504b0a46 100644
--- a/src/genericobject.h
+++ b/src/genericobject.h
@@ -61,7 +61,7 @@ std::string gob_cmd_set_animations(v2f frames, float frame_speed, float frame_bl
 
 std::string gob_cmd_set_bone_posrot(std::string bone, v3f position, v3f rotation);
 
-std::string gob_cmd_set_attachment(); // <- parameters here
+std::string gob_cmd_set_attachment(int parent_id, std::string bone, v3f position, v3f rotation);
 
 std::string gob_cmd_punched(s16 damage, s16 result_hp);
 
diff --git a/src/scriptapi.cpp b/src/scriptapi.cpp
index 9c5bbcc5b..3c505c881 100644
--- a/src/scriptapi.cpp
+++ b/src/scriptapi.cpp
@@ -2737,7 +2737,7 @@ private:
 		return 0;
 	}
 
-	// setboneposrot(std::string bone, v3f position, v3f rotation)
+	// setboneposrot(self, std::string bone, v3f position, v3f rotation)
 	static int l_set_bone_posrot(lua_State *L)
 	{
 		ObjectRef *ref = checkobject(L, 1);
@@ -2758,8 +2758,7 @@ private:
 		return 0;
 	}
 
-// Part of the attachment structure, not used yet!
-	// set_attachment() // <- parameters here
+	// set_attachment(self, parent, bone, position, rotation)
 	static int l_set_attachment(lua_State *L)
 	{
 		ObjectRef *ref = checkobject(L, 1);
@@ -2778,8 +2777,6 @@ private:
 		if(!lua_isnil(L, 5))
 			rotation = read_v3f(L, 5);
 		// Do it
-	
-//lua_pushnumber(L, cobj->getId()); // Push id
 
 		co->setAttachment(parent, bone, position, rotation);
 		return 0;