Fix instant mining of blocks not being recognised, tweak anti-cheat (#4938)

* Tried to fix a small issue...

Ended up rewriting a bunch of god awful, opaque code with no source
and no sense. Who names a function GetPlayerRelativeBlockHardness???
It's gone now. We're safe again.

* Testing anti-cheat.

* Tidy up debug logging.

* Remove empty member declaration.

* Rewrite GetDigSpeed slightly for better readability.

* GetMiningProgressPerTick now returns 1 when instantly mined. Fixed hasily written typo.

* Comment style and typo fixes.

src/BlockInfo.cpp

@@ -1466,22 +1466,22 @@ cBlockInfo::cBlockInfoArray::cBlockInfoArray()
 	Info[E_BLOCK_END_BRICKS                   ].m_Hardness = 0.8f;
 	Info[E_BLOCK_STRUCTURE_VOID               ].m_Hardness = 0.0f;
 	Info[E_BLOCK_OBSERVER                     ].m_Hardness = 3.5f;
-	Info[E_BLOCK_WHITE_SHULKER_BOX            ].m_Hardness = 0.2f;
+	Info[E_BLOCK_WHITE_SHULKER_BOX            ].m_Hardness = 2.0f;
-	Info[E_BLOCK_ORANGE_SHULKER_BOX           ].m_Hardness = 0.2f;
+	Info[E_BLOCK_ORANGE_SHULKER_BOX           ].m_Hardness = 2.0f;
-	Info[E_BLOCK_MAGENTA_SHULKER_BOX          ].m_Hardness = 0.2f;
+	Info[E_BLOCK_MAGENTA_SHULKER_BOX          ].m_Hardness = 2.0f;
-	Info[E_BLOCK_LIGHT_BLUE_SHULKER_BOX       ].m_Hardness = 0.2f;
+	Info[E_BLOCK_LIGHT_BLUE_SHULKER_BOX       ].m_Hardness = 2.0f;
-	Info[E_BLOCK_YELLOW_SHULKER_BOX           ].m_Hardness = 0.2f;
+	Info[E_BLOCK_YELLOW_SHULKER_BOX           ].m_Hardness = 2.0f;
-	Info[E_BLOCK_LIME_SHULKER_BOX             ].m_Hardness = 0.2f;
+	Info[E_BLOCK_LIME_SHULKER_BOX             ].m_Hardness = 2.0f;
-	Info[E_BLOCK_PINK_SHULKER_BOX             ].m_Hardness = 0.2f;
+	Info[E_BLOCK_PINK_SHULKER_BOX             ].m_Hardness = 2.0f;
-	Info[E_BLOCK_GRAY_SHULKER_BOX             ].m_Hardness = 0.2f;
+	Info[E_BLOCK_GRAY_SHULKER_BOX             ].m_Hardness = 2.0f;
-	Info[E_BLOCK_LIGHT_GRAY_SHULKER_BOX       ].m_Hardness = 0.2f;
+	Info[E_BLOCK_LIGHT_GRAY_SHULKER_BOX       ].m_Hardness = 2.0f;
-	Info[E_BLOCK_CYAN_SHULKER_BOX             ].m_Hardness = 0.2f;
+	Info[E_BLOCK_CYAN_SHULKER_BOX             ].m_Hardness = 2.0f;
-	Info[E_BLOCK_PURPLE_SHULKER_BOX           ].m_Hardness = 0.2f;
+	Info[E_BLOCK_PURPLE_SHULKER_BOX           ].m_Hardness = 2.0f;
-	Info[E_BLOCK_BLUE_SHULKER_BOX             ].m_Hardness = 0.2f;
+	Info[E_BLOCK_BLUE_SHULKER_BOX             ].m_Hardness = 2.0f;
-	Info[E_BLOCK_BROWN_SHULKER_BOX            ].m_Hardness = 0.2f;
+	Info[E_BLOCK_BROWN_SHULKER_BOX            ].m_Hardness = 2.0f;
-	Info[E_BLOCK_GREEN_SHULKER_BOX            ].m_Hardness = 0.2f;
+	Info[E_BLOCK_GREEN_SHULKER_BOX            ].m_Hardness = 2.0f;
-	Info[E_BLOCK_RED_SHULKER_BOX              ].m_Hardness = 0.2f;
+	Info[E_BLOCK_RED_SHULKER_BOX              ].m_Hardness = 2.0f;
-	Info[E_BLOCK_BLACK_SHULKER_BOX            ].m_Hardness = 0.2f;
+	Info[E_BLOCK_BLACK_SHULKER_BOX            ].m_Hardness = 2.0f;
 	Info[E_BLOCK_WHITE_GLAZED_TERRACOTTA      ].m_Hardness = 1.4f;
 	Info[E_BLOCK_ORANGE_GLAZED_TERRACOTTA     ].m_Hardness = 1.4f;
 	Info[E_BLOCK_MAGENTA_GLAZED_TERRACOTTA    ].m_Hardness = 1.4f;

src/BlockInfo.h

@@ -25,6 +25,9 @@ public:
 	inline static NIBBLETYPE GetLightValue        (BLOCKTYPE a_Type) { return Get(a_Type).m_LightValue;          }
 	inline static NIBBLETYPE GetSpreadLightFalloff(BLOCKTYPE a_Type) { return Get(a_Type).m_SpreadLightFalloff;  }
 	inline static bool IsTransparent              (BLOCKTYPE a_Type) { return Get(a_Type).m_Transparent;         }
+	/** Warning: IsOneHitDig does not take into account enchantments / status effects / swim state / floating state
+	and therefore may be incorrect. Only use to check if hardness is 0
+	If you want to check if a player would instantly mine a_Block use cPlayer::CanInstantlyMine(a_Block) */
 	inline static bool IsOneHitDig                (BLOCKTYPE a_Type) { return Get(a_Type).m_OneHitDig;           }
 	inline static bool IsPistonBreakable          (BLOCKTYPE a_Type) { return Get(a_Type).m_PistonBreakable;     }
 	inline static bool IsRainBlocker              (BLOCKTYPE a_Type) { return Get(a_Type).m_IsRainBlocker;       }

src/ClientHandle.cpp

@@ -1301,8 +1301,8 @@ void cClientHandle::HandleBlockDigStarted(int a_BlockX, int a_BlockY, int a_Bloc
 	m_LastDigBlockZ = a_BlockZ;
 
 	if (
-		(m_Player->IsGameModeCreative()) ||  // In creative mode, digging is done immediately
+		(m_Player->IsGameModeCreative()) ||     // In creative mode, digging is done immediately
-		cBlockInfo::IsOneHitDig(a_OldBlock)  // One-hit blocks get destroyed immediately, too
+		m_Player->CanInstantlyMine(a_OldBlock)  // Sometimes the player is fast enough to instantly mine
 	)
 	{
 		HandleBlockDigFinished(a_BlockX, a_BlockY, a_BlockZ, a_BlockFace, a_OldBlock, a_OldMeta);
@@ -1366,10 +1366,10 @@ void cClientHandle::HandleBlockDigFinished(int a_BlockX, int a_BlockY, int a_Blo
 		}
 	}
 
-	if (!m_Player->IsGameModeCreative() && !cBlockInfo::IsOneHitDig(a_OldBlock))
+	if (!m_Player->IsGameModeCreative() && !m_Player->CanInstantlyMine(a_OldBlock))
 	{
-		// Fix for very fast tools.
+		m_BreakProgress += m_Player->GetMiningProgressPerTick(a_OldBlock);
-		m_BreakProgress += m_Player->GetPlayerRelativeBlockHardness(a_OldBlock);
+		// Check for very fast tools. Maybe instead of FASTBREAK_PERCENTAGE we should check we are within x multiplied by the progress per tick
 		if (m_BreakProgress < FASTBREAK_PERCENTAGE)
 		{
 			LOGD("Break progress of player %s was less than expected: %f < %f\n", m_Player->GetName().c_str(), m_BreakProgress * 100, FASTBREAK_PERCENTAGE * 100);
@@ -1410,7 +1410,7 @@ void cClientHandle::HandleBlockDigFinished(int a_BlockX, int a_BlockY, int a_Blo
 		World->DigBlock(absPos);
 	}
 
-	// Damage the tool:
+	// Damage the tool, but not for 0 hardness blocks:
 	auto dlAction = cBlockInfo::IsOneHitDig(a_OldBlock) ? cItemHandler::dlaBreakBlockInstant : cItemHandler::dlaBreakBlock;
 	m_Player->UseEquippedItem(dlAction);
 
@@ -2120,7 +2120,7 @@ void cClientHandle::Tick(float a_Dt)
 	if (m_HasStartedDigging)
 	{
 		BLOCKTYPE Block = m_Player->GetWorld()->GetBlock(m_LastDigBlockX, m_LastDigBlockY, m_LastDigBlockZ);
-		m_BreakProgress += m_Player->GetPlayerRelativeBlockHardness(Block);
+		m_BreakProgress += m_Player->GetMiningProgressPerTick(Block);
 	}
 
 	ProcessProtocolInOut();

src/ClientHandle.h

@@ -562,6 +562,8 @@ private:
 	/** Shared pointer to self, so that this instance can keep itself alive when needed. */
 	cClientHandlePtr m_Self;
 
+	/** The fraction between 0 and 1, of how far through mining the currently mined block is.
+	0 for just started, 1 for broken. Used for anti-cheat. */
 	float m_BreakProgress;
 
 	/** Finish logging the user in after authenticating. */

src/Entities/Player.cpp

@@ -3156,61 +3156,109 @@ bool cPlayer::IsInsideWater()
 
 float cPlayer::GetDigSpeed(BLOCKTYPE a_Block)
 {
-	float f = GetEquippedItem().GetHandler()->GetBlockBreakingStrength(a_Block);
+	// Based on: https://minecraft.gamepedia.com/Breaking#Speed
-	if (f > 1.0f)
+
+	// Get the base speed multiplier of the equipped tool for the mined block
+	float MiningSpeed = GetEquippedItem().GetHandler()->GetBlockBreakingStrength(a_Block);
+
+	// If we can harvest the block then we can apply material and enchantment bonuses
+	if (GetEquippedItem().GetHandler()->CanHarvestBlock(a_Block))
 	{
-		unsigned int efficiencyModifier = GetEquippedItem().m_Enchantments.GetLevel(cEnchantments::eEnchantment::enchEfficiency);
+		if (MiningSpeed > 1.0f)  // If the base multiplier for this block is greater than 1, now we can check enchantments
-		if (efficiencyModifier > 0)
 		{
-			f += (efficiencyModifier * efficiencyModifier) + 1;
+			unsigned int EfficiencyModifier = GetEquippedItem().m_Enchantments.GetLevel(cEnchantments::eEnchantment::enchEfficiency);
+			if (EfficiencyModifier > 0)  // If an efficiency enchantment is present, apply formula as on wiki
+			{
+				MiningSpeed += (EfficiencyModifier * EfficiencyModifier) + 1;
+			}
 		}
 	}
+	else  // If we can't harvest the block then no bonuses:
+	{
+		MiningSpeed = 1;
+	}
 
+	// Haste increases speed by 20% per level
 	auto Haste = GetEntityEffect(cEntityEffect::effHaste);
 	if (Haste != nullptr)
 	{
 		int intensity = Haste->GetIntensity() + 1;
-		f *= 1.0f + (intensity * 0.2f);
+		MiningSpeed *= 1.0f + (intensity * 0.2f);
 	}
 
+	// Mining fatigue decreases speed a lot
 	auto MiningFatigue = GetEntityEffect(cEntityEffect::effMiningFatigue);
 	if (MiningFatigue != nullptr)
 	{
 		int intensity = MiningFatigue->GetIntensity();
 		switch (intensity)
 		{
-			case 0:  f *= 0.3f;     break;
+			case 0:  MiningSpeed *= 0.3f;     break;
-			case 1:  f *= 0.09f;    break;
+			case 1:  MiningSpeed *= 0.09f;    break;
-			case 2:  f *= 0.0027f;  break;
+			case 2:  MiningSpeed *= 0.0027f;  break;
-			default: f *= 0.00081f; break;
+			default: MiningSpeed *= 0.00081f; break;
 
 		}
 	}
 
+	// 5x speed loss for being in water
 	if (IsInsideWater() && !(GetEquippedItem().m_Enchantments.GetLevel(cEnchantments::eEnchantment::enchAquaAffinity) > 0))
 	{
-		f /= 5.0f;
+		MiningSpeed /= 5.0f;
 	}
 
+	// 5x speed loss for not touching ground
 	if (!IsOnGround())
 	{
-		f /= 5.0f;
+		MiningSpeed /= 5.0f;
 	}
 
-	return f;
+	return MiningSpeed;
 }
 
 
 
 
 
-float cPlayer::GetPlayerRelativeBlockHardness(BLOCKTYPE a_Block)
+float cPlayer::GetMiningProgressPerTick(BLOCKTYPE a_Block)
 {
-	float blockHardness = cBlockInfo::GetHardness(a_Block);
+	// Based on https://minecraft.gamepedia.com/Breaking#Calculation
-	float digSpeed = GetDigSpeed(a_Block);
+	// If we know it's instantly breakable then quit here:
-	float canHarvestBlockDivisor = GetEquippedItem().GetHandler()->CanHarvestBlock(a_Block) ? 30.0f : 100.0f;
+	if (cBlockInfo::IsOneHitDig(a_Block))
-	// LOGD("blockHardness: %f, digSpeed: %f, canHarvestBlockDivisor: %f\n", blockHardness, digSpeed, canHarvestBlockDivisor);
+	{
-	return (blockHardness < 0) ? 0 : ((digSpeed / blockHardness) / canHarvestBlockDivisor);
+		return 1;
+	}
+	float BlockHardness = cBlockInfo::GetHardness(a_Block);
+	ASSERT(BlockHardness > 0);  // Can't divide by 0 or less, IsOneHitDig should have returned true
+	if (GetEquippedItem().GetHandler()->CanHarvestBlock(a_Block))
+	{
+		BlockHardness*=1.5;
+	}
+	else
+	{
+		BlockHardness*=5;
+	}
+	float DigSpeed = GetDigSpeed(a_Block);
+	// LOGD("Time to mine block = %f", BlockHardness/DigSpeed);
+	// Number of ticks to mine = (20 * BlockHardness)/DigSpeed;
+	// Therefore take inverse to get fraction mined per tick:
+	return DigSpeed / (20 * BlockHardness);
+}
+
+
+
+
+
+bool cPlayer::CanInstantlyMine(BLOCKTYPE a_Block)
+{
+	// Based on: https://minecraft.gamepedia.com/Breaking#Calculation
+	// Check it has non-zero hardness
+	if (cBlockInfo::IsOneHitDig(a_Block))
+	{
+		return true;
+	}
+	// If the dig speed is greater than 30 times the hardness, then the wiki says we can instantly mine
+	return GetDigSpeed(a_Block) > 30 * cBlockInfo::GetHardness(a_Block);
 }
 
 

src/Entities/Player.h

@@ -597,11 +597,17 @@ public:
 	The player removes its m_ClientHandle ownership so that the ClientHandle gets deleted. */
 	void RemoveClientHandle(void);
 
-	/** Returns the relative block hardness for the block a_Block.
+	/** Returns the progress mined per tick for the block a_Block as a fraction
-	The bigger it is the faster the player can break the block.
+	(1 would be completely mined)
-	Returns zero if the block is instant breakable.
+	Depends on hardness values so check those are correct.
-	Otherwise it returns the dig speed (float GetDigSpeed(BLOCKTYPE a_Block)) divided by the block hardness (cBlockInfo::GetHardness(BLOCKTYPE a_Block)) divided by 30 if the player can harvest the block and divided by 100 if he can't. */
+	Source: https://minecraft.gamepedia.com/Breaking#Calculation */
-	float GetPlayerRelativeBlockHardness(BLOCKTYPE a_Block);
+	float GetMiningProgressPerTick(BLOCKTYPE a_Block);
+
+	/** Given tool, enchantments, status effects, and world position
+	returns whether a_Block would be instantly mined.
+	Depends on hardness values so check those are correct.
+	Source: https://minecraft.gamepedia.com/Breaking#Instant_breaking */
+	bool CanInstantlyMine(BLOCKTYPE a_Block);
 
 	/** get player explosion exposure rate */
 	virtual float GetExplosionExposureRate(Vector3d a_ExplosionPosition, float a_ExlosionPower) override;
@@ -805,8 +811,9 @@ private:
 	Returns one if using hand.
 	If the player is using a tool that is good to break the block the value is higher.
 	If he has an enchanted tool with efficiency or he has a haste or mining fatique effect it gets multiplied by a specific factor depending on the strength of the effect or enchantment.
-	In he is in water it gets divided by 5 except his tool is enchanted with aqa affinity.
+	In he is in water it gets divided by 5 except if his tool is enchanted with aqua affinity.
-	If he is not on ground it also gets divided by 5. */
+	If he is not on ground it also gets divided by 5.
+	Source: https://minecraft.gamepedia.com/Breaking#Calculation */
 	float GetDigSpeed(BLOCKTYPE a_Block);
 
 	/** Add the recipe Id to the known recipes.