get rid of the old hack

this dramatically reduces the server workload dealing with spammy packets like right-click interactions trigger.
It also solves the problem of players getting kicked for right-clicking for too long.

src/network/mcpe/NetworkSession.php

@@ -58,6 +58,7 @@ use pocketmine\network\mcpe\protocol\ChunkRadiusUpdatedPacket;
 use pocketmine\network\mcpe\protocol\ClientboundCloseFormPacket;
 use pocketmine\network\mcpe\protocol\ClientboundPacket;
 use pocketmine\network\mcpe\protocol\DisconnectPacket;
+use pocketmine\network\mcpe\protocol\InventoryTransactionPacket;
 use pocketmine\network\mcpe\protocol\ModalFormRequestPacket;
 use pocketmine\network\mcpe\protocol\MovePlayerPacket;
 use pocketmine\network\mcpe\protocol\NetworkChunkPublisherUpdatePacket;
@@ -109,6 +110,7 @@ use pocketmine\promise\PromiseResolver;
 use pocketmine\Server;
 use pocketmine\timings\Timings;
 use pocketmine\utils\AssumptionFailedError;
+use pocketmine\utils\Binary;
 use pocketmine\utils\BinaryDataException;
 use pocketmine\utils\BinaryStream;
 use pocketmine\utils\ObjectSet;
@@ -194,6 +196,17 @@ class NetworkSession{
 	 */
 	private ObjectSet $disposeHooks;
 
+	/**
+	 * @var string[]
+	 * @phpstan-var array<int, string>
+	 */
+	private array $repeatedPacketFilters = [];
+	/**
+	 * @var int[]
+	 * @phpstan-var array<int, int>
+	 */
+	private array $repeatedPacketFilterStats = [];
+
 	public function __construct(
 		private Server $server,
 		private NetworkSessionManager $manager,
@@ -221,6 +234,8 @@ class NetworkSession{
 			$this->onSessionStartSuccess(...)
 		));
 
+		$this->addRepeatedPacketFilter(InventoryTransactionPacket::NETWORK_ID);
+
 		$this->manager->add($this);
 		$this->logger->info($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_network_session_open()));
 	}
@@ -350,6 +365,44 @@ class NetworkSession{
 		}
 	}
 
+	public function addRepeatedPacketFilter(int $packetId) : void{
+		$this->repeatedPacketFilters[$packetId] = "";
+		$this->repeatedPacketFilterStats[$packetId] = 0;
+	}
+
+	public function removeRepeatedPacketFilter(int $packetId) : void{
+		unset($this->repeatedPacketFilters[$packetId]);
+		unset($this->repeatedPacketFilterStats[$packetId]);
+	}
+
+	/**
+	 * Returns the stats for repeated packet filters, indexed by packet ID.
+	 * The value is the number of times a packet was dropped due to being repeated.
+	 *
+	 * @return int[]
+	 * @phpstan-return array<int, int>
+	 */
+	public function getRepeatedPacketFilterStats() : array{
+		return $this->repeatedPacketFilterStats;
+	}
+
+	private function checkRepeatedPacketFilter(string $buffer) : bool{
+		//TODO: would be great if we didn't repeat reading the ID inside PacketPool
+		$dummy = 0;
+		$packetId = Binary::readUnsignedVarInt($buffer, $dummy);
+
+		if(isset($this->repeatedPacketFilters[$packetId])){
+			if($buffer === $this->repeatedPacketFilters[$packetId]){
+				$this->repeatedPacketFilterStats[$packetId]++;
+				return true;
+			}
+
+			$this->repeatedPacketFilters[$packetId] = $buffer;
+		}
+
+		return false;
+	}
+
 	/**
 	 * @throws PacketHandlingException
 	 */
@@ -403,6 +456,10 @@ class NetworkSession{
 			try{
 				$stream = new BinaryStream($decompressed);
 				foreach(PacketBatch::decodeRaw($stream) as $buffer){
+					if($this->checkRepeatedPacketFilter($buffer)){
+						continue;
+					}
+
 					$this->gamePacketLimiter->decrement();
 					$packet = $this->packetPool->getPacket($buffer);
 					if($packet === null){

src/network/mcpe/handler/InGamePacketHandler.php

@@ -116,7 +116,6 @@ use function is_nan;
 use function json_decode;
 use function max;
 use function mb_strlen;
-use function microtime;
 use function sprintf;
 use function str_starts_with;
 use function strlen;
@@ -128,9 +127,6 @@ use const JSON_THROW_ON_ERROR;
 class InGamePacketHandler extends PacketHandler{
 	private const MAX_FORM_RESPONSE_DEPTH = 2; //modal/simple will be 1, custom forms 2 - they will never contain anything other than string|int|float|bool|null
 
-	protected float $lastRightClickTime = 0.0;
-	protected ?UseItemTransactionData $lastRightClickData = null;
-
 	protected ?Vector3 $lastPlayerAuthInputPosition = null;
 	protected ?float $lastPlayerAuthInputYaw = null;
 	protected ?float $lastPlayerAuthInputPitch = null;
@@ -471,27 +467,11 @@ class InGamePacketHandler extends PacketHandler{
 
 		switch($data->getActionType()){
 			case UseItemTransactionData::ACTION_CLICK_BLOCK:
-				//TODO: start hack for client spam bug
-				$clickPos = $data->getClickPosition();
-				$spamBug = ($this->lastRightClickData !== null &&
-					microtime(true) - $this->lastRightClickTime < 0.1 && //100ms
-					$this->lastRightClickData->getPlayerPosition()->distanceSquared($data->getPlayerPosition()) < 0.00001 &&
-					$this->lastRightClickData->getBlockPosition()->equals($data->getBlockPosition()) &&
-					$this->lastRightClickData->getClickPosition()->distanceSquared($clickPos) < 0.00001 //signature spam bug has 0 distance, but allow some error
-				);
-				//get rid of continued spam if the player clicks and holds right-click
-				$this->lastRightClickData = $data;
-				$this->lastRightClickTime = microtime(true);
-				if($spamBug){
-					return true;
-				}
-				//TODO: end hack for client spam bug
-
 				self::validateFacing($data->getFace());
 
 				$blockPos = $data->getBlockPosition();
 				$vBlockPos = new Vector3($blockPos->getX(), $blockPos->getY(), $blockPos->getZ());
-				$this->player->interactBlock($vBlockPos, $data->getFace(), $clickPos);
+				$this->player->interactBlock($vBlockPos, $data->getFace(), $data->getClickPosition());
 				//always sync this in case plugins caused a different result than the client expected
 				//we *could* try to enhance detection of plugin-altered behaviour, but this would require propagating
 				//more information up the stack. For now I think this is good enough.