Fixed confusion over Item Frame directions

src/Entities/HangingEntity.cpp

@@ -11,7 +11,7 @@
 
 cHangingEntity::cHangingEntity(eEntityType a_EntityType, eBlockFace a_Facing, double a_X, double a_Y, double a_Z) :
 	cEntity(a_EntityType, a_X, a_Y, a_Z, 0.8, 0.8),
-	m_Facing(a_Facing)
+	m_Facing(cHangingEntity::BlockFaceToProtocolFace(a_Facing))
 {
 	SetMaxHealth(1);
 	SetHealth(1);
@@ -21,59 +21,10 @@ cHangingEntity::cHangingEntity(eEntityType a_EntityType, eBlockFace a_Facing, do
 
 
 
-void cHangingEntity::SetFacing(eBlockFace a_Facing)
-{
-	// Y-based faces are not allowed:
-	switch (a_Facing)
-	{
-		case BLOCK_FACE_NONE:
-		case BLOCK_FACE_YM:
-		case BLOCK_FACE_YP:
-		{
-			LOGWARNING("%s: Invalid facing: %d. Ignoring.", __FUNCTION__, a_Facing);
-			ASSERT(!"Tried to set a bad facing!");
-			return;
-		}
-		default: break;
-	}
-
-	m_Facing = a_Facing;
-}
-
-
-
-
-
 void cHangingEntity::SpawnOn(cClientHandle & a_ClientHandle)
 {
-	int Dir = 0;
+	SetYaw(GetProtocolFacing() * 90);
-
+	a_ClientHandle.SendSpawnObject(*this, 71, GetProtocolFacing(), (Byte)GetYaw(), (Byte)GetPitch());
-	// The client uses different values for item frame directions and block faces. Our constants are for the block faces, so we convert them here to item frame faces
-	switch (m_Facing)
-	{
-		case BLOCK_FACE_ZP: Dir = 0; break;
-		case BLOCK_FACE_ZM: Dir = 2; break;
-		case BLOCK_FACE_XM: Dir = 1; break;
-		case BLOCK_FACE_XP: Dir = 3; break;
-		default:
-		{
-			LOGINFO("Invalid facing (%d) in a cHangingEntity at {%d, %d, %d}, adjusting to BLOCK_FACE_XP.",
-				m_Facing, (int)GetPosX(), (int)GetPosY(), (int)GetPosZ()
-			);
-			Dir = 3;
-		}
-	}
-
-	if ((Dir == 0) || (Dir == 2))  // Probably a client bug, but two directions are flipped and contrary to the norm, so we do -180
-	{
-		SetYaw((Dir * 90) - 180);
-	}
-	else
-	{
-		SetYaw(Dir * 90);
-	}
-
-	a_ClientHandle.SendSpawnObject(*this, 71, Dir, (Byte)GetYaw(), (Byte)GetPitch());
 	a_ClientHandle.SendEntityMetadata(*this);
 }
 

src/Entities/HangingEntity.h

@@ -24,28 +24,77 @@ public:
 	// tolua_begin
 
 	/** Returns the direction in which the entity is facing. */
-	eBlockFace GetFacing() const { return m_Facing; }
+	eBlockFace GetFacing() const { return cHangingEntity::ProtocolFaceToBlockFace(m_Facing); }
-	
+
 	/** Set the direction in which the entity is facing. */
-	void SetFacing(eBlockFace a_Facing);
+	void SetFacing(eBlockFace a_Facing)	{ m_Facing = cHangingEntity::BlockFaceToProtocolFace(a_Facing); }
-	
-	/** Returns the X coord of the block in which the entity resides. */
-	int GetBlockX() const { return POSX_TOINT; }
-	
-	/** Returns the Y coord of the block in which the entity resides. */
-	int GetBlockY() const { return POSY_TOINT; }
-	
-	/** Returns the Z coord of the block in which the entity resides. */
-	int GetBlockZ() const { return POSZ_TOINT; }
 
 	// tolua_end
 
+	/** Returns the direction in which the entity is facing. */
+	Byte GetProtocolFacing() const { return m_Facing; }
+
+	/** Set the direction in which the entity is facing. */
+	void SetProtocolFacing(Byte a_Facing)
+	{
+		ASSERT((a_Facing <= 3) && (a_Facing >= 0));
+		m_Facing = a_Facing;
+	}
+
 private:
 
 	virtual void SpawnOn(cClientHandle & a_ClientHandle) override;
 	virtual void Tick(std::chrono::milliseconds a_Dt, cChunk & a_Chunk) override {}
 
-	eBlockFace m_Facing;
+	/** Converts protocol hanging item facing to eBlockFace values */
+	inline static eBlockFace ProtocolFaceToBlockFace(Byte a_ProtocolFace)
+	{
+		eBlockFace Dir;
+
+		// The client uses different values for item frame directions and block faces. Our constants are for the block faces, so we convert them here to item frame faces
+		switch (a_ProtocolFace)
+		{
+			case 0: Dir = BLOCK_FACE_ZP; break;
+			case 2: Dir = BLOCK_FACE_ZM; break;
+			case 1: Dir = BLOCK_FACE_XM; break;
+			case 3: Dir = BLOCK_FACE_XP; break;
+			default:
+			{
+				LOGINFO("Invalid facing (%d) in a cHangingEntity, adjusting to BLOCK_FACE_XP.", a_ProtocolFace);
+				ASSERT(!"Tried to convert a bad facing!");
+
+				Dir = cHangingEntity::ProtocolFaceToBlockFace(3);
+			}
+		}
+
+		return Dir;
+	}
+
+	/** Converts eBlockFace values to protocol hanging item faces */
+	inline static Byte BlockFaceToProtocolFace(eBlockFace a_BlockFace)
+	{
+		Byte Dir;
+
+		// The client uses different values for item frame directions and block faces. Our constants are for the block faces, so we convert them here to item frame faces
+		switch (a_BlockFace)
+		{
+			case BLOCK_FACE_ZP: Dir = 0; break;
+			case BLOCK_FACE_ZM: Dir = 2; break;
+			case BLOCK_FACE_XM: Dir = 1; break;
+			case BLOCK_FACE_XP: Dir = 3; break;
+			default:
+			{
+				LOGINFO("Invalid facing (%d) in a cHangingEntity, adjusting to BLOCK_FACE_XP.", a_BlockFace);
+				// Uncomment when entities are initialised with their real data, instead of dummy values: ASSERT(!"Tried to convert a bad facing!");
+
+				Dir = cHangingEntity::BlockFaceToProtocolFace(BLOCK_FACE_XP);
+			}
+		}
+
+		return Dir;
+	}
+
+	Byte m_Facing;
 
 };  // tolua_export
 

src/WorldStorage/NBTChunkSerializer.cpp

@@ -726,24 +726,10 @@ void cNBTChunkSerializer::AddProjectileEntity(cProjectileEntity * a_Projectile)
 
 void cNBTChunkSerializer::AddHangingEntity(cHangingEntity * a_Hanging)
 {
-	m_Writer.AddInt("TileX", a_Hanging->GetBlockX());
+	m_Writer.AddInt("TileX", FloorC(a_Hanging->GetPosX()));
-	m_Writer.AddInt("TileY", a_Hanging->GetBlockY());
+	m_Writer.AddInt("TileY", FloorC(a_Hanging->GetPosY()));
-	m_Writer.AddInt("TileZ", a_Hanging->GetBlockZ());
+	m_Writer.AddInt("TileZ", FloorC(a_Hanging->GetPosZ()));
-	switch (a_Hanging->GetFacing())
+	m_Writer.AddByte("Facing", a_Hanging->GetProtocolFacing());
-	{
-		case BLOCK_FACE_XM: m_Writer.AddByte("Facing", 1); break;
-		case BLOCK_FACE_XP: m_Writer.AddByte("Facing", 3); break;
-		case BLOCK_FACE_ZM: m_Writer.AddByte("Facing", 2); break;
-		case BLOCK_FACE_ZP: m_Writer.AddByte("Facing", 0); break;
-		
-		case BLOCK_FACE_YM:
-		case BLOCK_FACE_YP:
-		case BLOCK_FACE_NONE:
-		{
-			// These directions are invalid, but they may have been previously loaded, so keep them.
-			break;
-		}
-	}
 }
 
 

src/WorldStorage/WSSAnvil.cpp

@@ -1747,52 +1747,22 @@ void cWSSAnvil::LoadHangingFromNBT(cHangingEntity & a_Hanging, const cParsedNBT
 {
 	// "Facing" tag is the prime source of the Facing; if not available, translate from older "Direction" or "Dir"
 	int Facing = a_NBT.FindChildByName(a_TagIdx, "Facing");
-	if (Facing > 0)
+	if (Facing < 0)
 	{
-		Facing = (int)a_NBT.GetByte(Facing);
+		return;
-		if ((Facing >= 2) && (Facing <= 5))
-		{
-			a_Hanging.SetFacing(static_cast<eBlockFace>(Facing));
-		}
-	}
-	else
-	{
-		Facing = a_NBT.FindChildByName(a_TagIdx, "Direction");
-		if (Facing > 0)
-		{
-			switch ((int)a_NBT.GetByte(Facing))
-			{
-				case 0: a_Hanging.SetFacing(BLOCK_FACE_ZM); break;
-				case 1: a_Hanging.SetFacing(BLOCK_FACE_XM); break;
-				case 2: a_Hanging.SetFacing(BLOCK_FACE_ZP); break;
-				case 3: a_Hanging.SetFacing(BLOCK_FACE_XP); break;
-			}
-		}
-		else
-		{
-			Facing = a_NBT.FindChildByName(a_TagIdx, "Dir");  // Has values 0 and 2 swapped
-			if (Facing > 0)
-			{
-				switch ((int)a_NBT.GetByte(Facing))
-				{
-					case 0: a_Hanging.SetFacing(BLOCK_FACE_ZP); break;
-					case 1: a_Hanging.SetFacing(BLOCK_FACE_XM); break;
-					case 2: a_Hanging.SetFacing(BLOCK_FACE_ZM); break;
-					case 3: a_Hanging.SetFacing(BLOCK_FACE_XP); break;
-				}
-			}
-		}
 	}
 
+	a_Hanging.SetProtocolFacing(a_NBT.GetByte(Facing));
+
 	int TileX = a_NBT.FindChildByName(a_TagIdx, "TileX");
 	int TileY = a_NBT.FindChildByName(a_TagIdx, "TileY");
 	int TileZ = a_NBT.FindChildByName(a_TagIdx, "TileZ");
 	if ((TileX > 0) && (TileY > 0) && (TileZ > 0))
 	{
 		a_Hanging.SetPosition(
-			(double)a_NBT.GetInt(TileX),
+			static_cast<double>(a_NBT.GetInt(TileX)),
-			(double)a_NBT.GetInt(TileY),
+			static_cast<double>(a_NBT.GetInt(TileY)),
-			(double)a_NBT.GetInt(TileZ)
+			static_cast<double>(a_NBT.GetInt(TileZ))
 		);
 	}
 }