Server-side authority for attached players (#10952)

The server must have authority about attachments. This commit ignores any player movement packets as long they're attached.

3 files changed, 15 insertions, 35 deletions

diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp
index 270b8e01f..ddc6f4e47 100644
--- a/src/network/serverpackethandler.cpp
+++ b/src/network/serverpackethandler.cpp
@@ -488,8 +488,12 @@ void Server::process_PlayerPos(RemotePlayer *player, PlayerSAO *playersao,
 	pitch = modulo360f(pitch);
 	yaw = wrapDegrees_0_360(yaw);
 
-	playersao->setBasePosition(position);
-	player->setSpeed(speed);
+	if (!playersao->isAttached()) {
+		// Only update player positions when moving freely
+		// to not interfere with attachment handling
+		playersao->setBasePosition(position);
+		player->setSpeed(speed);
+	}
 	playersao->setLookPitch(pitch);
 	playersao->setPlayerYaw(yaw);
 	playersao->setFov(fov);
diff --git a/src/server/luaentity_sao.cpp b/src/server/luaentity_sao.cpp
index 5f35aaed8..3bcbe107b 100644
--- a/src/server/luaentity_sao.cpp
+++ b/src/server/luaentity_sao.cpp
@@ -146,15 +146,11 @@ void LuaEntitySAO::step(float dtime, bool send_recommended)
 
 	// Each frame, parent position is copied if the object is attached, otherwise it's calculated normally
 	// If the object gets detached this comes into effect automatically from the last known origin
-	if(isAttached())
-	{
-		v3f pos = m_env->getActiveObject(m_attachment_parent_id)->getBasePosition();
-		m_base_position = pos;
+	if (auto *parent = getParent()) {
+		m_base_position = parent->getBasePosition();
 		m_velocity = v3f(0,0,0);
 		m_acceleration = v3f(0,0,0);
-	}
-	else
-	{
+	} else {
 		if(m_prop.physical){
 			aabb3f box = m_prop.collisionbox;
 			box.MinEdge *= BS;
diff --git a/src/server/player_sao.cpp b/src/server/player_sao.cpp
index 110d2010d..0d31f2e0b 100644
--- a/src/server/player_sao.cpp
+++ b/src/server/player_sao.cpp
@@ -260,10 +260,13 @@ void PlayerSAO::step(float dtime, bool send_recommended)
 	// otherwise it's calculated normally.
 	// If the object gets detached this comes into effect automatically from
 	// the last known origin.
-	if (isAttached()) {
-		v3f pos = m_env->getActiveObject(m_attachment_parent_id)->getBasePosition();
+	if (auto *parent = getParent()) {
+		v3f pos = parent->getBasePosition();
 		m_last_good_position = pos;
 		setBasePosition(pos);
+
+		if (m_player)
+			m_player->setSpeed(v3f());
 	}
 
 	if (!send_recommended)
@@ -570,34 +573,11 @@ void PlayerSAO::setMaxSpeedOverride(const v3f &vel)
 bool PlayerSAO::checkMovementCheat()
 {
 	if (m_is_singleplayer ||
+			isAttached() ||
 			g_settings->getBool("disable_anticheat")) {
 		m_last_good_position = m_base_position;
 		return false;
 	}
-	if (UnitSAO *parent = dynamic_cast<UnitSAO *>(getParent())) {
-		v3f attachment_pos;
-		{
-			int parent_id;
-			std::string bone;
-			v3f attachment_rot;
-			bool force_visible;
-			getAttachment(&parent_id, &bone, &attachment_pos, &attachment_rot, &force_visible);
-		}
-
-		v3f parent_pos = parent->getBasePosition();
-		f32 diff = m_base_position.getDistanceFromSQ(parent_pos) - attachment_pos.getLengthSQ();
-		const f32 maxdiff = 4.0f * BS; // fair trade-off value for various latencies
-
-		if (diff > maxdiff * maxdiff) {
-			setBasePosition(parent_pos);
-			actionstream << "Server: " << m_player->getName()
-					<< " moved away from parent; diff=" << sqrtf(diff) / BS
-					<< " resetting position." << std::endl;
-			return true;
-		}
-		// Player movement is locked to the entity. Skip further checks
-		return false;
-	}
 
 	bool cheated = false;
 	/*