Attachments: Fix interpolation from (0,0,0) after detach

GenericCAO::getPosition() did not take the camera offset into account
LocalPlayer attachment cleanup: Use sane getParent() function
Make that getPosition() (GenericCAO and LocalPlayer) always return the absolute position

7 files changed, 57 insertions, 61 deletions

diff --git a/src/client/camera.cpp b/src/client/camera.cpp
index d1e76026d..464644044 100644
--- a/src/client/camera.cpp
+++ b/src/client/camera.cpp
@@ -285,9 +285,13 @@ void Camera::update(LocalPlayer* player, f32 frametime, f32 busytime, f32 tool_r
 	// Smooth the movement when walking up stairs
 	v3f old_player_position = m_playernode->getPosition();
 	v3f player_position = player->getPosition();
-	if (player->isAttached && player->parent)
-		player_position = player->parent->getPosition();
-	//if(player->touching_ground && player_position.Y > old_player_position.Y)
+
+	// This is worse than `LocalPlayer::getPosition()` but
+	// mods expect the player head to be at the parent's position
+	// plus eye height.
+	if (player->getParent())
+		player_position = player->getParent()->getPosition();
+
 	if(player->touching_ground &&
 			player_position.Y > old_player_position.Y)
 	{
diff --git a/src/client/clientobject.h b/src/client/clientobject.h
index c673fff9a..12e0db35b 100644
--- a/src/client/clientobject.h
+++ b/src/client/clientobject.h
@@ -49,8 +49,10 @@ public:
 	virtual bool getSelectionBox(aabb3f *toset) const { return false; }
 	virtual bool collideWithObjects() const { return false; }
 	virtual const v3f getPosition() const { return v3f(0.0f); }
-	virtual scene::ISceneNode *getSceneNode() { return NULL; }
-	virtual scene::IAnimatedMeshSceneNode *getAnimatedMeshSceneNode() { return NULL; }
+	virtual scene::ISceneNode *getSceneNode() const
+	{ return NULL; }
+	virtual scene::IAnimatedMeshSceneNode *getAnimatedMeshSceneNode() const
+	{ return NULL; }
 	virtual bool isLocalPlayer() const { return false; }
 
 	virtual ClientActiveObject *getParent() const { return nullptr; };
diff --git a/src/client/content_cao.cpp b/src/client/content_cao.cpp
index 5521a6cf1..0863f377c 100644
--- a/src/client/content_cao.cpp
+++ b/src/client/content_cao.cpp
@@ -403,13 +403,17 @@ bool GenericCAO::getSelectionBox(aabb3f *toset) const
 
 const v3f GenericCAO::getPosition() const
 {
-	if (getParent() != nullptr) {
-		if (m_matrixnode)
-			return m_matrixnode->getAbsolutePosition();
+	if (!getParent())
+		return pos_translator.val_current;
 
-		return m_position;
+	// Calculate real position in world based on MatrixNode
+	if (m_matrixnode) {
+		v3s16 camera_offset = m_env->getCameraOffset();
+		return m_matrixnode->getAbsolutePosition() +
+				intToFloat(camera_offset, BS);
 	}
-	return pos_translator.val_current;
+
+	return m_position;
 }
 
 const bool GenericCAO::isImmortal()
@@ -417,7 +421,7 @@ const bool GenericCAO::isImmortal()
 	return itemgroup_get(getGroups(), "immortal");
 }
 
-scene::ISceneNode* GenericCAO::getSceneNode()
+scene::ISceneNode *GenericCAO::getSceneNode() const
 {
 	if (m_meshnode) {
 		return m_meshnode;
@@ -437,7 +441,7 @@ scene::ISceneNode* GenericCAO::getSceneNode()
 	return NULL;
 }
 
-scene::IAnimatedMeshSceneNode* GenericCAO::getAnimatedMeshSceneNode()
+scene::IAnimatedMeshSceneNode *GenericCAO::getAnimatedMeshSceneNode() const
 {
 	return m_animated_meshnode;
 }
@@ -576,11 +580,15 @@ void GenericCAO::addToScene(ITextureSource *tsrc)
 	video::E_MATERIAL_TYPE material_type = (m_prop.use_texture_alpha) ?
 		video::EMT_TRANSPARENT_ALPHA_CHANNEL : video::EMT_TRANSPARENT_ALPHA_CHANNEL_REF;
 
-	if (m_prop.visual == "sprite") {
-		infostream<<"GenericCAO::addToScene(): single_sprite"<<std::endl;
+	auto grabMatrixNode = [this] {
+		infostream << "GenericCAO::addToScene(): " << m_prop.visual << std::endl;
 		m_matrixnode = RenderingEngine::get_scene_manager()->
 				addDummyTransformationSceneNode();
 		m_matrixnode->grab();
+	};
+
+	if (m_prop.visual == "sprite") {
+		getMatrixNode();
 		m_spritenode = RenderingEngine::get_scene_manager()->addBillboardSceneNode(
 				m_matrixnode, v2f(1, 1), v3f(0,0,0), -1);
 		m_spritenode->grab();
@@ -601,6 +609,7 @@ void GenericCAO::addToScene(ITextureSource *tsrc)
 					txs, tys, 0, 0);
 		}
 	} else if (m_prop.visual == "upright_sprite") {
+		getMatrixNode();
 		scene::SMesh *mesh = new scene::SMesh();
 		double dx = BS * m_prop.visual_size.X / 2;
 		double dy = BS * m_prop.visual_size.Y / 2;
@@ -655,9 +664,6 @@ void GenericCAO::addToScene(ITextureSource *tsrc)
 			mesh->addMeshBuffer(buf);
 			buf->drop();
 		}
-		m_matrixnode = RenderingEngine::get_scene_manager()->
-			addDummyTransformationSceneNode();
-		m_matrixnode->grab();
 		m_meshnode = RenderingEngine::get_scene_manager()->
 			addMeshSceneNode(mesh, m_matrixnode);
 		m_meshnode->grab();
@@ -666,11 +672,8 @@ void GenericCAO::addToScene(ITextureSource *tsrc)
 		// This is needed for changing the texture in the future
 		m_meshnode->setReadOnlyMaterials(true);
 	} else if (m_prop.visual == "cube") {
-		infostream<<"GenericCAO::addToScene(): cube"<<std::endl;
+		getMatrixNode();
 		scene::IMesh *mesh = createCubeMesh(v3f(BS,BS,BS));
-		m_matrixnode = RenderingEngine::get_scene_manager()->
-			addDummyTransformationSceneNode(nullptr);
-		m_matrixnode->grab();
 		m_meshnode = RenderingEngine::get_scene_manager()->
 			addMeshSceneNode(mesh, m_matrixnode);
 		m_meshnode->grab();
@@ -685,12 +688,9 @@ void GenericCAO::addToScene(ITextureSource *tsrc)
 		m_meshnode->setMaterialType(material_type);
 		m_meshnode->setMaterialFlag(video::EMF_FOG_ENABLE, true);
 	} else if (m_prop.visual == "mesh") {
-		infostream<<"GenericCAO::addToScene(): mesh"<<std::endl;
+		getMatrixNode();
 		scene::IAnimatedMesh *mesh = m_client->getMesh(m_prop.mesh, true);
 		if (mesh) {
-			m_matrixnode = RenderingEngine::get_scene_manager()->
-				addDummyTransformationSceneNode(nullptr);
-			m_matrixnode->grab();
 			m_animated_meshnode = RenderingEngine::get_scene_manager()->
 				addAnimatedMeshSceneNode(mesh, m_matrixnode);
 			m_animated_meshnode->grab();
@@ -713,8 +713,8 @@ void GenericCAO::addToScene(ITextureSource *tsrc)
 		} else
 			errorstream<<"GenericCAO::addToScene(): Could not load mesh "<<m_prop.mesh<<std::endl;
 	} else if (m_prop.visual == "wielditem" || m_prop.visual == "item") {
+		getMatrixNode();
 		ItemStack item;
-		infostream << "GenericCAO::addToScene(): wielditem" << std::endl;
 		if (m_prop.wield_item.empty()) {
 			// Old format, only textures are specified.
 			infostream << "textures: " << m_prop.textures.size() << std::endl;
@@ -728,12 +728,8 @@ void GenericCAO::addToScene(ITextureSource *tsrc)
 			infostream << "serialized form: " << m_prop.wield_item << std::endl;
 			item.deSerialize(m_prop.wield_item, m_client->idef());
 		}
-		m_matrixnode = RenderingEngine::get_scene_manager()->
-			addDummyTransformationSceneNode(nullptr);
-		m_matrixnode->grab();
 		m_wield_meshnode = new WieldMeshSceneNode(
 			RenderingEngine::get_scene_manager(), -1);
-		m_wield_meshnode->setParent(m_matrixnode);
 		m_wield_meshnode->setItem(item, m_client,
 			(m_prop.visual == "wielditem"));
 
@@ -751,6 +747,9 @@ void GenericCAO::addToScene(ITextureSource *tsrc)
 
 	scene::ISceneNode *node = getSceneNode();
 
+	if (node && m_matrixnode)
+		node->setParent(m_matrixnode);
+
 	if (node && !m_prop.nametag.empty() && !m_is_local_player) {
 		// Add nametag
 		v3f pos;
@@ -884,7 +883,7 @@ void GenericCAO::step(float dtime, ClientEnvironment *env)
 
 			// Apply animations if input detected and not attached
 			// or set idle animation
-			if ((new_anim.X + new_anim.Y) > 0 && !player->isAttached) {
+			if ((new_anim.X + new_anim.Y) > 0 && !getParent()) {
 				allow_update = true;
 				m_animation_range = new_anim;
 				m_animation_speed = new_speed;
@@ -946,12 +945,7 @@ void GenericCAO::step(float dtime, ClientEnvironment *env)
 		m_velocity = v3f(0,0,0);
 		m_acceleration = v3f(0,0,0);
 		pos_translator.val_current = m_position;
-
-		if(m_is_local_player) // Update local player attachment position
-		{
-			LocalPlayer *player = m_env->getLocalPlayer();
-			player->overridePosition = getParent()->getPosition();
-		}
+		pos_translator.val_target = m_position;
 	} else {
 		rot_translator.translate(dtime);
 		v3f lastpos = pos_translator.val_current;
@@ -975,16 +969,14 @@ void GenericCAO::step(float dtime, ClientEnvironment *env)
 
 			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();
 		}
+		pos_translator.translate(dtime);
+		updateNodePos();
 
 		float moved = lastpos.getDistanceFrom(pos_translator.val_current);
 		m_step_distance_counter += moved;
@@ -1348,7 +1340,8 @@ void GenericCAO::updateAttachments()
 
 	if (!parent) { // Detach or don't attach
 		if (m_matrixnode) {
-			v3f old_pos = m_matrixnode->getAbsolutePosition();
+			v3f old_pos = getPosition();
+
 			m_matrixnode->setParent(m_smgr->getRootSceneNode());
 			getPosRotMatrix().setTranslation(old_pos);
 			m_matrixnode->updateAbsolutePosition();
@@ -1372,11 +1365,6 @@ void GenericCAO::updateAttachments()
 			m_matrixnode->updateAbsolutePosition();
 		}
 	}
-	if (m_is_local_player) {
-		LocalPlayer *player = m_env->getLocalPlayer();
-		player->isAttached = parent;
-		player->parent = parent;
-	}
 }
 
 void GenericCAO::processMessage(const std::string &data)
diff --git a/src/client/content_cao.h b/src/client/content_cao.h
index 2c2d11077..6f3b2f06b 100644
--- a/src/client/content_cao.h
+++ b/src/client/content_cao.h
@@ -165,9 +165,9 @@ public:
 
 	const bool isImmortal();
 
-	scene::ISceneNode *getSceneNode();
+	scene::ISceneNode *getSceneNode() const;
 
-	scene::IAnimatedMeshSceneNode *getAnimatedMeshSceneNode();
+	scene::IAnimatedMeshSceneNode *getAnimatedMeshSceneNode() const;
 
 	// m_matrixnode controls the position and rotation of the child node
 	// for all scene nodes, as a workaround for an Irrlicht problem with
diff --git a/src/client/localplayer.cpp b/src/client/localplayer.cpp
index c086d860a..c20c3619f 100644
--- a/src/client/localplayer.cpp
+++ b/src/client/localplayer.cpp
@@ -184,8 +184,8 @@ void LocalPlayer::move(f32 dtime, Environment *env, f32 pos_max_d,
 	v3f position = getPosition();
 
 	// Copy parent position if local player is attached
-	if (isAttached) {
-		setPosition(overridePosition);
+	if (getParent()) {
+		setPosition(m_cao->getPosition());
 		added_velocity = v3f(0.0f); // ignored
 		return;
 	}
@@ -474,7 +474,7 @@ void LocalPlayer::applyControl(float dtime, Environment *env)
 	setYaw(control.yaw);
 
 	// Nullify speed and don't run positioning code if the player is attached
-	if (isAttached) {
+	if (getParent()) {
 		setSpeed(v3f(0.0f));
 		return;
 	}
@@ -706,6 +706,11 @@ v3f LocalPlayer::getEyeOffset() const
 	return v3f(0.0f, BS * eye_height, 0.0f);
 }
 
+ClientActiveObject *LocalPlayer::getParent() const
+{
+	return m_cao ? m_cao->getParent() : nullptr;
+}
+
 bool LocalPlayer::isDead() const
 {
 	FATAL_ERROR_IF(!getCAO(), "LocalPlayer's CAO isn't initialized");
@@ -764,8 +769,8 @@ void LocalPlayer::old_move(f32 dtime, Environment *env, f32 pos_max_d,
 	v3f position = getPosition();
 
 	// Copy parent position if local player is attached
-	if (isAttached) {
-		setPosition(overridePosition);
+	if (getParent()) {
+		setPosition(m_cao->getPosition());
 		m_sneak_node_exists = false;
 		added_velocity = v3f(0.0f);
 		return;
diff --git a/src/client/localplayer.h b/src/client/localplayer.h
index 45dc6776e..95dceb1f4 100644
--- a/src/client/localplayer.h
+++ b/src/client/localplayer.h
@@ -47,12 +47,9 @@ public:
 	LocalPlayer(Client *client, const char *name);
 	virtual ~LocalPlayer() = default;
 
-	ClientActiveObject *parent = nullptr;
-
 	// Initialize hp to 0, so that no hearts will be shown if server
 	// doesn't support health points
 	u16 hp = 0;
-	bool isAttached = false;
 	bool touching_ground = false;
 	// This oscillates so that the player jumps a bit above the surface
 	bool in_liquid = false;
@@ -72,8 +69,6 @@ public:
 	// Temporary option for old move code
 	bool physics_override_new_move = true;
 
-	v3f overridePosition;
-
 	void move(f32 dtime, Environment *env, f32 pos_max_d);
 	void move(f32 dtime, Environment *env, f32 pos_max_d,
 			std::vector<CollisionInfo> *collision_info);
@@ -112,6 +107,8 @@ public:
 
 	GenericCAO *getCAO() const { return m_cao; }
 
+	ClientActiveObject *getParent() const;
+
 	void setCAO(GenericCAO *toset)
 	{
 		assert(!m_cao); // Pre-condition
diff --git a/src/script/lua_api/l_localplayer.cpp b/src/script/lua_api/l_localplayer.cpp
index 3e14e48e4..821b1cb66 100644
--- a/src/script/lua_api/l_localplayer.cpp
+++ b/src/script/lua_api/l_localplayer.cpp
@@ -78,7 +78,7 @@ int LuaLocalPlayer::l_is_attached(lua_State *L)
 {
 	LocalPlayer *player = getobject(L, 1);
 
-	lua_pushboolean(L, player->isAttached);
+	lua_pushboolean(L, player->getParent() != nullptr);
 	return 1;
 }
 
@@ -157,7 +157,7 @@ int LuaLocalPlayer::l_get_override_pos(lua_State *L)
 {
 	LocalPlayer *player = getobject(L, 1);
 
-	push_v3f(L, player->overridePosition);
+	push_v3f(L, player->getPosition());
 	return 1;
 }