Chest, weather, crash, and miscellaneous fixes (#5215)

* Alpha-sort cChestEntity

* Chests: use SendUpdateBlockEntity

* Pathfinder: fix out of range Y

* 1.13: correct weather packet ID

* Chests: fix neighbour scanner

+ Add OnAddToWorld and overload to scan neighbours there, instead of in the constructor/OnUse. This fixes hoppers accessing newly loaded double chests and seeing a null m_Neighbour, thus thinking its a single chest.
* Fix typo in cross coords computation.
* Simplify hopper logic.

* Block entities: ASSERT that type is correct

If you match the block type first before calling DoWithBlockEntity, the corresponding block entity must either be empty or correspond to the block type.

* Chunk: fix some forgotten PendingSendBE cleanup

+ Add cleanup in SetAllData, WriteBlockArea
- Remove RemoveBlockEntity (used once), HasBlockEntity (not used)

* Replace MakeIndex with MakeIndexNoCheck

* Remove extraneous MarkDirty in hopper & chests

src/Bindings/LuaWindow.cpp

@@ -196,11 +196,7 @@ void cLuaWindow::DistributeStack(cItem & a_ItemStack, int a_Slot, cPlayer & a_Pl
 
 void cLuaWindow::OnSlotChanged(cItemGrid * a_ItemGrid, int a_SlotNum)
 {
-	if (a_ItemGrid != &m_Contents)
+	ASSERT(a_ItemGrid == &m_Contents);
-	{
-		ASSERT(!"Invalid ItemGrid in callback");
-		return;
-	}
 
 	// If an OnSlotChanged callback has been registered, call it:
 	if (m_OnSlotChanged != nullptr)

src/BlockEntities/BlockEntity.cpp

@@ -168,6 +168,15 @@ bool cBlockEntity::IsBlockEntityBlockType(const BLOCKTYPE a_BlockType)
 
 
 
+void cBlockEntity::OnAddToWorld(cWorld & a_World, cChunk & a_Chunk)
+{
+	m_World = &a_World;
+}
+
+
+
+
+
 void cBlockEntity::OnRemoveFromWorld()
 {
 }

src/BlockEntities/BlockEntity.h

@@ -51,12 +51,19 @@ public:
 	Returns nullptr for unknown block types. */
 	static OwnedBlockEntity CreateByBlockType(BLOCKTYPE a_BlockType, NIBBLETYPE a_BlockMeta, Vector3i a_Pos, cWorld * a_World = nullptr);
 
+	/** Called when this block entity's associated block is destroyed.
+	It is guaranteed that this function is called before OnRemoveFromWorld. */
 	virtual void Destroy();
 
 	/** Returns true if the specified blocktype is supposed to have an associated block entity. */
 	static bool IsBlockEntityBlockType(BLOCKTYPE a_BlockType);
 
-	/** Called when the block entity is removed from a world. */
+	/** Called when the block entity object is added to a world. */
+	virtual void OnAddToWorld(cWorld & a_World, cChunk & a_Chunk);
+
+	/** Called when the block entity object is removed from a world.
+	This occurs when the chunk it resides in is unloaded, or when the associated block is destroyed.
+	If it is the latter, Destroy() is guaranteed to be called first. */
 	virtual void OnRemoveFromWorld();
 
 	/** Sends the packet defining the block entity to the client specified.

src/BlockEntities/ChestEntity.cpp

@@ -2,6 +2,7 @@
 #include "Globals.h"  // NOTE: MSVC stupidness requires this to be the same across all modules
 
 #include "ChestEntity.h"
+#include "../Chunk.h"
 #include "../BlockInfo.h"
 #include "../Item.h"
 #include "../Entities/Player.h"
@@ -18,125 +19,59 @@ cChestEntity::cChestEntity(BLOCKTYPE a_BlockType, NIBBLETYPE a_BlockMeta, Vector
 	m_NumActivePlayers(0),
 	m_Neighbour(nullptr)
 {
-	auto chunkCoord = cChunkDef::BlockToChunk(a_Pos);
-	if (
-		(m_World != nullptr) &&
-		m_World->IsChunkValid(chunkCoord.m_ChunkX, chunkCoord.m_ChunkZ)
-	)
-	{
-		ScanNeighbours();
-	}
 }
 
 
 
 
 
-void cChestEntity::CopyFrom(const cBlockEntity & a_Src)
+cChestEntity & cChestEntity::GetPrimaryChest()
 {
-	Super::CopyFrom(a_Src);
-	auto & src = static_cast<const cChestEntity &>(a_Src);
-	m_Contents.CopyFrom(src.m_Contents);
-
-	// Reset the neighbor and player count, there's no sense in copying these:
-	m_Neighbour = nullptr;
-	m_NumActivePlayers = 0;
-}
-
-
-
-
-
-void cChestEntity::OnRemoveFromWorld()
-{
-	if (m_Neighbour != nullptr)
-	{
-		// Neighbour may share a window with us, force the window shut:
-		m_Neighbour->DestroyWindow();
-		m_Neighbour->m_Neighbour = nullptr;
-	}
-
-	DestroyWindow();
-}
-
-
-
-
-
-void cChestEntity::SendTo(cClientHandle & a_Client)
-{
-	// Send a dummy "number of players with chest open" packet to make the chest visible:
-	a_Client.SendBlockAction(m_Pos.x, m_Pos.y, m_Pos.z, 1, 0, m_BlockType);
-}
-
-
-
-
-
-bool cChestEntity::UsedBy(cPlayer * a_Player)
-{
-	if (IsBlocked())
-	{
-		// Obstruction, don't open
-		return true;
-	}
-
 	if (m_Neighbour == nullptr)
 	{
-		ScanNeighbours();
+		return *this;
 	}
 
 	// The primary chest should be the one with lesser X or Z coord:
-	cChestEntity * PrimaryChest = this;
+	return (
-	if (m_Neighbour != nullptr)
+		(m_Neighbour->GetPosX() < GetPosX()) ||
-	{
+		(m_Neighbour->GetPosZ() < GetPosZ())
-		if (m_Neighbour->IsBlocked())
+	) ? *m_Neighbour : *this;
-		{
+}
-			// Obstruction, don't open
-			return true;
-		}
 
-		if (
+
-			(m_Neighbour->GetPosX() > GetPosX()) ||
+
-			(m_Neighbour->GetPosZ() > GetPosZ())
+
-		)
+
-		{
+cChestEntity * cChestEntity::GetSecondaryChest()
-			PrimaryChest = m_Neighbour;
+{
-		}
+	// If we're the primary, then our neighbour is the secondary, and vice versa:
+	return (&GetPrimaryChest() == this) ? m_Neighbour : this;
+}
+
+
+
+
+
+bool cChestEntity::ScanNeighbour(cChunk & a_Chunk, Vector3i a_Position)
+{
+	const auto Chunk = a_Chunk.GetRelNeighborChunkAdjustCoords(a_Position);
+
+	if ((Chunk == nullptr) || !Chunk->IsValid())
+	{
+		// If a chest was in fact there, they'll find us when their chunk loads.
+		return false;
 	}
 
-	if (m_BlockType == E_BLOCK_CHEST)
+	const auto BlockEntity = Chunk->GetBlockEntityRel(a_Position);
+
+	if ((BlockEntity == nullptr) || (BlockEntity->GetBlockType() != m_BlockType))
 	{
-		a_Player->GetStatManager().AddValue(Statistic::OpenChest);
+		// Neighbouring block is not the same type of chest:
-	}
+		return false;
-	else  // E_BLOCK_TRAPPED_CHEST
-	{
-		a_Player->GetStatManager().AddValue(Statistic::TriggerTrappedChest);
 	}
 
-	// If the window is not created, open it anew:
+	m_Neighbour = static_cast<cChestEntity *>(BlockEntity);
-	cWindow * Window = PrimaryChest->GetWindow();
-	if (Window == nullptr)
-	{
-		PrimaryChest->OpenNewWindow();
-		Window = PrimaryChest->GetWindow();
-	}
-
-	// Open the window for the player:
-	if (Window != nullptr)
-	{
-		if (a_Player->GetWindow() != Window)
-		{
-			a_Player->OpenWindow(*Window);
-		}
-	}
-
-	// This is rather a hack
-	// Instead of marking the chunk as dirty upon chest contents change, we mark it dirty now
-	// We cannot properly detect contents change, but such a change doesn't happen without a player opening the chest first.
-	// The few false positives aren't much to worry about
-	auto chunkCoords = cChunkDef::BlockToChunk(m_Pos);
-	m_World->MarkChunkDirty(chunkCoords.m_ChunkX, chunkCoords.m_ChunkZ);
 	return true;
 }
 
@@ -144,69 +79,6 @@ bool cChestEntity::UsedBy(cPlayer * a_Player)
 
 
 
-cChestEntity * cChestEntity::GetNeighbour()
-{
-	return m_Neighbour;
-}
-
-
-
-
-
-void cChestEntity::ScanNeighbours()
-{
-	// Callback for finding neighbouring chest.
-	auto FindNeighbour = [this](cBlockEntity & a_BlockEntity)
-	{
-		if (a_BlockEntity.GetBlockType() != m_BlockType)
-		{
-			// Neighboring block is not the same type of chest
-			return false;
-		}
-
-		m_Neighbour = static_cast<cChestEntity *>(&a_BlockEntity);
-		return true;
-	};
-
-	// Scan horizontally adjacent blocks for any neighbouring chest of the same type:
-	if (
-		m_World->DoWithBlockEntityAt(m_Pos.addedX(-1), FindNeighbour) ||
-		m_World->DoWithBlockEntityAt(m_Pos.addedX(+1), FindNeighbour) ||
-		m_World->DoWithBlockEntityAt(m_Pos.addedZ(-1), FindNeighbour) ||
-		m_World->DoWithBlockEntityAt(m_Pos.addedX(+1), FindNeighbour)
-	)
-	{
-		m_Neighbour->m_Neighbour = this;
-		// Force neighbour's window shut. Does Mojang server do this or should a double window open?
-		m_Neighbour->DestroyWindow();
-	}
-}
-
-
-
-
-
-void cChestEntity::OpenNewWindow(void)
-{
-	if (m_Neighbour != nullptr)
-	{
-		ASSERT(  // This should be the primary chest
-			(m_Neighbour->GetPosX() < GetPosX()) ||
-			(m_Neighbour->GetPosZ() < GetPosZ())
-		);
-		OpenWindow(new cChestWindow(this, m_Neighbour));
-	}
-	else
-	{
-		// There is no chest neighbour, open a single-chest window:
-		OpenWindow(new cChestWindow(this));
-	}
-}
-
-
-
-
-
 void cChestEntity::DestroyWindow()
 {
 	const auto Window = GetWindow();
@@ -235,15 +107,140 @@ bool cChestEntity::IsBlocked()
 
 
 
+void cChestEntity::OpenNewWindow(void)
+{
+	if (m_Neighbour != nullptr)
+	{
+		ASSERT(&GetPrimaryChest() == this);  // Should only open windows for the primary chest.
+
+		OpenWindow(new cChestWindow(this, m_Neighbour));
+	}
+	else
+	{
+		// There is no chest neighbour, open a single-chest window:
+		OpenWindow(new cChestWindow(this));
+	}
+}
+
+
+
+
+
+void cChestEntity::CopyFrom(const cBlockEntity & a_Src)
+{
+	Super::CopyFrom(a_Src);
+	auto & src = static_cast<const cChestEntity &>(a_Src);
+	m_Contents.CopyFrom(src.m_Contents);
+
+	// Reset the neighbor and player count, there's no sense in copying these:
+	m_Neighbour = nullptr;
+	m_NumActivePlayers = 0;
+}
+
+
+
+
+
+void cChestEntity::OnAddToWorld(cWorld & a_World, cChunk & a_Chunk)
+{
+	Super::OnAddToWorld(a_World, a_Chunk);
+
+	// Scan horizontally adjacent blocks for any neighbouring chest of the same type:
+	if (
+		const auto Position = GetRelPos();
+
+		ScanNeighbour(a_Chunk, Position.addedX(-1)) ||
+		ScanNeighbour(a_Chunk, Position.addedX(+1)) ||
+		ScanNeighbour(a_Chunk, Position.addedZ(-1)) ||
+		ScanNeighbour(a_Chunk, Position.addedZ(+1))
+	)
+	{
+		m_Neighbour->m_Neighbour = this;
+		m_Neighbour->DestroyWindow();  // Force neighbour's window shut. Does Mojang server do this or should a double window open?
+	}
+}
+
+
+
+
+
+void cChestEntity::OnRemoveFromWorld()
+{
+	if (m_Neighbour != nullptr)
+	{
+		// Neighbour may share a window with us, force the window shut:
+		m_Neighbour->DestroyWindow();
+		m_Neighbour->m_Neighbour = nullptr;
+	}
+
+	DestroyWindow();
+}
+
+
+
+
+
+void cChestEntity::SendTo(cClientHandle & a_Client)
+{
+	a_Client.SendUpdateBlockEntity(*this);
+}
+
+
+
+
+
+bool cChestEntity::UsedBy(cPlayer * a_Player)
+{
+	if (IsBlocked())
+	{
+		// Obstruction, don't open
+		return true;
+	}
+
+	if ((m_Neighbour != nullptr) && m_Neighbour->IsBlocked())
+	{
+		return true;
+	}
+
+	if (m_BlockType == E_BLOCK_CHEST)
+	{
+		a_Player->GetStatManager().AddValue(Statistic::OpenChest);
+	}
+	else  // E_BLOCK_TRAPPED_CHEST
+	{
+		a_Player->GetStatManager().AddValue(Statistic::TriggerTrappedChest);
+	}
+
+	auto & PrimaryChest = GetPrimaryChest();
+	cWindow * Window = PrimaryChest.GetWindow();
+
+	// If the window is not created, open it anew:
+	if (Window == nullptr)
+	{
+		PrimaryChest.OpenNewWindow();
+		Window = PrimaryChest.GetWindow();
+	}
+
+	// Open the window for the player:
+	if (Window != nullptr)
+	{
+		if (a_Player->GetWindow() != Window)
+		{
+			a_Player->OpenWindow(*Window);
+		}
+	}
+
+	return true;
+}
+
+
+
+
+
 void cChestEntity::OnSlotChanged(cItemGrid * a_Grid, int a_SlotNum)
 {
 	ASSERT(a_Grid == &m_Contents);
 
-	if (m_World == nullptr)
-	{
-		return;
-	}
-
 	// Have cBlockEntityWithItems update redstone and try to broadcast our window:
 	Super::OnSlotChanged(a_Grid, a_SlotNum);
 
@@ -259,9 +256,4 @@ void cChestEntity::OnSlotChanged(cItemGrid * a_Grid, int a_SlotNum)
 	{
 		Window->BroadcastWholeWindow();
 	}
-
-	m_World->MarkChunkDirty(GetChunkX(), GetChunkZ());
-
-	// Notify comparators:
-	m_World->WakeUpSimulators(m_Pos);
 }

src/BlockEntities/ChestEntity.h

@@ -34,33 +34,21 @@ public:
 
 	// tolua_end
 
-	/** Constructor used for normal operation */
 	cChestEntity(BLOCKTYPE a_BlockType, NIBBLETYPE a_BlockMeta, Vector3i a_Pos, cWorld * a_World);
 
-	// cBlockEntity overrides:
-	virtual void CopyFrom(const cBlockEntity & a_Src) override;
-	virtual void OnRemoveFromWorld() override;
-	virtual void SendTo(cClientHandle & a_Client) override;
-	virtual bool UsedBy(cPlayer * a_Player) override;
-
-	/** Search horizontally adjacent blocks for neighbouring chests of the same type and links them together. */
-	void ScanNeighbours();
-
-	/** Returns the value of m_Neighbour. */
-	cChestEntity * GetNeighbour();
-
-	/** Opens a new chest window where this is the primary chest and any neighbour is the secondary. */
-	void OpenNewWindow();
-
-	/** Forces any players to close the owned window. */
-	void DestroyWindow();
-
-	/** Returns true if the chest should not be accessible by players. */
-	bool IsBlocked();
-
 	/** Gets the number of players who currently have this chest open */
 	int GetNumberOfPlayers(void) const { return m_NumActivePlayers; }
 
+	/** Returns the associated primary chest. */
+	cChestEntity & GetPrimaryChest();
+
+	/** Returns the associated secondary chest, if any. */
+	cChestEntity * GetSecondaryChest();
+
+	/** Search the given horizontally adjacent relative position for a neighbouring chest of the same type.
+	If found, links them together with ourselves and returns true. */
+	bool ScanNeighbour(cChunk & a_Chunk, Vector3i a_Position);
+
 	/** Sets the number of players who currently have this chest open */
 	void SetNumberOfPlayers(int a_NumActivePlayers) { m_NumActivePlayers = a_NumActivePlayers; }
 
@@ -72,6 +60,22 @@ private:
 	/** Neighbouring chest that links to form a double chest */
 	cChestEntity * m_Neighbour;
 
+	/** Forces any players to close the owned window. */
+	void DestroyWindow();
+
+	/** Returns true if the chest should not be accessible by players. */
+	bool IsBlocked();
+
+	/** Opens a new chest window where this is the primary chest and any neighbour is the secondary. */
+	void OpenNewWindow();
+
+	// cBlockEntity overrides:
+	virtual void CopyFrom(const cBlockEntity & a_Src) override;
+	virtual void OnAddToWorld(cWorld & a_World, cChunk & a_Chunk) override;
+	virtual void OnRemoveFromWorld() override;
+	virtual void SendTo(cClientHandle & a_Client) override;
+	virtual bool UsedBy(cPlayer * a_Player) override;
+
 	/** cItemGrid::cListener overrides: */
 	virtual void OnSlotChanged(cItemGrid * a_Grid, int a_SlotNum) override;
 } ;  // tolua_export

src/BlockEntities/HopperEntity.cpp

@@ -132,12 +132,6 @@ bool cHopperEntity::UsedBy(cPlayer * a_Player)
 		}
 	}
 
-	// This is rather a hack
-	// Instead of marking the chunk as dirty upon chest contents change, we mark it dirty now
-	// We cannot properly detect contents change, but such a change doesn't happen without a player opening the chest first.
-	// The few false positives aren't much to worry about
-	cChunkCoords ChunkPos = cChunkDef::BlockToChunk(GetPos());
-	m_World->MarkChunkDirty(ChunkPos.m_ChunkX, ChunkPos.m_ChunkZ);
 	return true;
 }
 
@@ -172,15 +166,15 @@ bool cHopperEntity::MoveItemsIn(cChunk & a_Chunk, const cTickTimeLong a_CurrentT
 	bool res = false;
 	switch (a_Chunk.GetBlock(GetRelPos().addedY(1)))
 	{
-		case E_BLOCK_TRAPPED_CHEST:
 		case E_BLOCK_CHEST:
+		case E_BLOCK_TRAPPED_CHEST:
 		{
 			// Chests have special handling because of double-chests
 			res = MoveItemsFromChest(a_Chunk);
 			break;
 		}
-		case E_BLOCK_LIT_FURNACE:
 		case E_BLOCK_FURNACE:
+		case E_BLOCK_LIT_FURNACE:
 		{
 			// Furnaces have special handling because only the output and leftover fuel buckets shall be moved
 			res = MoveItemsFromFurnace(a_Chunk);
@@ -331,15 +325,15 @@ bool cHopperEntity::MoveItemsOut(cChunk & a_Chunk, const cTickTimeLong a_Current
 	auto absCoord = destChunk->RelativeToAbsolute(relCoord);
 	switch (destChunk->GetBlock(relCoord))
 	{
-		case E_BLOCK_TRAPPED_CHEST:
 		case E_BLOCK_CHEST:
+		case E_BLOCK_TRAPPED_CHEST:
 		{
 			// Chests have special handling because of double-chests
 			res = MoveItemsToChest(*destChunk, absCoord);
 			break;
 		}
-		case E_BLOCK_LIT_FURNACE:
 		case E_BLOCK_FURNACE:
+		case E_BLOCK_LIT_FURNACE:
 		{
 			// Furnaces have special handling because of the direction-to-slot relation
 			res = MoveItemsToFurnace(*destChunk, absCoord, meta);
@@ -375,52 +369,22 @@ bool cHopperEntity::MoveItemsOut(cChunk & a_Chunk, const cTickTimeLong a_Current
 
 bool cHopperEntity::MoveItemsFromChest(cChunk & a_Chunk)
 {
-	auto ChestPos = GetPos().addedY(1);
+	const auto ConnectedBlockEntity = a_Chunk.GetBlockEntityRel(GetRelPos().addedY(1));
-	auto MainChest = static_cast<cChestEntity *>(a_Chunk.GetBlockEntity(ChestPos));
+
-	if (MainChest == nullptr)
+	if (ConnectedBlockEntity == nullptr)
 	{
-		FLOGWARNING("{0}: A chest entity was not found where expected, at {1}", __FUNCTION__, ChestPos);
 		return false;
 	}
-	auto SideChest = MainChest->GetNeighbour();
+
-	if (SideChest == nullptr)
+	const auto ConnectedChest = static_cast<cChestEntity *>(ConnectedBlockEntity);
+
+	if (MoveItemsFromGrid(ConnectedChest->GetPrimaryChest()))
 	{
-		if (MoveItemsFromGrid(*MainChest))
+		return true;
-		{
-			return true;
-		}
 	}
-	else
+
-	{
+	const auto SecondaryChest = ConnectedChest->GetSecondaryChest();
-		auto SideAbsCoords = SideChest->GetPos();
+	return (SecondaryChest != nullptr) && MoveItemsFromGrid(*SecondaryChest);
-		// the "primary" chest is the one with the higher z or x value
-		if (SideAbsCoords.z > ChestPos.z || SideAbsCoords.x > ChestPos.x)
-		{
-			// side is "primary" so pull from it first
-			if (MoveItemsFromGrid(*SideChest))
-			{
-				return true;
-			}
-			// main is secondary, pull from next
-			if (MoveItemsFromGrid(*MainChest))
-			{
-				return true;
-			}
-		}
-		else
-		{
-			if (MoveItemsFromGrid(*MainChest))
-			{
-				return true;
-			}
-			if (MoveItemsFromGrid(*SideChest))
-			{
-				return true;
-			}
-		}
-	}
-	// The chest was empty
-	return false;
 }
 
 
@@ -529,48 +493,22 @@ bool cHopperEntity::MoveItemsFromSlot(cBlockEntityWithItems & a_Entity, int a_Sl
 
 bool cHopperEntity::MoveItemsToChest(cChunk & a_Chunk, Vector3i a_Coords)
 {
-	// Try the chest directly connected to the hopper:
+	const auto ConnectedBlockEntity = a_Chunk.GetBlockEntity(a_Coords);
-	auto ConnectedChest = static_cast<cChestEntity *>(a_Chunk.GetBlockEntity(a_Coords));
+
-	if (ConnectedChest == nullptr)
+	if (ConnectedBlockEntity == nullptr)
 	{
-		FLOGWARNING("{0}: A chest entity was not found where expected, at {1}", __FUNCTION__, a_Coords);
 		return false;
 	}
-	auto SideChest = ConnectedChest->GetNeighbour();
+
-	if (SideChest == nullptr)
+	const auto ConnectedChest = static_cast<cChestEntity *>(ConnectedBlockEntity);
+
+	if (MoveItemsToGrid(ConnectedChest->GetPrimaryChest()))
 	{
-		if (MoveItemsToGrid(*ConnectedChest))
+		return true;
-		{
-			return true;
-		}
 	}
-	else
+
-	{
+	const auto SecondaryChest = ConnectedChest->GetSecondaryChest();
-		auto SideAbsCoords = SideChest->GetPos();
+	return (SecondaryChest != nullptr) && MoveItemsToGrid(*SecondaryChest);
-		if (SideAbsCoords.z > a_Coords.z || SideAbsCoords.x > a_Coords.x)
-		{
-			if (MoveItemsToGrid(*SideChest))
-			{
-				return true;
-			}
-			if (MoveItemsToGrid(*ConnectedChest))
-			{
-				return true;
-			}
-		}
-		else
-		{
-			if (MoveItemsToGrid(*ConnectedChest))
-			{
-				return true;
-			}
-			if (MoveItemsToGrid(*SideChest))
-			{
-				return true;
-			}
-		}
-	}
-	return false;
 }
 
 
@@ -661,7 +599,3 @@ bool cHopperEntity::MoveItemsToSlot(cBlockEntityWithItems & a_Entity, int a_DstS
 		return false;
 	}
 }
-
-
-
-

src/Blocks/BlockBed.cpp

@@ -163,6 +163,7 @@ void cBlockBedHandler::OnPlacedByPlayer(cChunkInterface & a_ChunkInterface, cWor
 	a_Player.GetWorld()->DoWithBlockEntityAt(a_BlockChange.GetAbsolutePos(), [&a_Player](cBlockEntity & a_BlockEntity)
 	{
 		ASSERT(a_BlockEntity.GetBlockType() == E_BLOCK_BED);
+
 		static_cast<cBedEntity &>(a_BlockEntity).SetColor(a_Player.GetEquippedItem().m_ItemDamage);
 		return false;
 	});

src/Blocks/BlockEnchantingTable.h

@@ -34,10 +34,7 @@ private:
 		AString WindowName = "Enchant";
 		a_WorldInterface.DoWithBlockEntityAt(a_BlockPos, [&WindowName](cBlockEntity & a_Entity)
 		{
-			if (a_Entity.GetBlockType() != E_BLOCK_ENCHANTMENT_TABLE)
+			ASSERT(a_Entity.GetBlockType() == E_BLOCK_ENCHANTMENT_TABLE);
-			{
-				return false;
-			}
 
 			const auto & EnchantingTable = static_cast<cEnchantingTableEntity &>(a_Entity);
 			const auto & CustomName = EnchantingTable.GetCustomName();

src/Blocks/BlockNoteBlock.h

@@ -28,10 +28,7 @@ private:
 	{
 		a_WorldInterface.DoWithBlockEntityAt(a_BlockPos, [](cBlockEntity & a_BlockEntity)
 		{
-			if (a_BlockEntity.GetBlockType() != E_BLOCK_NOTE_BLOCK)
+			ASSERT(a_BlockEntity.GetBlockType() == E_BLOCK_NOTE_BLOCK);
-			{
-				return false;
-			}
 
 			static_cast<cNoteEntity &>(a_BlockEntity).MakeSound();
 			return false;

src/Chunk.cpp

@@ -336,13 +336,16 @@ void cChunk::GetAllData(cChunkDataCallback & a_Callback) const
 
 void cChunk::SetAllData(SetChunkData && a_SetChunkData)
 {
-	std::copy(a_SetChunkData.HeightMap, a_SetChunkData.HeightMap + std::size(a_SetChunkData.HeightMap), m_HeightMap);
+	std::copy_n(a_SetChunkData.HeightMap, std::size(a_SetChunkData.HeightMap), m_HeightMap);
-	std::copy(a_SetChunkData.BiomeMap, a_SetChunkData.BiomeMap + std::size(a_SetChunkData.BiomeMap), m_BiomeMap);
+	std::copy_n(a_SetChunkData.BiomeMap, std::size(a_SetChunkData.BiomeMap), m_BiomeMap);
 
 	m_BlockData = std::move(a_SetChunkData.BlockData);
 	m_LightData = std::move(a_SetChunkData.LightData);
 	m_IsLightValid = a_SetChunkData.IsLightValid;
 
+	m_PendingSendBlocks.clear();
+	m_PendingSendBlockEntities.clear();
+
 	// Entities need some extra steps to destroy, so here we're keeping the old ones.
 	// Move the entities already in the chunk, including player entities, so that we don't lose any:
 	a_SetChunkData.Entities.insert(
@@ -383,15 +386,17 @@ void cChunk::SetAllData(SetChunkData && a_SetChunkData)
 	}
 #endif
 
-	// Set all block entities' World variable:
+	// Set the chunk data as valid.
+	// This may be needed for some simulators that perform actions upon block adding (Vaporize),
+	// as well as some block entities upon being added to the chunk (Chests).
+	SetPresence(cpPresent);
+
+	// Initialise all block entities:
 	for (auto & KeyPair : m_BlockEntities)
 	{
-		KeyPair.second->SetWorld(m_World);
+		KeyPair.second->OnAddToWorld(*m_World, *this);
 	}
 
-	// Set the chunk data as valid. This may be needed for some simulators that perform actions upon block adding (Vaporize)
-	SetPresence(cpPresent);
-
 	// Wake up all simulators for their respective blocks:
 	WakeUpSimulators();
 }
@@ -466,22 +471,34 @@ void cChunk::WriteBlockArea(cBlockArea & a_Area, int a_MinBlockX, int a_MinBlock
 	}  // for y
 
 	// Erase all affected block entities:
-	cCuboid affectedArea(  // In world coordinates
-		{BlockStartX, a_MinBlockY, BlockStartZ},
-		{BlockEndX, a_MinBlockY + SizeY - 1, BlockEndZ}
-	);
-	for (auto itr = m_BlockEntities.begin(); itr != m_BlockEntities.end();)
 	{
-		if (affectedArea.IsInside(itr->second->GetPos()))
+		// The affected area, in world coordinates.
+		cCuboid affectedArea(
+			{ BlockStartX, a_MinBlockY, BlockStartZ },
+			{ BlockEndX, a_MinBlockY + SizeY - 1, BlockEndZ }
+		);
+
+		// Where in the pending block entity send list to start removing the invalidated elements from.
+		auto PendingRemove = m_PendingSendBlockEntities.end();
+
+		for (auto itr = m_BlockEntities.begin(); itr != m_BlockEntities.end();)
 		{
-			itr->second->Destroy();
+			if (affectedArea.IsInside(itr->second->GetPos()))
-			itr->second->OnRemoveFromWorld();
+			{
-			itr = m_BlockEntities.erase(itr);
+				itr->second->Destroy();
-		}
+				itr->second->OnRemoveFromWorld();
-		else
+
-		{
+				PendingRemove = std::remove(m_PendingSendBlockEntities.begin(), PendingRemove, itr->second.get());  // Search the remaining valid pending sends.
-			++itr;
+				itr = m_BlockEntities.erase(itr);
+			}
+			else
+			{
+				++itr;
+			}
 		}
+
+		// Remove all the deleted block entities from the pending send list:
+		m_PendingSendBlockEntities.erase(PendingRemove, m_PendingSendBlockEntities.end());
 	}
 
 	// Clone block entities from a_Area into this chunk:
@@ -500,16 +517,13 @@ void cChunk::WriteBlockArea(cBlockArea & a_Area, int a_MinBlockX, int a_MinBlock
 			{
 				continue;
 			}
-			// This block entity is inside the chunk, clone it (and remove any that is there currently):
+
-			auto idx = static_cast<size_t>(cChunkDef::MakeIndex(posX - m_PosX * cChunkDef::Width, posY, posZ - m_PosZ * cChunkDef::Width));
+			// This block entity is inside the chunk.
-			auto itr = m_BlockEntities.find(idx);
+			// The code above should have removed any that were here before:
-			if (itr != m_BlockEntities.end())
+			ASSERT(GetBlockEntityRel(cChunkDef::AbsoluteToRelative({ posX, posY, posZ })) == nullptr);
-			{
+
-				m_BlockEntities.erase(itr);
+			// Clone, and add the new one:
-			}
+			AddBlockEntity(be->Clone({posX, posY, posZ}));
-			auto clone = be->Clone({posX, posY, posZ});
-			clone->SetWorld(m_World);
-			AddBlockEntity(std::move(clone));
 		}
 	}
 }
@@ -518,15 +532,6 @@ void cChunk::WriteBlockArea(cBlockArea & a_Area, int a_MinBlockX, int a_MinBlock
 
 
 
-bool cChunk::HasBlockEntityAt(Vector3i a_BlockPos)
-{
-	return (GetBlockEntity(a_BlockPos) != nullptr);
-}
-
-
-
-
-
 void cChunk::Stay(bool a_Stay)
 {
 	if (a_Stay)
@@ -1287,12 +1292,15 @@ void cChunk::SetBlock(Vector3i a_RelPos, BLOCKTYPE a_BlockType, NIBBLETYPE a_Blo
 	GetWorld()->GetSimulatorManager()->WakeUp(*this, a_RelPos);
 
 	// If there was a block entity, remove it:
-	cBlockEntity * BlockEntity = GetBlockEntityRel(a_RelPos);
+	if (const auto FindResult = m_BlockEntities.find(cChunkDef::MakeIndex(a_RelPos)); FindResult != m_BlockEntities.end())
-	if (BlockEntity != nullptr)
 	{
-		BlockEntity->Destroy();
+		auto & BlockEntity = *FindResult->second;
-		BlockEntity->OnRemoveFromWorld();
+
-		RemoveBlockEntity(BlockEntity);
+		BlockEntity.Destroy();
+		BlockEntity.OnRemoveFromWorld();
+
+		m_BlockEntities.erase(FindResult);
+		m_PendingSendBlockEntities.erase(std::remove(m_PendingSendBlockEntities.begin(), m_PendingSendBlockEntities.end(), &BlockEntity), m_PendingSendBlockEntities.end());
 	}
 
 	// If the new block is a block entity, create the entity object:
@@ -1414,11 +1422,14 @@ void cChunk::SendBlockTo(int a_RelX, int a_RelY, int a_RelZ, cClientHandle * a_C
 
 void cChunk::AddBlockEntity(OwnedBlockEntity a_BlockEntity)
 {
+	const auto BlockEntityPtr = a_BlockEntity.get();
 	[[maybe_unused]] const auto Result = m_BlockEntities.emplace(
 		cChunkDef::MakeIndex(a_BlockEntity->GetRelX(), a_BlockEntity->GetPosY(), a_BlockEntity->GetRelZ()),
 		std::move(a_BlockEntity)
 	);
-	ASSERT(Result.second);  // No block entity already at this position
+
+	ASSERT(Result.second);  // No block entity already at this position.
+	BlockEntityPtr->OnAddToWorld(*m_World, *this);
 }
 
 
@@ -1435,7 +1446,7 @@ cBlockEntity * cChunk::GetBlockEntity(Vector3i a_AbsPos)
 		return nullptr;
 	}
 
-	auto itr = m_BlockEntities.find(static_cast<size_t>(cChunkDef::MakeIndexNoCheck(relPos)));
+	auto itr = m_BlockEntities.find(cChunkDef::MakeIndex(relPos));
 	return (itr == m_BlockEntities.end()) ? nullptr : itr->second.get();
 }
 
@@ -1446,7 +1457,7 @@ cBlockEntity * cChunk::GetBlockEntity(Vector3i a_AbsPos)
 cBlockEntity * cChunk::GetBlockEntityRel(Vector3i a_RelPos)
 {
 	ASSERT(cChunkDef::IsValidRelPos(a_RelPos));
-	auto itr = m_BlockEntities.find(static_cast<size_t>(cChunkDef::MakeIndexNoCheck(a_RelPos)));
+	auto itr = m_BlockEntities.find(cChunkDef::MakeIndex(a_RelPos));
 	return (itr == m_BlockEntities.end()) ? nullptr : itr->second.get();
 }
 
@@ -1527,18 +1538,6 @@ void cChunk::SetAreaBiome(int a_MinRelX, int a_MaxRelX, int a_MinRelZ, int a_Max
 
 
 
-void cChunk::RemoveBlockEntity(cBlockEntity * a_BlockEntity)
-{
-	MarkDirty();
-	ASSERT(a_BlockEntity != nullptr);
-	m_BlockEntities.erase(static_cast<size_t>(cChunkDef::MakeIndex(a_BlockEntity->GetRelX(), a_BlockEntity->GetPosY(), a_BlockEntity->GetRelZ())));
-	m_PendingSendBlockEntities.erase(std::remove(m_PendingSendBlockEntities.begin(), m_PendingSendBlockEntities.end(), a_BlockEntity), m_PendingSendBlockEntities.end());
-}
-
-
-
-
-
 bool cChunk::AddClient(cClientHandle * a_Client)
 {
 	if (std::find(m_LoadedByClient.begin(), m_LoadedByClient.end(), a_Client) != m_LoadedByClient.end())

src/Chunk.h

@@ -114,9 +114,6 @@ public:
 	/** Writes the specified cBlockArea at the coords specified. Note that the coords may extend beyond the chunk! */
 	void WriteBlockArea(cBlockArea & a_Area, int a_MinBlockX, int a_MinBlockY, int a_MinBlockZ, int a_DataTypes);
 
-	/** Returns true if there is a block entity at the coords specified */
-	bool HasBlockEntityAt(Vector3i a_BlockPos);
-
 	/** Sets or resets the internal flag that prevents chunk from being unloaded.
 	The flag is cumulative - it can be set multiple times and then needs to be un-set that many times
 	before the chunk is unloadable again. */
@@ -493,7 +490,7 @@ private:
 
 	/** Block entities that have been touched and need to be sent to all clients.
 	Because block changes are buffered and we need to happen after them, this buffer exists too.
-	Pointers to block entities that were destroyed are guaranteed to be removed from this array by RemoveBlockEntity. */
+	Pointers to block entities that were destroyed are guaranteed to be removed from this array by SetAllData, SetBlock, WriteBlockArea. */
 	std::vector<cBlockEntity *> m_PendingSendBlockEntities;
 
 	/** A queue of relative positions to call cBlockHandler::Check on.
@@ -543,8 +540,8 @@ private:
 	void GetRandomBlockCoords(int & a_X, int & a_Y, int & a_Z);
 	void GetThreeRandomNumbers(int & a_X, int & a_Y, int & a_Z, int a_MaxX, int a_MaxY, int a_MaxZ);
 
-	void RemoveBlockEntity(cBlockEntity * a_BlockEntity);
+	/** Takes ownership of a block entity, which MUST actually reside in this chunk. */
-	void AddBlockEntity   (OwnedBlockEntity a_BlockEntity);
+	void AddBlockEntity(OwnedBlockEntity a_BlockEntity);
 
 	/** Wakes up each simulator for its specific blocks; through all the blocks in the chunk */
 	void WakeUpSimulators(void);

src/ChunkData.cpp

@@ -26,7 +26,7 @@ namespace
 		return
 		{
 			static_cast<size_t>(a_RelPos.y / cChunkDef::SectionHeight),
-			static_cast<size_t>(cChunkDef::MakeIndexNoCheck(a_RelPos.x, a_RelPos.y % cChunkDef::SectionHeight, a_RelPos.z))
+			cChunkDef::MakeIndex(a_RelPos.x, a_RelPos.y % cChunkDef::SectionHeight, a_RelPos.z)
 		};
 	}
 

src/ChunkDef.h

@@ -207,36 +207,22 @@ public:
 	}
 
 
-	inline static int MakeIndex(int x, int y, int z)
+	inline static size_t MakeIndex(int x, int y, int z)
 	{
-		if (
+		ASSERT(IsValidRelPos({ x, y, z }));
-			(x < Width)  && (x > -1) &&
-			(y < Height) && (y > -1) &&
-			(z < Width)  && (z > -1)
-		)
-		{
-			return MakeIndexNoCheck(x, y, z);
-		}
-		FLOGERROR("cChunkDef::MakeIndex(): coords out of range: {0}; returning fake index 0", Vector3i{x, y, z});
-		ASSERT(!"cChunkDef::MakeIndex(): coords out of chunk range!");
-		return 0;
-	}
 
-
-	inline static int MakeIndexNoCheck(int x, int y, int z)
-	{
 		#if AXIS_ORDER == AXIS_ORDER_XZY
 			// For some reason, NOT using the Horner schema is faster. Weird.
-			return x + (z * cChunkDef::Width) + (y * cChunkDef::Width * cChunkDef::Width);   // 1.2 uses XZY
+			return static_cast<size_t>(x + (z * Width) + (y * Width * Width));   // 1.2 uses XZY
 		#elif AXIS_ORDER == AXIS_ORDER_YZX
-			return y + (z * cChunkDef::Width) + (x * cChunkDef::Height * cChunkDef::Width);  // 1.1 uses YZX
+			return static_cast<size_t>(y + (z * Width) + (x * Height * Width));  // 1.1 uses YZX
 		#endif
 	}
 
 
-	inline static int MakeIndexNoCheck(Vector3i a_RelPos)
+	inline static size_t MakeIndex(Vector3i a_RelPos)
 	{
-		return MakeIndexNoCheck(a_RelPos.x, a_RelPos.y, a_RelPos.z);
+		return MakeIndex(a_RelPos.x, a_RelPos.y, a_RelPos.z);
 	}
 
 
@@ -263,7 +249,7 @@ public:
 		ASSERT((a_X >= 0) && (a_X < Width));
 		ASSERT((a_Y >= 0) && (a_Y < Height));
 		ASSERT((a_Z >= 0) && (a_Z < Width));
-		a_BlockTypes[MakeIndexNoCheck(a_X, a_Y, a_Z)] = a_Type;
+		a_BlockTypes[MakeIndex(a_X, a_Y, a_Z)] = a_Type;
 	}
 
 
@@ -277,7 +263,7 @@ public:
 	inline static BLOCKTYPE GetBlock(const BLOCKTYPE * a_BlockTypes, Vector3i a_RelPos)
 	{
 		ASSERT(IsValidRelPos(a_RelPos));
-		return a_BlockTypes[MakeIndexNoCheck(a_RelPos)];
+		return a_BlockTypes[MakeIndex(a_RelPos)];
 	}
 
 
@@ -286,7 +272,7 @@ public:
 		ASSERT((a_X >= 0) && (a_X < Width));
 		ASSERT((a_Y >= 0) && (a_Y < Height));
 		ASSERT((a_Z >= 0) && (a_Z < Width));
-		return a_BlockTypes[MakeIndexNoCheck(a_X, a_Y, a_Z)];
+		return a_BlockTypes[MakeIndex(a_X, a_Y, a_Z)];
 	}
 
 
@@ -333,8 +319,7 @@ public:
 	{
 		if ((x < Width) && (x > -1) && (y < Height) && (y > -1) && (z < Width) && (z > -1))
 		{
-			int Index = MakeIndexNoCheck(x, y, z);
+			return ExpandNibble(a_Buffer, MakeIndex(x, y, z));
-			return ExpandNibble(a_Buffer, static_cast<size_t>(Index));
 		}
 		ASSERT(!"cChunkDef::GetNibble(): coords out of chunk range!");
 		return 0;
@@ -436,7 +421,6 @@ struct sSetBlock
 	}
 };
 
-typedef std::list<sSetBlock> sSetBlockList;
 typedef std::vector<sSetBlock> sSetBlockVector;
 
 typedef std::list<cChunkCoords> cChunkCoordsList;
@@ -461,27 +445,6 @@ public:
 
 
 
-class cChunkCoordsWithBool
-{
-public:
-	int m_ChunkX;
-	int m_ChunkZ;
-	bool m_ForceGenerate;
-
-	cChunkCoordsWithBool(int a_ChunkX, int a_ChunkZ, bool a_ForceGenerate) : m_ChunkX(a_ChunkX), m_ChunkZ(a_ChunkZ), m_ForceGenerate(a_ForceGenerate){}
-
-	bool operator == (const cChunkCoordsWithBool & a_Other) const
-	{
-		return ((m_ChunkX == a_Other.m_ChunkX) && (m_ChunkZ == a_Other.m_ChunkZ) && (m_ForceGenerate == a_Other.m_ForceGenerate));
-	}
-};
-
-typedef std::list<cChunkCoordsWithBool> cChunkCoordsWithBoolList;
-
-
-
-
-
 /** Interface class used as a callback for operations that involve chunk coords */
 class cChunkCoordCallback
 {
@@ -518,7 +481,6 @@ public:
 } ;
 
 typedef cCoordWithData<int>        cCoordWithInt;
-typedef cCoordWithData<BLOCKTYPE>  cCoordWithBlock;
 
 typedef std::list<cCoordWithInt>   cCoordWithIntList;
 typedef std::vector<cCoordWithInt> cCoordWithIntVector;

src/Generating/Caves.cpp

@@ -513,7 +513,7 @@ void cCaveTunnel::ProcessChunk(
 				{
 					if (cChunkDef::GetBlock(a_BlockTypes, x, y, z) == E_BLOCK_SAND)
 					{
-						int Index = cChunkDef::MakeIndexNoCheck(x, y, z);
+						const auto Index = cChunkDef::MakeIndex(x, y, z);
 						if (a_BlockMetas[Index] == 1)
 						{
 							a_BlockMetas[Index] = 0;

src/Generating/ChunkDesc.cpp

@@ -572,8 +572,8 @@ void cChunkDesc::RandomFillRelCuboid(
 
 cBlockEntity * cChunkDesc::GetBlockEntity(int a_RelX, int a_RelY, int a_RelZ)
 {
-	auto Idx = static_cast<size_t>(cChunkDef::MakeIndex(a_RelX, a_RelY, a_RelZ));
+	const auto Idx = cChunkDef::MakeIndex(a_RelX, a_RelY, a_RelZ);
-	auto itr = m_BlockEntities.find(Idx);
+	const auto itr = m_BlockEntities.find(Idx);
 
 	if (itr != m_BlockEntities.end())
 	{

src/Generating/FinishGen.cpp

@@ -1227,7 +1227,7 @@ void cFinishGenBottomLava::GenFinish(cChunkDesc & a_ChunkDesc)
 	{
 		for (int z = 0; z < cChunkDef::Width; z++) for (int x = 0; x < cChunkDef::Width; x++)
 		{
-			int Index = cChunkDef::MakeIndexNoCheck(x, y, z);
+			const auto Index = cChunkDef::MakeIndex(x, y, z);
 			if (BlockTypes[Index] == E_BLOCK_AIR)
 			{
 				BlockTypes[Index] = E_BLOCK_STATIONARY_LAVA;
@@ -2000,8 +2000,8 @@ void cFinishGenOreNests::GenerateOre(
 							continue;
 						}
 
-						int Index = cChunkDef::MakeIndexNoCheck(BlockX, BlockY, BlockZ);
+						const auto Index = cChunkDef::MakeIndex(BlockX, BlockY, BlockZ);
-						auto blockType = blockTypes[Index];
+						const auto blockType = blockTypes[Index];
 						if ((blockType == E_BLOCK_STONE) || (blockType == E_BLOCK_NETHERRACK))
 						{
 							blockTypes[Index] = a_OreType;

src/Items/ItemMobHead.h

@@ -244,10 +244,7 @@ public:
 				(BlockType == E_BLOCK_HEAD) &&
 				!a_World.DoWithBlockEntityAt({ BlockX, BlockY, BlockZ }, [&](cBlockEntity & a_BlockEntity)
 				{
-					if (a_BlockEntity.GetBlockType() != E_BLOCK_HEAD)
+					ASSERT(a_BlockEntity.GetBlockType() == E_BLOCK_HEAD);
-					{
-						return false;
-					}
 
 					return static_cast<cMobHeadEntity &>(a_BlockEntity).GetType() == SKULL_TYPE_WITHER;
 				})

src/Mobs/PathFinder.cpp

@@ -192,7 +192,7 @@ bool cPathFinder::EnsureProperPoint(Vector3d & a_Vector, cChunk & a_Chunk)
 	// This fixes the player leaning issue.
 	// If that failed, we instead go down to the lowest air block.
 	int YBelowUs = FloorC(a_Vector.y) - 1;
-	if (YBelowUs < 0)
+	if (!cChunkDef::IsValidHeight(YBelowUs))
 	{
 		return false;
 

src/Protocol/Protocol_1_11.cpp

@@ -463,8 +463,16 @@ void cProtocol_1_11_0::SendUpdateBlockEntity(cBlockEntity & a_BlockEntity)
 	Byte Action;
 	switch (a_BlockEntity.GetBlockType())
 	{
-		case E_BLOCK_ENCHANTMENT_TABLE: Action = 0;  break;  // The ones with a action of 0 is just a workaround to send the block entities to a client.
+		case E_BLOCK_CHEST:
-		case E_BLOCK_END_PORTAL:        Action = 0;  break;  // Todo: 18.09.2020 - remove this when block entities are transmitted in the ChunkData packet - 12xx12
+		case E_BLOCK_ENCHANTMENT_TABLE:
+		case E_BLOCK_END_PORTAL:
+		case E_BLOCK_TRAPPED_CHEST:
+		{
+			// The ones with a action of 0 is just a workaround to send the block entities to a client.
+			// Todo: 18.09.2020 - remove this when block entities are transmitted in the ChunkData packet - 12xx12
+			Action = 0;
+			break;
+		}
 
 		case E_BLOCK_MOB_SPAWNER:       Action = 1;  break;  // Update mob spawner spinny mob thing
 		case E_BLOCK_COMMAND_BLOCK:     Action = 2;  break;  // Update command block text

src/Protocol/Protocol_1_13.cpp

@@ -205,8 +205,16 @@ void cProtocol_1_13::SendUpdateBlockEntity(cBlockEntity & a_BlockEntity)
 	Byte Action;
 	switch (a_BlockEntity.GetBlockType())
 	{
-		case E_BLOCK_ENCHANTMENT_TABLE: Action = 0; break;  // The ones with a action of 0 is just a workaround to send the block entities to a client.
+		case E_BLOCK_CHEST:
-		case E_BLOCK_END_PORTAL:        Action = 0; break;  // Todo: 18.09.2020 - remove this when block entities are transmitted in the ChunkData packet - 12xx12
+		case E_BLOCK_ENCHANTMENT_TABLE:
+		case E_BLOCK_END_PORTAL:
+		case E_BLOCK_TRAPPED_CHEST:
+		{
+			// The ones with a action of 0 is just a workaround to send the block entities to a client.
+			// Todo: 18.09.2020 - remove this when block entities are transmitted in the ChunkData packet - 12xx12
+			Action = 0;
+			break;
+		}
 
 		case E_BLOCK_MOB_SPAWNER:       Action = 1;  break;  // Update mob spawner spinny mob thing
 		case E_BLOCK_COMMAND_BLOCK:     Action = 2;  break;  // Update command block text
@@ -464,6 +472,7 @@ UInt32 cProtocol_1_13::GetPacketID(ePacketType a_PacketType) const
 		case pktUpdateScore:            return 0x48;
 		case pktUpdateSign:             return GetPacketID(pktUpdateBlockEntity);
 		case pktUseBed:                 return 0x33;
+		case pktWeather:                return 0x20;
 		case pktWindowClose:            return 0x13;
 		case pktWindowItems:            return 0x15;
 		case pktWindowOpen:             return 0x14;

src/Protocol/Protocol_1_13.h

@@ -45,9 +45,6 @@ protected:
 	virtual void SendStatistics                 (const cStatManager & a_Manager) override;
 	virtual void SendTabCompletionResults       (const AStringVector & a_Results) override;
 	virtual void SendUpdateBlockEntity          (cBlockEntity & a_BlockEntity) override;
-
-	virtual void SendWeather                    (eWeather a_Weather) override {}  // This packet was removed. This keeps players from joining the server with 1.13 while there is downfall
-
 	virtual UInt8 GetEntityMetadataID(EntityMetadata a_Metadata) const;
 	virtual UInt8 GetEntityMetadataID(EntityMetadataType a_FieldType) const;
 	virtual std::pair<short, short> GetItemFromProtocolID(UInt32 a_ProtocolID) const;

src/Protocol/Protocol_1_8.cpp

@@ -1582,8 +1582,16 @@ void cProtocol_1_8_0::SendUpdateBlockEntity(cBlockEntity & a_BlockEntity)
 	Byte Action;
 	switch (a_BlockEntity.GetBlockType())
 	{
-		case E_BLOCK_ENCHANTMENT_TABLE: Action = 0;  break;  // The ones with a action of 0 is just a workaround to send the block entities to a client.
+		case E_BLOCK_CHEST:
-		case E_BLOCK_END_PORTAL:        Action = 0;  break;  // Todo: 18.09.2020 - remove this when block entities are transmitted in the ChunkData packet - 12xx12
+		case E_BLOCK_ENCHANTMENT_TABLE:
+		case E_BLOCK_END_PORTAL:
+		case E_BLOCK_TRAPPED_CHEST:
+		{
+			// The ones with a action of 0 is just a workaround to send the block entities to a client.
+			// Todo: 18.09.2020 - remove this when block entities are transmitted in the ChunkData packet - 12xx12
+			Action = 0;
+			break;
+		}
 
 		case E_BLOCK_MOB_SPAWNER:       Action = 1;  break;  // Update mob spawner spinny mob thing
 		case E_BLOCK_COMMAND_BLOCK:     Action = 2;  break;  // Update command block text

src/Simulator/DelayedFluidSimulator.cpp

@@ -22,11 +22,11 @@ bool cDelayedFluidSimulatorChunkData::cSlot::Add(int a_RelX, int a_RelY, int a_R
 	ASSERT(a_RelZ >= 0);
 	ASSERT(a_RelZ < static_cast<int>(ARRAYCOUNT(m_Blocks)));
 
-	cCoordWithIntVector & Blocks = m_Blocks[a_RelZ];
+	auto & Blocks = m_Blocks[a_RelZ];
-	int Index = cChunkDef::MakeIndexNoCheck(a_RelX, a_RelY, a_RelZ);
+	const auto Index = cChunkDef::MakeIndex(a_RelX, a_RelY, a_RelZ);
-	for (cCoordWithIntVector::const_iterator itr = Blocks.begin(), end = Blocks.end(); itr != end; ++itr)
+	for (const auto & Block : Blocks)
 	{
-		if (itr->Data == Index)
+		if (Block.Data == Index)
 		{
 			// Already present
 			return false;
@@ -101,14 +101,14 @@ void cDelayedFluidSimulator::SimulateChunk(std::chrono::milliseconds a_Dt, int a
 	// Simulate all the blocks in the scheduled slot:
 	for (size_t i = 0; i < ARRAYCOUNT(Slot.m_Blocks); i++)
 	{
-		cCoordWithIntVector & Blocks = Slot.m_Blocks[i];
+		auto & Blocks = Slot.m_Blocks[i];
 		if (Blocks.empty())
 		{
 			continue;
 		}
-		for (cCoordWithIntVector::iterator itr = Blocks.begin(), end = Blocks.end(); itr != end; ++itr)
+		for (const auto & Block : Blocks)
 		{
-			SimulateBlock(a_Chunk, itr->x, itr->y, itr->z);
+			SimulateBlock(a_Chunk, Block.x, Block.y, Block.z);
 		}
 		m_TotalBlocks -= static_cast<int>(Blocks.size());
 		Blocks.clear();

src/Simulator/DelayedFluidSimulator.h

@@ -29,9 +29,9 @@ public:
 		bool Add(int a_RelX, int a_RelY, int a_RelZ);
 
 		/** Array of block containers, each item stores blocks for one Z coord
-		Int param is the block index (for faster duplicate comparison in Add())
+		size_t param is the block index (for faster duplicate comparison in Add())
 		*/
-		cCoordWithIntVector m_Blocks[16];
+		std::vector<cCoordWithData<size_t>> m_Blocks[16];
 	} ;
 
 	cDelayedFluidSimulatorChunkData(int a_TickDelay);

src/Simulator/IncrementalRedstoneSimulator/CommandBlockHandler.h

@@ -33,10 +33,7 @@ namespace CommandBlockHandler
 
 		a_Chunk.DoWithBlockEntityAt(a_Position, [](cBlockEntity & a_BlockEntity)
 		{
-			if (a_BlockEntity.GetBlockType() != E_BLOCK_COMMAND_BLOCK)
+			ASSERT(a_BlockEntity.GetBlockType() == E_BLOCK_COMMAND_BLOCK);
-			{
-				return false;
-			}
 
 			static_cast<cCommandBlockEntity &>(a_BlockEntity).Activate();
 			return false;

src/Simulator/IncrementalRedstoneSimulator/DropSpenserHandler.h

@@ -48,10 +48,7 @@ namespace DropSpenserHandler
 		{
 			a_Chunk.DoWithBlockEntityAt(a_Position, [](cBlockEntity & a_BlockEntity)
 			{
-				if ((a_BlockEntity.GetBlockType() != E_BLOCK_DISPENSER) && (a_BlockEntity.GetBlockType() != E_BLOCK_DROPPER))
+				ASSERT((a_BlockEntity.GetBlockType() == E_BLOCK_DISPENSER) || (a_BlockEntity.GetBlockType() == E_BLOCK_DROPPER));
-				{
-					return false;
-				}
 
 				static_cast<cDropSpenserEntity &>(a_BlockEntity).Activate();
 				return false;

src/Simulator/IncrementalRedstoneSimulator/HopperHandler.h

@@ -43,10 +43,7 @@ namespace HopperHandler
 
 		a_Chunk.DoWithBlockEntityAt(a_Position, [ShouldBeLocked](cBlockEntity & a_BlockEntity)
 		{
-			if (a_BlockEntity.GetBlockType() != E_BLOCK_HOPPER)
+			ASSERT(a_BlockEntity.GetBlockType() == E_BLOCK_HOPPER);
-			{
-				return false;
-			}
 
 			static_cast<cHopperEntity &>(a_BlockEntity).SetLocked(ShouldBeLocked);
 			return false;

src/Simulator/IncrementalRedstoneSimulator/NoteBlockHandler.h

@@ -33,10 +33,7 @@ namespace NoteBlockHandler
 
 		a_Chunk.DoWithBlockEntityAt(a_Position, [](cBlockEntity & a_BlockEntity)
 		{
-			if (a_BlockEntity.GetBlockType() != E_BLOCK_NOTE_BLOCK)
+			ASSERT(a_BlockEntity.GetBlockType() == E_BLOCK_NOTE_BLOCK);
-			{
-				return false;
-			}
 
 			static_cast<cNoteEntity &>(a_BlockEntity).MakeSound();
 			return false;

src/Simulator/IncrementalRedstoneSimulator/TrappedChestHandler.h

@@ -24,10 +24,7 @@ namespace TrappedChestHandler
 		int NumberOfPlayers = 0;
 		a_Chunk.DoWithBlockEntityAt(a_Position, [&NumberOfPlayers](cBlockEntity & a_BlockEntity)
 		{
-			if (a_BlockEntity.GetBlockType() != E_BLOCK_TRAPPED_CHEST)
+			ASSERT(a_BlockEntity.GetBlockType() == E_BLOCK_TRAPPED_CHEST);
-			{
-				return false;
-			}
 
 			NumberOfPlayers = static_cast<cChestEntity &>(a_BlockEntity).GetNumberOfPlayers();
 			return false;

src/WorldStorage/WSSAnvil.cpp

@@ -439,7 +439,7 @@ bool cWSSAnvil::LoadChunkFromNBT(const cChunkCoords & a_Chunk, const cParsedNBT
 	{
 		for (int x = 0; x < cChunkDef::Width; x++) for (int z = 0; z < cChunkDef::Width; z++)
 		{
-			int Index = cChunkDef::MakeIndexNoCheck(x, y, z);
+			const auto Index = cChunkDef::MakeIndex(x, y, z);
 			if (ShouldInvert[x + cChunkDef::Width * z])
 			{
 				BlockTypes[Index] = (BlockTypes[Index] == E_BLOCK_AIR) ? E_BLOCK_STONE : E_BLOCK_AIR;
@@ -618,10 +618,10 @@ void cWSSAnvil::LoadBlockEntitiesFromNBT(cBlockEntities & a_BlockEntities, const
 		}
 
 		// Index computed before Entity moved.
-		const auto Idx = cChunkDef::MakeIndexNoCheck(Entity->GetRelPos());
+		const auto Index = cChunkDef::MakeIndex(Entity->GetRelPos());
 
 		// Add the BlockEntity to the loaded data:
-		a_BlockEntities.emplace(Idx, std::move(Entity));
+		a_BlockEntities.emplace(Index, std::move(Entity));
 	}  // for Child - tag children
 }