From 33b9c5dc6dae42fb6fc33283ded888a8001e5394 Mon Sep 17 00:00:00 2001 From: hle0 <91701075+hle0@users.noreply.github.com> Date: Thu, 7 Nov 2024 18:05:47 -0500 Subject: Fix cChunkMap issues below with coords below y=0 (#5397) * return false in cChunkMap::GetBlockTypeMeta if requested height is invalid * add checks to users of cWorld::GetBlockTypeMeta * add checks for invalid height to cChunkMap::GetBlock and cChunkMap::GetBlockMeta * add hle0 to CONTRIBUTORS * Fix merge conflict with isValidHeight * Add initialisation contract and fulfil it. --------- Co-authored-by: Alexander Harkness --- src/Blocks/BlockPiston.cpp | 59 ++++++++++++++++++++----------- src/ChunkMap.cpp | 22 ++++++++++++ src/ChunkMap.h | 4 +++ src/ClientHandle.cpp | 10 ++++-- src/Entities/Player.cpp | 7 ++-- src/Items/ItemBed.h | 5 ++- src/Items/ItemBigFlower.h | 5 ++- src/Items/ItemDoor.h | 5 ++- src/Items/ItemDye.h | 6 ++-- src/Items/ItemEyeOfEnder.h | 4 +-- src/Items/ItemHoe.h | 5 ++- src/Items/ItemShears.h | 5 ++- src/UI/SlotArea.cpp | 7 ++-- src/World.cpp | 34 +++++++++++------- tests/CompositeChat/CompositeChatTest.cpp | 1 + 15 files changed, 130 insertions(+), 49 deletions(-) diff --git a/src/Blocks/BlockPiston.cpp b/src/Blocks/BlockPiston.cpp index 124757b44..568e2eaa5 100644 --- a/src/Blocks/BlockPiston.cpp +++ b/src/Blocks/BlockPiston.cpp @@ -48,8 +48,12 @@ void cBlockPistonHandler::ExtendPiston(Vector3i a_BlockPos, cWorld & a_World) BLOCKTYPE pistonBlock; NIBBLETYPE pistonMeta; - a_World.GetBlockTypeMeta(a_BlockPos, pistonBlock, pistonMeta); - a_World.BroadcastBlockAction(a_BlockPos, PistonExtendAction, pistonMeta, pistonBlock); + if (a_World.GetBlockTypeMeta(a_BlockPos, pistonBlock, pistonMeta)) + { + a_World.BroadcastBlockAction( + a_BlockPos, PistonExtendAction, pistonMeta, pistonBlock + ); + } } // Client expects the server to "play" the animation before setting the final blocks @@ -60,9 +64,12 @@ void cBlockPistonHandler::ExtendPiston(Vector3i a_BlockPos, cWorld & a_World) { BLOCKTYPE pistonBlock; NIBBLETYPE pistonMeta; - World.GetBlockTypeMeta(a_BlockPos, pistonBlock, pistonMeta); - if ((pistonBlock != E_BLOCK_PISTON) && !IsSticky(pistonBlock)) + + if ( + !World.GetBlockTypeMeta(a_BlockPos, pistonBlock, pistonMeta) || + ((pistonBlock != E_BLOCK_PISTON) && !IsSticky(pistonBlock)) + ) { // Ensure we operate on a piston to avoid spurious behaviour // Note that the scheduled task may result in the block type of a_BlockPos changing @@ -104,17 +111,23 @@ void cBlockPistonHandler::RetractPiston(Vector3i a_BlockPos, cWorld & a_World) { BLOCKTYPE pistonBlock; NIBBLETYPE pistonMeta; - a_World.GetBlockTypeMeta(a_BlockPos, pistonBlock, pistonMeta); - a_World.BroadcastBlockAction(a_BlockPos, PistonRetractAction, pistonMeta, pistonBlock); + if (a_World.GetBlockTypeMeta(a_BlockPos, pistonBlock, pistonMeta)) + { + a_World.BroadcastBlockAction( + a_BlockPos, PistonRetractAction, pistonMeta, pistonBlock + ); + } } a_World.ScheduleTask(1_tick, [a_BlockPos](cWorld & World) { BLOCKTYPE pistonBlock; NIBBLETYPE pistonMeta; - World.GetBlockTypeMeta(a_BlockPos, pistonBlock, pistonMeta); - if ((pistonBlock != E_BLOCK_PISTON) && !IsSticky(pistonBlock)) + if ( + !World.GetBlockTypeMeta(a_BlockPos, pistonBlock, pistonMeta) || + ((pistonBlock != E_BLOCK_PISTON) && !IsSticky(pistonBlock)) + ) { // Ensure we operate on a piston to avoid spurious behaviour // Note that the scheduled task may result in the block type of a_BlockPos changing @@ -189,19 +202,20 @@ void cBlockPistonHandler::PushBlocks( NIBBLETYPE moveMeta; for (auto & moveBlockPos : sortedBlocks) { - a_World.GetBlockTypeMeta(moveBlockPos, moveBlock, moveMeta); - - if (cBlockInfo::IsPistonBreakable(moveBlock)) - { - // Block is breakable, drop it: - a_World.DropBlockAsPickups(moveBlockPos, nullptr, nullptr); - } - else + if (a_World.GetBlockTypeMeta(moveBlockPos, moveBlock, moveMeta)) { - // Not breakable, just move it - a_World.SetBlock(moveBlockPos, E_BLOCK_AIR, 0); - moveBlockPos += a_PushDir; - a_World.SetBlock(moveBlockPos, moveBlock, moveMeta); + if (cBlockInfo::IsPistonBreakable(moveBlock)) + { + // Block is breakable, drop it: + a_World.DropBlockAsPickups(moveBlockPos, nullptr, nullptr); + } + else + { + // Not breakable, just move it + a_World.SetBlock(moveBlockPos, E_BLOCK_AIR, 0); + moveBlockPos += a_PushDir; + a_World.SetBlock(moveBlockPos, moveBlock, moveMeta); + } } } } @@ -232,7 +246,10 @@ bool cBlockPistonHandler::CanPushBlock( BLOCKTYPE currBlock; NIBBLETYPE currMeta; - a_World.GetBlockTypeMeta(a_BlockPos, currBlock, currMeta); + if (!a_World.GetBlockTypeMeta(a_BlockPos, currBlock, currMeta)) + { + return !a_RequirePushable; + } if (currBlock == E_BLOCK_AIR) { diff --git a/src/ChunkMap.cpp b/src/ChunkMap.cpp index a2bc50fe9..d982d0cf3 100644 --- a/src/ChunkMap.cpp +++ b/src/ChunkMap.cpp @@ -474,6 +474,11 @@ void cChunkMap::CollectPickupsByEntity(cEntity & a_Entity) BLOCKTYPE cChunkMap::GetBlock(Vector3i a_BlockPos) const { + if (!cChunkDef::IsValidHeight(a_BlockPos)) + { + return 0; + } + auto chunkPos = cChunkDef::BlockToChunk(a_BlockPos); auto relPos = cChunkDef::AbsoluteToRelative(a_BlockPos, chunkPos); @@ -493,6 +498,11 @@ BLOCKTYPE cChunkMap::GetBlock(Vector3i a_BlockPos) const NIBBLETYPE cChunkMap::GetBlockMeta(Vector3i a_BlockPos) const { + if (!cChunkDef::IsValidHeight(a_BlockPos)) + { + return 0; + } + auto chunkPos = cChunkDef::BlockToChunk(a_BlockPos); auto relPos = cChunkDef::AbsoluteToRelative(a_BlockPos, chunkPos); @@ -585,6 +595,14 @@ void cChunkMap::SetBlock(Vector3i a_BlockPos, BLOCKTYPE a_BlockType, NIBBLETYPE bool cChunkMap::GetBlockTypeMeta(Vector3i a_BlockPos, BLOCKTYPE & a_BlockType, NIBBLETYPE & a_BlockMeta) const { + if (!cChunkDef::IsValidHeight(a_BlockPos)) + { + // Initialise the params to fulfil our contract. + a_BlockType = 0; + a_BlockMeta = 0; + return false; + } + auto chunkCoord = cChunkDef::BlockToChunk(a_BlockPos); auto relPos = cChunkDef::AbsoluteToRelative(a_BlockPos, chunkCoord); @@ -595,6 +613,10 @@ bool cChunkMap::GetBlockTypeMeta(Vector3i a_BlockPos, BLOCKTYPE & a_BlockType, N Chunk->GetBlockTypeMeta(relPos, a_BlockType, a_BlockMeta); return true; } + + // Initialise the params to fulfil our contract. + a_BlockType = 0; + a_BlockMeta = 0; return false; } diff --git a/src/ChunkMap.h b/src/ChunkMap.h index 578c49b8a..8538b7624 100644 --- a/src/ChunkMap.h +++ b/src/ChunkMap.h @@ -115,6 +115,10 @@ public: void SetBlockMeta(Vector3i a_BlockPos, NIBBLETYPE a_BlockMeta); void SetBlock (Vector3i a_BlockPos, BLOCKTYPE a_BlockType, NIBBLETYPE a_BlockMeta); + /** Get the block type and meta at the specified coords + Will always initialise a_BlockType and a_BlockMeta if called. + Returns false if the data could not be retrieved, either because the chunk is invalid or the height is invalid. + Otherwise, returns true. */ bool GetBlockTypeMeta (Vector3i a_BlockPos, BLOCKTYPE & a_BlockType, NIBBLETYPE & a_BlockMeta) const; bool GetBlockInfo (Vector3i, BLOCKTYPE & a_BlockType, NIBBLETYPE & a_Meta, NIBBLETYPE & a_SkyLight, NIBBLETYPE & a_BlockLight) const; diff --git a/src/ClientHandle.cpp b/src/ClientHandle.cpp index 01c95095a..2b32c5c07 100644 --- a/src/ClientHandle.cpp +++ b/src/ClientHandle.cpp @@ -1254,7 +1254,10 @@ void cClientHandle::HandleBlockDigStarted(Vector3i a_BlockPos, eBlockFace a_Bloc BLOCKTYPE DiggingBlock; NIBBLETYPE DiggingMeta; - m_Player->GetWorld()->GetBlockTypeMeta(a_BlockPos, DiggingBlock, DiggingMeta); + if (!m_Player->GetWorld()->GetBlockTypeMeta(a_BlockPos, DiggingBlock, DiggingMeta)) + { + return; + } if ( m_Player->IsGameModeCreative() && @@ -1322,7 +1325,10 @@ void cClientHandle::HandleBlockDigFinished(Vector3i a_BlockPos, eBlockFace a_Blo BLOCKTYPE DugBlock; NIBBLETYPE DugMeta; - m_Player->GetWorld()->GetBlockTypeMeta(a_BlockPos, DugBlock, DugMeta); + if (!m_Player->GetWorld()->GetBlockTypeMeta(a_BlockPos, DugBlock, DugMeta)) + { + return; + } if (!m_Player->IsGameModeCreative()) { diff --git a/src/Entities/Player.cpp b/src/Entities/Player.cpp index de99a299d..50fd034a5 100644 --- a/src/Entities/Player.cpp +++ b/src/Entities/Player.cpp @@ -2594,9 +2594,10 @@ bool cPlayer::IsInsideWater() BLOCKTYPE Block; NIBBLETYPE Meta; - m_World->GetBlockTypeMeta(EyePos, Block, Meta); - - if ((Block != E_BLOCK_WATER) && (Block != E_BLOCK_STATIONARY_WATER)) + if ( + !m_World->GetBlockTypeMeta(GetEyePosition().Floor(), Block, Meta) || + ((Block != E_BLOCK_WATER) && (Block != E_BLOCK_STATIONARY_WATER)) + ) { return false; } diff --git a/src/Items/ItemBed.h b/src/Items/ItemBed.h index a2e254171..77818e7c7 100644 --- a/src/Items/ItemBed.h +++ b/src/Items/ItemBed.h @@ -27,7 +27,10 @@ public: auto & World = *a_Player.GetWorld(); BLOCKTYPE HeadType; NIBBLETYPE HeadMeta; - World.GetBlockTypeMeta(HeadPosition, HeadType, HeadMeta); + if (!World.GetBlockTypeMeta(HeadPosition, HeadType, HeadMeta)) + { + return false; + } // Vanilla only allows beds to be placed into air. // Check if there is empty space for the "head" block: diff --git a/src/Items/ItemBigFlower.h b/src/Items/ItemBigFlower.h index cbdecbed7..a135a220d 100644 --- a/src/Items/ItemBigFlower.h +++ b/src/Items/ItemBigFlower.h @@ -29,7 +29,10 @@ public: const auto TopPos = a_PlacePosition.addedY(1); BLOCKTYPE TopType; NIBBLETYPE TopMeta; - World.GetBlockTypeMeta(TopPos, TopType, TopMeta); + if (!World.GetBlockTypeMeta(TopPos, TopType, TopMeta)) + { + return false; + } if (!cBlockHandler::For(TopType).DoesIgnoreBuildCollision(World, a_HeldItem, TopPos, TopMeta, a_ClickedBlockFace, false)) { diff --git a/src/Items/ItemDoor.h b/src/Items/ItemDoor.h index 6538a5bef..f28d3ad36 100644 --- a/src/Items/ItemDoor.h +++ b/src/Items/ItemDoor.h @@ -54,7 +54,10 @@ public: { BLOCKTYPE TopType; NIBBLETYPE TopMeta; - World.GetBlockTypeMeta(UpperBlockPosition, TopType, TopMeta); + if (!World.GetBlockTypeMeta(UpperBlockPosition, TopType, TopMeta)) + { + return false; + } if (!cBlockHandler::For(TopType).DoesIgnoreBuildCollision(World, a_HeldItem, UpperBlockPosition, TopMeta, a_ClickedBlockFace, false)) { diff --git a/src/Items/ItemDye.h b/src/Items/ItemDye.h index 636a1b477..b0f00b9ba 100644 --- a/src/Items/ItemDye.h +++ b/src/Items/ItemDye.h @@ -54,10 +54,12 @@ public: // Cocoa (brown dye) can be planted on jungle logs: BLOCKTYPE BlockType; NIBBLETYPE BlockMeta; - a_World->GetBlockTypeMeta(a_ClickedBlockPos, BlockType, BlockMeta); // Check if the block that the player clicked is a jungle log. - if ((BlockType != E_BLOCK_LOG) || ((BlockMeta & 0x03) != E_META_LOG_JUNGLE)) + if ( + !a_World->GetBlockTypeMeta(a_ClickedBlockPos, BlockType, BlockMeta) || + ((BlockType != E_BLOCK_LOG) || ((BlockMeta & 0x03) != E_META_LOG_JUNGLE)) + ) { return false; } diff --git a/src/Items/ItemEyeOfEnder.h b/src/Items/ItemEyeOfEnder.h index fc6fac336..414d81c39 100644 --- a/src/Items/ItemEyeOfEnder.h +++ b/src/Items/ItemEyeOfEnder.h @@ -35,8 +35,8 @@ public: { BLOCKTYPE FacingBlock; NIBBLETYPE FacingMeta; - a_World->GetBlockTypeMeta(a_ClickedBlockPos, FacingBlock, FacingMeta); - if (FacingBlock == E_BLOCK_END_PORTAL_FRAME) + + if (a_World->GetBlockTypeMeta(a_ClickedBlockPos, FacingBlock, FacingMeta) && (FacingBlock == E_BLOCK_END_PORTAL_FRAME)) { // Fill the portal frame. E_META_END_PORTAL_EYE is the bit for holding the eye of ender. if ((FacingMeta & E_META_END_PORTAL_FRAME_EYE) != E_META_END_PORTAL_FRAME_EYE) diff --git a/src/Items/ItemHoe.h b/src/Items/ItemHoe.h index 2bca5f5ca..503047328 100644 --- a/src/Items/ItemHoe.h +++ b/src/Items/ItemHoe.h @@ -46,7 +46,10 @@ public: // Can only transform dirt or grass blocks: BLOCKTYPE BlockType; NIBBLETYPE BlockMeta; - a_World->GetBlockTypeMeta(a_ClickedBlockPos, BlockType, BlockMeta); + if (!a_World->GetBlockTypeMeta(a_ClickedBlockPos, BlockType, BlockMeta)) + { + return false; + } if ((BlockType != E_BLOCK_DIRT) && (BlockType != E_BLOCK_GRASS)) { return false; diff --git a/src/Items/ItemShears.h b/src/Items/ItemShears.h index 5f8e9e2f1..00504884c 100644 --- a/src/Items/ItemShears.h +++ b/src/Items/ItemShears.h @@ -32,7 +32,10 @@ public: { BLOCKTYPE Block; NIBBLETYPE BlockMeta; - a_World->GetBlockTypeMeta(a_ClickedBlockPos, Block, BlockMeta); + if (!a_World->GetBlockTypeMeta(a_ClickedBlockPos, Block, BlockMeta)) + { + return false; + } if ((Block == E_BLOCK_LEAVES) || (Block == E_BLOCK_NEW_LEAVES)) { diff --git a/src/UI/SlotArea.cpp b/src/UI/SlotArea.cpp index 4085dc816..ef5f7382e 100644 --- a/src/UI/SlotArea.cpp +++ b/src/UI/SlotArea.cpp @@ -1065,9 +1065,12 @@ void cSlotAreaAnvil::OnTakeResult(cPlayer & a_Player) BLOCKTYPE Block; NIBBLETYPE BlockMeta; - a_Player.GetWorld()->GetBlockTypeMeta(BlockPos, Block, BlockMeta); - if (!a_Player.IsGameModeCreative() && (Block == E_BLOCK_ANVIL) && GetRandomProvider().RandBool(0.12)) + if ( + a_Player.GetWorld()->GetBlockTypeMeta(BlockPos, Block, BlockMeta) && + !a_Player.IsGameModeCreative() && (Block == E_BLOCK_ANVIL) && + GetRandomProvider().RandBool(0.12) + ) { NIBBLETYPE Orientation = BlockMeta & 0x3; NIBBLETYPE AnvilDamage = BlockMeta >> 2; diff --git a/src/World.cpp b/src/World.cpp index 2f92020bf..781d44c3a 100644 --- a/src/World.cpp +++ b/src/World.cpp @@ -1539,8 +1539,7 @@ bool cWorld::GetLargeTreeAdjustment(Vector3i & a_BlockPos, NIBBLETYPE a_Meta) { NIBBLETYPE meta; BLOCKTYPE type; - GetBlockTypeMeta(a_BlockPos.addedXZ(x, z), type, meta); - IsLarge = IsLarge && (type == E_BLOCK_SAPLING) && ((meta & 0x07) == a_Meta); + IsLarge = IsLarge && GetBlockTypeMeta(a_BlockPos.addedXZ(x, z), type, meta) && (type == E_BLOCK_SAPLING) && ((meta & 0x07) == a_Meta); } } @@ -1557,8 +1556,7 @@ bool cWorld::GetLargeTreeAdjustment(Vector3i & a_BlockPos, NIBBLETYPE a_Meta) { NIBBLETYPE meta; BLOCKTYPE type; - GetBlockTypeMeta(a_BlockPos.addedXZ(x, z), type, meta); - IsLarge = IsLarge && (type == E_BLOCK_SAPLING) && ((meta & 0x07) == a_Meta); + IsLarge = IsLarge && GetBlockTypeMeta(a_BlockPos.addedXZ(x, z), type, meta) && (type == E_BLOCK_SAPLING) && ((meta & 0x07) == a_Meta); } } @@ -1576,8 +1574,7 @@ bool cWorld::GetLargeTreeAdjustment(Vector3i & a_BlockPos, NIBBLETYPE a_Meta) { NIBBLETYPE meta; BLOCKTYPE type; - GetBlockTypeMeta(a_BlockPos.addedXZ(x, z), type, meta); - IsLarge = IsLarge && (type == E_BLOCK_SAPLING) && ((meta & 0x07) == a_Meta); + IsLarge = IsLarge && GetBlockTypeMeta(a_BlockPos.addedXZ(x, z), type, meta) && (type == E_BLOCK_SAPLING) && ((meta & 0x07) == a_Meta); } } @@ -1596,8 +1593,7 @@ bool cWorld::GetLargeTreeAdjustment(Vector3i & a_BlockPos, NIBBLETYPE a_Meta) { NIBBLETYPE meta; BLOCKTYPE type; - GetBlockTypeMeta(a_BlockPos.addedXZ(x, z), type, meta); - IsLarge = IsLarge && (type == E_BLOCK_SAPLING) && ((meta & 0x07) == a_Meta); + IsLarge = IsLarge && GetBlockTypeMeta(a_BlockPos.addedXZ(x, z), type, meta) && (type == E_BLOCK_SAPLING) && ((meta & 0x07) == a_Meta); } } @@ -2044,7 +2040,10 @@ void cWorld::PlaceBlock(const Vector3i a_Position, const BLOCKTYPE a_BlockType, { BLOCKTYPE BlockType; NIBBLETYPE BlockMeta; - GetBlockTypeMeta(a_Position, BlockType, BlockMeta); + if (!GetBlockTypeMeta(a_Position, BlockType, BlockMeta)) + { + return; + } SetBlock(a_Position, a_BlockType, a_BlockMeta); @@ -2070,7 +2069,10 @@ bool cWorld::DigBlock(Vector3i a_BlockPos, const cEntity * a_Digger) { BLOCKTYPE BlockType; NIBBLETYPE BlockMeta; - GetBlockTypeMeta(a_BlockPos, BlockType, BlockMeta); + if (!GetBlockTypeMeta(a_BlockPos, BlockType, BlockMeta)) + { + return false; + } if (!m_ChunkMap.DigBlock(a_BlockPos)) { @@ -2617,7 +2619,11 @@ bool cWorld::IsTrapdoorOpen(int a_BlockX, int a_BlockY, int a_BlockZ) { BLOCKTYPE Block; NIBBLETYPE Meta; - GetBlockTypeMeta({ a_BlockX, a_BlockY, a_BlockZ }, Block, Meta); + if (!GetBlockTypeMeta({ a_BlockX, a_BlockY, a_BlockZ }, Block, Meta)) + { + return false; + } + if ((Block != E_BLOCK_TRAPDOOR) && (Block != E_BLOCK_IRON_TRAPDOOR)) { return false; @@ -2634,7 +2640,11 @@ bool cWorld::SetTrapdoorOpen(int a_BlockX, int a_BlockY, int a_BlockZ, bool a_Op { BLOCKTYPE Block; NIBBLETYPE Meta; - GetBlockTypeMeta({ a_BlockX, a_BlockY, a_BlockZ }, Block, Meta); + if (!GetBlockTypeMeta({ a_BlockX, a_BlockY, a_BlockZ }, Block, Meta)) + { + return false; + } + if ((Block != E_BLOCK_TRAPDOOR) && (Block != E_BLOCK_IRON_TRAPDOOR)) { return false; diff --git a/tests/CompositeChat/CompositeChatTest.cpp b/tests/CompositeChat/CompositeChatTest.cpp index ca05e79a2..9bc1c31b2 100644 --- a/tests/CompositeChat/CompositeChatTest.cpp +++ b/tests/CompositeChat/CompositeChatTest.cpp @@ -109,6 +109,7 @@ static void TestParser5(void) + static void TestParser6(void) { cCompositeChat Msg; -- cgit v1.2.3