7 files changed, 120 insertions, 72 deletions

diff --git a/src/Entities/HangingEntity.cpp b/src/Entities/HangingEntity.cpp
index c20415e36..926c45fa1 100644
--- a/src/Entities/HangingEntity.cpp
+++ b/src/Entities/HangingEntity.cpp
@@ -2,7 +2,9 @@
 #include "Globals.h"  // NOTE: MSVC stupidness requires this to be the same across all modules
 
 #include "HangingEntity.h"
+#include "BlockInfo.h"
 #include "Player.h"
+#include "Chunk.h"
 #include "../ClientHandle.h"
 
 
@@ -21,6 +23,26 @@ cHangingEntity::cHangingEntity(eEntityType a_EntityType, eBlockFace a_Facing, Ve
 
 
 
+bool cHangingEntity::IsValidSupportBlock(const BLOCKTYPE a_BlockType)
+{
+	return cBlockInfo::IsSolid(a_BlockType) && (a_BlockType != E_BLOCK_REDSTONE_REPEATER_OFF) && (a_BlockType != E_BLOCK_REDSTONE_REPEATER_ON);
+}
+
+
+
+
+
+void cHangingEntity::KilledBy(TakeDamageInfo & a_TDI)
+{
+	Super::KilledBy(a_TDI);
+
+	Destroy();
+}
+
+
+
+
+
 void cHangingEntity::SpawnOn(cClientHandle & a_ClientHandle)
 {
 	SetYaw(GetProtocolFacing() * 90);
@@ -29,3 +51,24 @@ void cHangingEntity::SpawnOn(cClientHandle & a_ClientHandle)
 
 
 
+
+void cHangingEntity::Tick(std::chrono::milliseconds a_Dt, cChunk & a_Chunk)
+{
+	UNUSED(a_Dt);
+
+	// Check for a valid support block once every 64 ticks (3.2 seconds):
+	if ((m_World->GetWorldTickAge() % 64_tick) != 0_tick)
+	{
+		return;
+	}
+
+	BLOCKTYPE Block;
+	const auto SupportPosition = AddFaceDirection(cChunkDef::AbsoluteToRelative(GetPosition()), ProtocolFaceToBlockFace(m_Facing), true);
+	if (!a_Chunk.UnboundedRelGetBlockType(SupportPosition, Block) || IsValidSupportBlock(Block))
+	{
+		return;
+	}
+
+	// Take environmental damage, intending to self-destruct, with "take damage" handling done by child classes:
+	TakeDamage(dtEnvironment, nullptr, static_cast<int>(GetMaxHealth()), 0);
+}
diff --git a/src/Entities/HangingEntity.h b/src/Entities/HangingEntity.h
index 2a15582be..dfed17dc5 100644
--- a/src/Entities/HangingEntity.h
+++ b/src/Entities/HangingEntity.h
@@ -21,10 +21,14 @@ public:  // tolua_export
 
 	cHangingEntity(eEntityType a_EntityType, eBlockFace a_BlockFace, Vector3d a_Pos);
 
-	// tolua_begin
+	/** Returns the direction in which the entity is facing. */
+	eBlockFace GetFacing() const { return cHangingEntity::ProtocolFaceToBlockFace(m_Facing); }  // tolua_export
 
 	/** Returns the direction in which the entity is facing. */
-	eBlockFace GetFacing() const { return cHangingEntity::ProtocolFaceToBlockFace(m_Facing); }
+	Byte GetProtocolFacing() const { return m_Facing; }
+
+	/** Returns if the given block can support hanging entity placements. */
+	static bool IsValidSupportBlock(BLOCKTYPE a_BlockType);  // tolua_export
 
 	/** Set the direction in which the entity is facing. */
 	void SetFacing(eBlockFace a_Facing)
@@ -32,11 +36,6 @@ public:  // tolua_export
 		m_Facing = cHangingEntity::BlockFaceToProtocolFace(a_Facing);
 	}
 
-	// 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)
 	{
@@ -48,14 +47,9 @@ protected:
 
 	Byte m_Facing;
 
-
+	virtual void KilledBy(TakeDamageInfo & a_TDI) override;
 	virtual void SpawnOn(cClientHandle & a_ClientHandle) override;
-	virtual void Tick(std::chrono::milliseconds a_Dt, cChunk & a_Chunk) override
-	{
-		UNUSED(a_Dt);
-		UNUSED(a_Chunk);
-	}
-
+	virtual void Tick(std::chrono::milliseconds a_Dt, cChunk & a_Chunk) override;
 
 	/** Converts protocol hanging item facing to eBlockFace values */
 	inline static eBlockFace ProtocolFaceToBlockFace(Byte a_ProtocolFace)
@@ -102,7 +96,3 @@ protected:
 		UNREACHABLE("Unsupported block face");
 	}
 };  // tolua_export
-
-
-
-
diff --git a/src/Entities/ItemFrame.cpp b/src/Entities/ItemFrame.cpp
index eeab06737..90d3bb049 100644
--- a/src/Entities/ItemFrame.cpp
+++ b/src/Entities/ItemFrame.cpp
@@ -21,6 +21,54 @@ cItemFrame::cItemFrame(eBlockFace a_BlockFace, Vector3d a_Pos):
 
 
 
+bool cItemFrame::DoTakeDamage(TakeDamageInfo & a_TDI)
+{
+	// Take environmental or non-player damage normally:
+	if (m_Item.IsEmpty() || (a_TDI.Attacker == nullptr) || !a_TDI.Attacker->IsPlayer())
+	{
+		return Super::DoTakeDamage(a_TDI);
+	}
+
+	// Only pop out a pickup if attacked by a non-creative player:
+	if (!static_cast<cPlayer *>(a_TDI.Attacker)->IsGameModeCreative())
+	{
+		// Where the pickup spawns, offset by half cPickup height to centre in the block.
+		const auto SpawnPosition = GetPosition().addedY(-0.125);
+
+		// The direction the pickup travels to simulate a pop-out effect.
+		const auto FlyOutSpeed = AddFaceDirection(Vector3i(), ProtocolFaceToBlockFace(m_Facing)) * 2;
+
+		// Spawn the frame's held item:
+		GetWorld()->SpawnItemPickup(SpawnPosition, m_Item, FlyOutSpeed);
+	}
+
+	// In any case we have a held item and were hit by a player, so clear it:
+	m_Item.Empty();
+	m_ItemRotation = 0;
+	a_TDI.FinalDamage = 0;
+	SetInvulnerableTicks(0);
+	GetWorld()->BroadcastEntityMetadata(*this);
+	return false;
+}
+
+
+
+
+
+void cItemFrame::GetDrops(cItems & a_Items, cEntity * a_Killer)
+{
+	if (!m_Item.IsEmpty())
+	{
+		a_Items.push_back(m_Item);
+	}
+
+	a_Items.emplace_back(E_ITEM_ITEM_FRAME);
+}
+
+
+
+
+
 void cItemFrame::OnRightClicked(cPlayer & a_Player)
 {
 	Super::OnRightClicked(a_Player);
@@ -54,46 +102,6 @@ void cItemFrame::OnRightClicked(cPlayer & a_Player)
 
 
 
-void cItemFrame::KilledBy(TakeDamageInfo & a_TDI)
-{
-	if (m_Item.IsEmpty())
-	{
-		Super::KilledBy(a_TDI);
-		Destroy();
-		return;
-	}
-
-	if ((a_TDI.Attacker != nullptr) && a_TDI.Attacker->IsPlayer() && !static_cast<cPlayer *>(a_TDI.Attacker)->IsGameModeCreative())
-	{
-		cItems Item;
-		Item.push_back(m_Item);
-
-		GetWorld()->SpawnItemPickups(Item, GetPosX(), GetPosY(), GetPosZ());
-	}
-
-	SetHealth(GetMaxHealth());
-	m_Item.Empty();
-	m_ItemRotation = 0;
-	SetInvulnerableTicks(0);
-	GetWorld()->BroadcastEntityMetadata(*this);
-}
-
-
-
-
-
-void cItemFrame::GetDrops(cItems & a_Items, cEntity * a_Killer)
-{
-	if ((a_Killer != nullptr) && a_Killer->IsPlayer() && !static_cast<cPlayer *>(a_Killer)->IsGameModeCreative())
-	{
-		a_Items.emplace_back(E_ITEM_ITEM_FRAME);
-	}
-}
-
-
-
-
-
 void cItemFrame::SpawnOn(cClientHandle & a_ClientHandle)
 {
 	Super::SpawnOn(a_ClientHandle);
diff --git a/src/Entities/ItemFrame.h b/src/Entities/ItemFrame.h
index 82aece7b7..08363e5d8 100644
--- a/src/Entities/ItemFrame.h
+++ b/src/Entities/ItemFrame.h
@@ -39,16 +39,12 @@ public:  // tolua_export
 
 private:
 
-	virtual void OnRightClicked(cPlayer & a_Player) override;
-	virtual void KilledBy(TakeDamageInfo & a_TDI) override;
+	virtual bool DoTakeDamage(TakeDamageInfo & a_TDI) override;
 	virtual void GetDrops(cItems & a_Items, cEntity * a_Killer) override;
+	virtual void OnRightClicked(cPlayer & a_Player) override;
 	virtual void SpawnOn(cClientHandle & a_ClientHandle) override;
 
 	cItem m_Item;
 	Byte m_ItemRotation;
 
 };  // tolua_export
-
-
-
-
diff --git a/src/Entities/Painting.cpp b/src/Entities/Painting.cpp
index 1f9662019..dc1781fde 100644
--- a/src/Entities/Painting.cpp
+++ b/src/Entities/Painting.cpp
@@ -34,10 +34,7 @@ void cPainting::SpawnOn(cClientHandle & a_Client)
 
 void cPainting::GetDrops(cItems & a_Items, cEntity * a_Killer)
 {
-	if ((a_Killer != nullptr) && a_Killer->IsPlayer() && !static_cast<cPlayer *>(a_Killer)->IsGameModeCreative())
-	{
-		a_Items.emplace_back(E_ITEM_PAINTING);
-	}
+	a_Items.emplace_back(E_ITEM_PAINTING);
 }
 
 
@@ -47,6 +44,6 @@ void cPainting::GetDrops(cItems & a_Items, cEntity * a_Killer)
 void cPainting::KilledBy(TakeDamageInfo & a_TDI)
 {
 	Super::KilledBy(a_TDI);
+
 	m_World->BroadcastSoundEffect("entity.painting.break", GetPosition(), 1, 1);
-	Destroy();
 }
diff --git a/src/Items/ItemItemFrame.h b/src/Items/ItemItemFrame.h
index f1f687461..0f7a4ee8c 100644
--- a/src/Items/ItemItemFrame.h
+++ b/src/Items/ItemItemFrame.h
@@ -40,6 +40,12 @@ public:
 			return false;
 		}
 
+		// Make sure the support block is a valid block to place an item frame on:
+		if (!cHangingEntity::IsValidSupportBlock(a_World->GetBlock(a_ClickedBlockPos)))
+		{
+			return false;
+		}
+
 		// Make sure block that will be occupied by the item frame is free now:
 		const auto PlacePos = AddFaceDirection(a_ClickedBlockPos, a_ClickedBlockFace);
 		BLOCKTYPE Block = a_World->GetBlock(PlacePos);
@@ -48,8 +54,8 @@ public:
 			return false;
 		}
 
-		// Place the item frame:
-		auto ItemFrame = std::make_unique<cItemFrame>(a_ClickedBlockFace, PlacePos);
+		// An item frame, centred so pickups spawn nicely.
+		auto ItemFrame = std::make_unique<cItemFrame>(a_ClickedBlockFace, Vector3d(0.5, 0.5, 0.5) + PlacePos);
 		auto ItemFramePtr = ItemFrame.get();
 		if (!ItemFramePtr->Initialize(std::move(ItemFrame), *a_World))
 		{
diff --git a/src/Items/ItemPainting.h b/src/Items/ItemPainting.h
index 78d9f040a..058f8d3b2 100644
--- a/src/Items/ItemPainting.h
+++ b/src/Items/ItemPainting.h
@@ -45,6 +45,12 @@ public:
 			return false;
 		}
 
+		// Make sure the support block is a valid block to place a painting on:
+		if (!cHangingEntity::IsValidSupportBlock(a_World->GetBlock(a_ClickedBlockPos)))
+		{
+			return false;
+		}
+
 		// Make sure block that will be occupied is free:
 		auto PlacePos = AddFaceDirection(a_ClickedBlockPos, a_ClickedBlockFace);
 		BLOCKTYPE PlaceBlockType = a_World->GetBlock(PlacePos);
@@ -85,7 +91,9 @@ public:
 		};
 
 		auto PaintingTitle = gPaintingTitlesList[a_World->GetTickRandomNumber(ARRAYCOUNT(gPaintingTitlesList) - 1)];
-		auto Painting = std::make_unique<cPainting>(PaintingTitle, a_ClickedBlockFace, PlacePos);
+
+		// A painting, centred so pickups spawn nicely.
+		auto Painting = std::make_unique<cPainting>(PaintingTitle, a_ClickedBlockFace, Vector3d(0.5, 0.5, 0.5) + PlacePos);
 		auto PaintingPtr = Painting.get();
 		if (!PaintingPtr->Initialize(std::move(Painting), *a_World))
 		{