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/ChestEntity.cpp | 260 ++++++++++++++++++-------------------- 1 file changed, 126 insertions(+), 134 deletions(-) (limited to 'src/BlockEntities/ChestEntity.cpp') 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); } -- cgit v1.2.3