From 9b97d63f8f939dbc431cc2dcd9eddf959f86603a Mon Sep 17 00:00:00 2001 From: Tiger Wang Date: Fri, 30 Apr 2021 14:23:46 +0100 Subject: 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/BlockEntities/BlockEntity.cpp | 9 ++ src/BlockEntities/BlockEntity.h | 9 +- src/BlockEntities/ChestEntity.cpp | 260 ++++++++++++++++++------------------- src/BlockEntities/ChestEntity.h | 46 ++++--- src/BlockEntities/HopperEntity.cpp | 118 ++++------------- 5 files changed, 194 insertions(+), 248 deletions(-) (limited to 'src/BlockEntities') diff --git a/src/BlockEntities/BlockEntity.cpp b/src/BlockEntities/BlockEntity.cpp index 4b17f10a6..7c0660431 100644 --- a/src/BlockEntities/BlockEntity.cpp +++ b/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() { } diff --git a/src/BlockEntities/BlockEntity.h b/src/BlockEntities/BlockEntity.h index 3e9cf38e7..e13600e8c 100644 --- a/src/BlockEntities/BlockEntity.h +++ b/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. diff --git a/src/BlockEntities/ChestEntity.cpp b/src/BlockEntities/ChestEntity.cpp index 3c80a7aa3..c2c31b30a 100644 --- a/src/BlockEntities/ChestEntity.cpp +++ b/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,167 +19,144 @@ 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) - ) +} + + + + + +cChestEntity & cChestEntity::GetPrimaryChest() +{ + if (m_Neighbour == nullptr) { - ScanNeighbours(); + return *this; } + + // The primary chest should be the one with lesser X or Z coord: + return ( + (m_Neighbour->GetPosX() < GetPosX()) || + (m_Neighbour->GetPosZ() < GetPosZ()) + ) ? *m_Neighbour : *this; } -void cChestEntity::CopyFrom(const cBlockEntity & a_Src) +cChestEntity * cChestEntity::GetSecondaryChest() { - Super::CopyFrom(a_Src); - auto & src = static_cast(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; + // If we're the primary, then our neighbour is the secondary, and vice versa: + return (&GetPrimaryChest() == this) ? m_Neighbour : this; } -void cChestEntity::OnRemoveFromWorld() +bool cChestEntity::ScanNeighbour(cChunk & a_Chunk, Vector3i a_Position) { - if (m_Neighbour != nullptr) + const auto Chunk = a_Chunk.GetRelNeighborChunkAdjustCoords(a_Position); + + if ((Chunk == nullptr) || !Chunk->IsValid()) { - // Neighbour may share a window with us, force the window shut: - m_Neighbour->DestroyWindow(); - m_Neighbour->m_Neighbour = nullptr; + // If a chest was in fact there, they'll find us when their chunk loads. + return false; } - DestroyWindow(); + const auto BlockEntity = Chunk->GetBlockEntityRel(a_Position); + + if ((BlockEntity == nullptr) || (BlockEntity->GetBlockType() != m_BlockType)) + { + // Neighbouring block is not the same type of chest: + return false; + } + + m_Neighbour = static_cast(BlockEntity); + return true; } -void cChestEntity::SendTo(cClientHandle & a_Client) +void cChestEntity::DestroyWindow() { - // 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); + const auto Window = GetWindow(); + if (Window != nullptr) + { + Window->OwnerDestroyed(); + } } -bool cChestEntity::UsedBy(cPlayer * a_Player) +bool cChestEntity::IsBlocked() { - if (IsBlocked()) - { - // Obstruction, don't open - return true; - } + return ( + (GetPosY() < cChunkDef::Height - 1) && + ( + !cBlockInfo::IsTransparent(GetWorld()->GetBlock(GetPos().addedY(1))) || + !cOcelot::IsCatSittingOnBlock(GetWorld(), Vector3d(GetPos())) + ) + ); +} - if (m_Neighbour == nullptr) - { - ScanNeighbours(); - } - // The primary chest should be the one with lesser X or Z coord: - cChestEntity * PrimaryChest = this; - if (m_Neighbour != nullptr) - { - if (m_Neighbour->IsBlocked()) - { - // Obstruction, don't open - return true; - } - if ( - (m_Neighbour->GetPosX() > GetPosX()) || - (m_Neighbour->GetPosZ() > GetPosZ()) - ) - { - PrimaryChest = m_Neighbour; - } - } - if (m_BlockType == E_BLOCK_CHEST) - { - a_Player->GetStatManager().AddValue(Statistic::OpenChest); - } - else // E_BLOCK_TRAPPED_CHEST - { - a_Player->GetStatManager().AddValue(Statistic::TriggerTrappedChest); - } - // If the window is not created, open it anew: - cWindow * Window = PrimaryChest->GetWindow(); - if (Window == nullptr) +void cChestEntity::OpenNewWindow(void) +{ + if (m_Neighbour != nullptr) { - PrimaryChest->OpenNewWindow(); - Window = PrimaryChest->GetWindow(); - } + ASSERT(&GetPrimaryChest() == this); // Should only open windows for the primary chest. - // Open the window for the player: - if (Window != nullptr) + OpenWindow(new cChestWindow(this, m_Neighbour)); + } + else { - if (a_Player->GetWindow() != Window) - { - a_Player->OpenWindow(*Window); - } + // There is no chest neighbour, open a single-chest window: + OpenWindow(new cChestWindow(this)); } - - // 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; } -cChestEntity * cChestEntity::GetNeighbour() +void cChestEntity::CopyFrom(const cBlockEntity & a_Src) { - return m_Neighbour; + Super::CopyFrom(a_Src); + auto & src = static_cast(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::ScanNeighbours() +void cChestEntity::OnAddToWorld(cWorld & a_World, cChunk & a_Chunk) { - // 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(&a_BlockEntity); - return true; - }; + Super::OnAddToWorld(a_World, a_Chunk); // 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) + 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; - // Force neighbour's window shut. Does Mojang server do this or should a double window open? - m_Neighbour->DestroyWindow(); + m_Neighbour->DestroyWindow(); // Force neighbour's window shut. Does Mojang server do this or should a double window open? } } @@ -186,49 +164,73 @@ void cChestEntity::ScanNeighbours() -void cChestEntity::OpenNewWindow(void) +void cChestEntity::OnRemoveFromWorld() { 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)); + // Neighbour may share a window with us, force the window shut: + m_Neighbour->DestroyWindow(); + m_Neighbour->m_Neighbour = nullptr; } + + DestroyWindow(); } -void cChestEntity::DestroyWindow() +void cChestEntity::SendTo(cClientHandle & a_Client) { - const auto Window = GetWindow(); - if (Window != nullptr) - { - Window->OwnerDestroyed(); - } + a_Client.SendUpdateBlockEntity(*this); } -bool cChestEntity::IsBlocked() +bool cChestEntity::UsedBy(cPlayer * a_Player) { - return ( - (GetPosY() < cChunkDef::Height - 1) && - ( - !cBlockInfo::IsTransparent(GetWorld()->GetBlock(GetPos().addedY(1))) || - !cOcelot::IsCatSittingOnBlock(GetWorld(), Vector3d(GetPos())) - ) - ); + 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; } @@ -239,11 +241,6 @@ 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); } diff --git a/src/BlockEntities/ChestEntity.h b/src/BlockEntities/ChestEntity.h index 02b182481..ee3c59b8b 100644 --- a/src/BlockEntities/ChestEntity.h +++ b/src/BlockEntities/ChestEntity.h @@ -34,32 +34,20 @@ 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; + /** Gets the number of players who currently have this chest open */ + int GetNumberOfPlayers(void) const { return m_NumActivePlayers; } - /** Search horizontally adjacent blocks for neighbouring chests of the same type and links them together. */ - void ScanNeighbours(); + /** Returns the associated primary chest. */ + cChestEntity & GetPrimaryChest(); - /** Returns the value of m_Neighbour. */ - cChestEntity * GetNeighbour(); + /** Returns the associated secondary chest, if any. */ + cChestEntity * GetSecondaryChest(); - /** 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; } + /** 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 diff --git a/src/BlockEntities/HopperEntity.cpp b/src/BlockEntities/HopperEntity.cpp index 5b273b13c..82e07f6a0 100644 --- a/src/BlockEntities/HopperEntity.cpp +++ b/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); - auto MainChest = static_cast(a_Chunk.GetBlockEntity(ChestPos)); - if (MainChest == nullptr) + const auto ConnectedBlockEntity = a_Chunk.GetBlockEntityRel(GetRelPos().addedY(1)); + + 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) - { - if (MoveItemsFromGrid(*MainChest)) - { - return true; - } - } - else + + const auto ConnectedChest = static_cast(ConnectedBlockEntity); + + if (MoveItemsFromGrid(ConnectedChest->GetPrimaryChest())) { - auto SideAbsCoords = SideChest->GetPos(); - // 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; - } - } + return true; } - // The chest was empty - return false; + + const auto SecondaryChest = ConnectedChest->GetSecondaryChest(); + return (SecondaryChest != nullptr) && MoveItemsFromGrid(*SecondaryChest); } @@ -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: - auto ConnectedChest = static_cast(a_Chunk.GetBlockEntity(a_Coords)); - if (ConnectedChest == nullptr) + const auto ConnectedBlockEntity = a_Chunk.GetBlockEntity(a_Coords); + + 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) - { - if (MoveItemsToGrid(*ConnectedChest)) - { - return true; - } - } - else + + const auto ConnectedChest = static_cast(ConnectedBlockEntity); + + if (MoveItemsToGrid(ConnectedChest->GetPrimaryChest())) { - auto SideAbsCoords = SideChest->GetPos(); - 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 true; } - return false; + + const auto SecondaryChest = ConnectedChest->GetSecondaryChest(); + return (SecondaryChest != nullptr) && MoveItemsToGrid(*SecondaryChest); } @@ -661,7 +599,3 @@ bool cHopperEntity::MoveItemsToSlot(cBlockEntityWithItems & a_Entity, int a_DstS return false; } } - - - - -- cgit v1.2.3