ClientHandle: improve right-click robustness (#5372)

* ClientHandle: improve right-click robustness

+ Add checks for result of GetBlockTypeMeta
+ Kick if the client sent an invalid block face or coordinate
* Update outdated comments
This commit is contained in:
Tiger Wang 2022-01-02 12:19:13 +00:00 committed by GitHub
parent c52f299e72
commit 178d1d2bda
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 57 additions and 64 deletions

View File

@ -1437,38 +1437,45 @@ void cClientHandle::FinishDigAnimation()
void cClientHandle::HandleRightClick(int a_BlockX, int a_BlockY, int a_BlockZ, eBlockFace a_BlockFace, int a_CursorX, int a_CursorY, int a_CursorZ, bool a_UsedMainHand)
{
// This function handles three actions:
// (1) Place a block;
// (2) "Use" a block: Interactive with the block, like opening a chest/crafting table/furnace;
// (3) Use the held item targeting a block. E.g. farming.
//
// Sneaking player will not use the block if hand is not empty.
// Frozen player can do nothing.
// In Game Mode Spectator, player cannot use item or place block, but can interactive with some block depending on cBlockInfo::IsUseableBySpectator(BlockType)
//
// If the action failed, we need to send an update of the placed block or inventory to the client.
//
// Actions rejected by plugin will not lead to other attempts.
// E.g., when opening a chest with a dirt in hand, if the plugin rejects opening the chest, the dirt will not be placed.
/* This function handles three actions:
(1) Place a block;
(2) "Use" a block: Interactive with the block, like opening a chest/crafting table/furnace;
(3) Use the held item targeting a block. E.g. farming.
Sneaking player will not use the block if hand is not empty.
Frozen player can do nothing.
In Game Mode Spectator, player cannot use item or place block, but can interactive with some block depending on cBlockInfo::IsUseableBySpectator(BlockType)
If the action failed, we need to send an update of the placed block or inventory to the client.
Actions rejected by plugin will not lead to other attempts.
E.g., when opening a chest with a dirt in hand, if the plugin rejects opening the chest, the dirt will not be placed. */
if ((a_BlockFace == BLOCK_FACE_NONE) || !cChunkDef::IsValidHeight(a_BlockY))
{
LOGD("Player \"%s\" sent an invalid click - hacked client?", m_Username.c_str());
return;
}
// TODO: We are still consuming the items in main hand. Remove this override when the off-hand consumption is handled correctly.
a_UsedMainHand = true;
const cItem & HeldItem = a_UsedMainHand ? m_Player->GetEquippedItem() : m_Player->GetInventory().GetShieldSlot();
auto & ItemHandler = HeldItem.GetHandler();
// TODO: This distance should be calculated from the point that the cursor pointing at, instead of the center of the block
// Distance from the block's center to the player's eye height
auto ClickedBlockPos = Vector3i(a_BlockX, a_BlockY, a_BlockZ);
auto CursorPos = Vector3i(a_CursorX, a_CursorY, a_CursorZ);
double Dist = (Vector3d(ClickedBlockPos) + Vector3d(0.5, 0.5, 0.5) - m_Player->GetEyePosition()).Length();
FLOGD("HandleRightClick: {0}, face {1}, Cursor {2}, Hand: {3}, HeldItem: {4}; Dist: {5:.02f}",
ClickedBlockPos, a_BlockFace, CursorPos, a_UsedMainHand, ItemToFullString(HeldItem), Dist
);
const Vector3i ClickedPosition(a_BlockX, a_BlockY, a_BlockZ);
const Vector3i CursorPosition(a_CursorX, a_CursorY, a_CursorZ);
const cItem & HeldItem = a_UsedMainHand ? m_Player->GetEquippedItem() : m_Player->GetInventory().GetShieldSlot();
// Distance from the block's center to the player's eye height.
const double Dist = (Vector3d(0.5, 0.5, 0.5) + ClickedPosition - m_Player->GetEyePosition()).SqrLength();
// Check the reach distance:
// _X 2014-11-25: I've maxed at 5.26 with a Survival client and 5.78 with a Creative client in my tests
double MaxDist = m_Player->IsGameModeCreative() ? 5.78 : 5.26;
double MaxDist = m_Player->IsGameModeCreative() ? 33.4084 : 27.6676;
bool IsWithinReach = (Dist <= MaxDist);
FLOGD("HandleRightClick: {0}, face {1}, Cursor {2}, Hand: {3}, HeldItem: {4}; Dist: {5:.02f}",
ClickedPosition, a_BlockFace, CursorPosition, a_UsedMainHand, ItemToFullString(HeldItem), Dist
);
cWorld * World = m_Player->GetWorld();
cPluginManager * PlgMgr = cRoot::Get()->GetPluginManager();
@ -1479,11 +1486,16 @@ void cClientHandle::HandleRightClick(int a_BlockX, int a_BlockY, int a_BlockZ, e
{
BLOCKTYPE BlockType;
NIBBLETYPE BlockMeta;
World->GetBlockTypeMeta(ClickedBlockPos, BlockType, BlockMeta);
const auto & BlockHandler = cBlockHandler::For(BlockType);
bool Placeable = ItemHandler.IsPlaceable() && !m_Player->IsGameModeAdventure() && !m_Player->IsGameModeSpectator();
bool BlockUsable = BlockHandler.IsUseable() && (!m_Player->IsGameModeSpectator() || cBlockInfo::IsUseableBySpectator(BlockType));
if (!World->GetBlockTypeMeta(ClickedPosition, BlockType, BlockMeta))
{
return;
}
const auto & ItemHandler = HeldItem.GetHandler();
const auto & BlockHandler = cBlockHandler::For(BlockType);
const bool Placeable = ItemHandler.IsPlaceable() && !m_Player->IsGameModeAdventure() && !m_Player->IsGameModeSpectator();
const bool BlockUsable = BlockHandler.IsUseable() && (!m_Player->IsGameModeSpectator() || cBlockInfo::IsUseableBySpectator(BlockType));
if (BlockUsable && !(m_Player->IsCrouched() && !HeldItem.IsEmpty()))
{
@ -1491,7 +1503,7 @@ void cClientHandle::HandleRightClick(int a_BlockX, int a_BlockY, int a_BlockZ, e
if (!PlgMgr->CallHookPlayerUsingBlock(*m_Player, a_BlockX, a_BlockY, a_BlockZ, a_BlockFace, a_CursorX, a_CursorY, a_CursorZ, BlockType, BlockMeta))
{
// Use a block:
if (BlockHandler.OnUse(ChunkInterface, *World, *m_Player, ClickedBlockPos, a_BlockFace, CursorPos))
if (BlockHandler.OnUse(ChunkInterface, *World, *m_Player, ClickedPosition, a_BlockFace, CursorPosition))
{
PlgMgr->CallHookPlayerUsedBlock(*m_Player, a_BlockX, a_BlockY, a_BlockZ, a_BlockFace, a_CursorX, a_CursorY, a_CursorZ, BlockType, BlockMeta);
return; // Block use was successful, we're done.
@ -1501,7 +1513,7 @@ void cClientHandle::HandleRightClick(int a_BlockX, int a_BlockY, int a_BlockZ, e
if (Placeable)
{
// Place a block:
ItemHandler.OnPlayerPlace(*m_Player, HeldItem, {a_BlockX, a_BlockY, a_BlockZ}, a_BlockFace, {a_CursorX, a_CursorY, a_CursorZ});
ItemHandler.OnPlayerPlace(*m_Player, HeldItem, ClickedPosition, BlockType, BlockMeta, a_BlockFace, CursorPosition);
}
return;
@ -1518,7 +1530,7 @@ void cClientHandle::HandleRightClick(int a_BlockX, int a_BlockY, int a_BlockZ, e
}
// Place a block:
ItemHandler.OnPlayerPlace(*m_Player, HeldItem, {a_BlockX, a_BlockY, a_BlockZ}, a_BlockFace, {a_CursorX, a_CursorY, a_CursorZ});
ItemHandler.OnPlayerPlace(*m_Player, HeldItem, ClickedPosition, BlockType, BlockMeta, a_BlockFace, CursorPosition);
return;
}
else if (!PlgMgr->CallHookPlayerUsingItem(*m_Player, a_BlockX, a_BlockY, a_BlockZ, a_BlockFace, a_CursorX, a_CursorY, a_CursorZ))
@ -1527,7 +1539,7 @@ void cClientHandle::HandleRightClick(int a_BlockX, int a_BlockY, int a_BlockZ, e
// Use an item in hand with a target block.
cBlockInServerPluginInterface PluginInterface(*World);
ItemHandler.OnItemUse(World, m_Player, PluginInterface, HeldItem, {a_BlockX, a_BlockY, a_BlockZ}, a_BlockFace);
ItemHandler.OnItemUse(World, m_Player, PluginInterface, HeldItem, ClickedPosition, a_BlockFace);
PlgMgr->CallHookPlayerUsedItem(*m_Player, a_BlockX, a_BlockY, a_BlockZ, a_BlockFace, a_CursorX, a_CursorY, a_CursorZ);
return;
}

View File

@ -1042,30 +1042,15 @@ const cItemHandler & cItemHandler::For(int a_ItemType)
void cItemHandler::OnPlayerPlace(cPlayer & a_Player, const cItem & a_HeldItem, const Vector3i a_ClickedBlockPosition, const eBlockFace a_ClickedBlockFace, const Vector3i a_CursorPosition) const
void cItemHandler::OnPlayerPlace(cPlayer & a_Player, const cItem & a_HeldItem, const Vector3i a_ClickedPosition, const BLOCKTYPE a_ClickedBlockType, const NIBBLETYPE a_ClickedBlockMeta, const eBlockFace a_ClickedBlockFace, const Vector3i a_CursorPosition) const
{
if (a_ClickedBlockFace == BLOCK_FACE_NONE)
{
// Clicked in the air, no placement possible
return;
}
if (!cChunkDef::IsValidHeight(a_ClickedBlockPosition.y))
{
// The clicked block is outside the world, ignore this call altogether (GH #128):
return;
}
const auto & World = *a_Player.GetWorld();
BLOCKTYPE ClickedBlockType;
NIBBLETYPE ClickedBlockMeta;
World.GetBlockTypeMeta(a_ClickedBlockPosition, ClickedBlockType, ClickedBlockMeta);
// Check if the block ignores build collision (water, grass etc.):
if (cBlockHandler::For(ClickedBlockType).DoesIgnoreBuildCollision(World, a_HeldItem, a_ClickedBlockPosition, ClickedBlockMeta, a_ClickedBlockFace, true))
if (cBlockHandler::For(a_ClickedBlockType).DoesIgnoreBuildCollision(World, a_HeldItem, a_ClickedPosition, a_ClickedBlockMeta, a_ClickedBlockFace, true))
{
// Try to place the block at the clicked position:
if (!CommitPlacement(a_Player, a_HeldItem, a_ClickedBlockPosition, a_ClickedBlockFace, a_CursorPosition))
if (!CommitPlacement(a_Player, a_HeldItem, a_ClickedPosition, a_ClickedBlockFace, a_CursorPosition))
{
// The placement failed, the blocks have already been re-sent, re-send inventory:
a_Player.GetInventory().SendEquippedSlot();
@ -1074,21 +1059,19 @@ void cItemHandler::OnPlayerPlace(cPlayer & a_Player, const cItem & a_HeldItem, c
}
else
{
const auto PlacedPosition = AddFaceDirection(a_ClickedBlockPosition, a_ClickedBlockFace);
BLOCKTYPE PlaceBlock;
NIBBLETYPE PlaceMeta;
const auto PlacePosition = AddFaceDirection(a_ClickedPosition, a_ClickedBlockFace);
if (!cChunkDef::IsValidHeight(PlacedPosition.y))
if (!cChunkDef::IsValidHeight(PlacePosition.y) || !World.GetBlockTypeMeta(PlacePosition, PlaceBlock, PlaceMeta))
{
// The block is being placed outside the world, ignore this packet altogether (GH #128):
return;
}
NIBBLETYPE PlaceMeta;
BLOCKTYPE PlaceBlock;
World.GetBlockTypeMeta(PlacedPosition, PlaceBlock, PlaceMeta);
// Clicked on side of block, make sure that placement won't be cancelled if there is a slab able to be double slabbed.
// No need to do combinability (dblslab) checks, client will do that here.
if (!cBlockHandler::For(PlaceBlock).DoesIgnoreBuildCollision(World, a_HeldItem, PlacedPosition, PlaceMeta, a_ClickedBlockFace, false))
if (!cBlockHandler::For(PlaceBlock).DoesIgnoreBuildCollision(World, a_HeldItem, PlacePosition, PlaceMeta, a_ClickedBlockFace, false))
{
// Tried to place a block into another?
// Happens when you place a block aiming at side of block with a torch on it or stem beside it.
@ -1098,7 +1081,7 @@ void cItemHandler::OnPlayerPlace(cPlayer & a_Player, const cItem & a_HeldItem, c
}
// Try to place the block:
if (!CommitPlacement(a_Player, a_HeldItem, PlacedPosition, a_ClickedBlockFace, a_CursorPosition))
if (!CommitPlacement(a_Player, a_HeldItem, PlacePosition, a_ClickedBlockFace, a_CursorPosition))
{
// The placement failed, the blocks have already been re-sent, re-send inventory:
a_Player.GetInventory().SendEquippedSlot();

View File

@ -40,13 +40,11 @@ public:
/** Called when the player tries to place the item (right mouse button, IsPlaceable() == true).
a_ClickedBlockPos is the (neighbor) block that has been clicked to place this item.
a_ClickedBlockFace is the face of the neighbor that has been clicked to place this item.
a_CursorPos is the position of the player's cursor within a_ClickedBlockFace.
The default handler uses GetBlocksToPlace() and places the returned blocks.
Override if the item needs advanced processing, such as spawning a mob based on the blocks being placed.
a_ClickedPosition is the block that has been clicked to place this item.
a_ClickedBlockFace is the face has been clicked to place this item.
a_CursorPosition is the position of the player's cursor within a_ClickedBlockFace.
If the block placement is refused inside this call, it will automatically revert the client-side changes. */
void OnPlayerPlace(cPlayer & a_Player, const cItem & a_HeldItem, Vector3i a_ClickedBlockPosition, eBlockFace a_ClickedBlockFace, Vector3i a_CursorPosition) const;
void OnPlayerPlace(cPlayer & a_Player, const cItem & a_HeldItem, Vector3i a_ClickedPosition, BLOCKTYPE a_ClickedBlockType, NIBBLETYPE a_ClickedBlockMeta, eBlockFace a_ClickedBlockFace, Vector3i a_CursorPosition) const;
/** Called when the player tries to use the item (right mouse button).
Descendants can return false to abort the usage (default behavior). */