Introduce Item use results - can be success, fail or none

closes #2693, closes #2705, closes #2734

src/pocketmine/Player.php

@@ -80,6 +80,7 @@ use pocketmine\item\Durable;
 use pocketmine\item\enchantment\EnchantmentInstance;
 use pocketmine\item\enchantment\MeleeWeaponEnchantment;
 use pocketmine\item\Item;
+use pocketmine\item\ItemUseResult;
 use pocketmine\item\WritableBook;
 use pocketmine\item\WrittenBook;
 use pocketmine\lang\TextContainer;
@@ -2061,11 +2062,14 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 			return false;
 		}
 
-		if($item->onClickAir($this, $directionVector)){
+		$result = $item->onClickAir($this, $directionVector);
+		if($result === ItemUseResult::success()){
 			$this->resetItemCooldown($item);
 			if($this->isSurvival()){
 				$this->inventory->setItemInHand($item);
 			}
+		}elseif($result === ItemUseResult::fail()){
+			$this->inventory->sendHeldItem($this);
 		}
 
 		//TODO: check if item has a release action - if it doesn't, this shouldn't be set
@@ -2120,11 +2124,16 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 					$this->inventory->sendContents($this);
 					return false;
 				}
-				if($item->onReleaseUsing($this)){
+				$result = $item->onReleaseUsing($this);
+				if($result === ItemUseResult::success()){
 					$this->resetItemCooldown($item);
 					$this->inventory->setItemInHand($item);
 					return true;
 				}
+				if($result === ItemUseResult::fail()){
+					$this->inventory->sendContents($this);
+					return true;
+				}
 			}
 
 			return false;

src/pocketmine/item/Bow.php

@@ -47,10 +47,9 @@ class Bow extends Tool{
 		return 385;
 	}
 
-	public function onReleaseUsing(Player $player) : bool{
+	public function onReleaseUsing(Player $player) : ItemUseResult{
 		if($player->isSurvival() and !$player->getInventory()->contains(ItemFactory::get(Item::ARROW, 0, 1))){
-			$player->getInventory()->sendContents($player);
+			return ItemUseResult::fail();
-			return false;
 		}
 
 		$nbt = EntityFactory::createBaseNBT(
@@ -93,9 +92,25 @@ class Bow extends Tool{
 
 		if($ev->isCancelled()){
 			$entity->flagForDespawn();
-			$player->getInventory()->sendContents($player);
+			return ItemUseResult::fail();
-		}else{
+		}
+
 		$entity->setMotion($entity->getMotion()->multiply($ev->getForce()));
+
+		if($entity instanceof Projectile){
+			$projectileEv = new ProjectileLaunchEvent($entity);
+			$projectileEv->call();
+			if($projectileEv->isCancelled()){
+				$ev->getProjectile()->flagForDespawn();
+				return ItemUseResult::fail();
+			}
+
+			$ev->getProjectile()->spawnToAll();
+			$player->getLevel()->broadcastLevelSoundEvent($player, LevelSoundEventPacket::SOUND_BOW);
+		}else{
+			$entity->spawnToAll();
+		}
+
 		if($player->isSurvival()){
 			if(!$infinity){ //TODO: tipped arrows are still consumed when Infinity is applied
 				$player->getInventory()->removeItem(ItemFactory::get(Item::ARROW, 0, 1));
@@ -103,20 +118,6 @@ class Bow extends Tool{
 			$this->applyDamage(1);
 		}
 
-			if($entity instanceof Projectile){
+		return ItemUseResult::success();
-				$projectileEv = new ProjectileLaunchEvent($entity);
-				$projectileEv->call();
-				if($projectileEv->isCancelled()){
-					$ev->getProjectile()->flagForDespawn();
-				}else{
-					$ev->getProjectile()->spawnToAll();
-					$player->getLevel()->broadcastLevelSoundEvent($player, LevelSoundEventPacket::SOUND_BOW);
-				}
-			}else{
-				$entity->spawnToAll();
-			}
-		}
-
-		return true;
 	}
 }

src/pocketmine/item/Bucket.php

@@ -36,7 +36,7 @@ class Bucket extends Item{
 		return 16;
 	}
 
-	public function onActivate(Player $player, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector) : bool{
+	public function onActivate(Player $player, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector) : ItemUseResult{
 		//TODO: move this to generic placement logic
 		if($blockClicked instanceof Liquid and $blockClicked->isSource()){
 			$stack = clone $this;
@@ -58,13 +58,12 @@ class Bucket extends Item{
 				}else{
 					$player->getInventory()->addItem($ev->getItem());
 				}
-			}else{
+				return ItemUseResult::success();
-				$player->getInventory()->sendContents($player);
 			}
 
-			return true;
+			return ItemUseResult::fail();
 		}
 
-		return false;
+		return ItemUseResult::none();
 	}
 }

src/pocketmine/item/FlintSteel.php

@@ -35,7 +35,7 @@ class FlintSteel extends Tool{
 		parent::__construct(self::FLINT_STEEL, 0, "Flint and Steel");
 	}
 
-	public function onActivate(Player $player, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector) : bool{
+	public function onActivate(Player $player, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector) : ItemUseResult{
 		if($blockReplace->getId() === self::AIR){
 			$level = $player->getLevel();
 			assert($level !== null);
@@ -44,10 +44,10 @@ class FlintSteel extends Tool{
 
 			$this->applyDamage(1);
 
-			return true;
+			return ItemUseResult::success();
 		}
 
-		return false;
+		return ItemUseResult::none();
 	}
 
 	public function getMaxDurability() : int{

src/pocketmine/item/Item.php

@@ -735,10 +735,10 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @param int     $face
 	 * @param Vector3 $clickVector
 	 *
-	 * @return bool
+	 * @return ItemUseResult
 	 */
-	public function onActivate(Player $player, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector) : bool{
+	public function onActivate(Player $player, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector) : ItemUseResult{
-		return false;
+		return ItemUseResult::none();
 	}
 
 	/**
@@ -748,10 +748,10 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @param Player  $player
 	 * @param Vector3 $directionVector
 	 *
-	 * @return bool
+	 * @return ItemUseResult
 	 */
-	public function onClickAir(Player $player, Vector3 $directionVector) : bool{
+	public function onClickAir(Player $player, Vector3 $directionVector) : ItemUseResult{
-		return false;
+		return ItemUseResult::none();
 	}
 
 	/**
@@ -760,10 +760,10 @@ class Item implements ItemIds, \JsonSerializable{
 	 *
 	 * @param Player $player
 	 *
-	 * @return bool
+	 * @return ItemUseResult
 	 */
-	public function onReleaseUsing(Player $player) : bool{
+	public function onReleaseUsing(Player $player) : ItemUseResult{
-		return false;
+		return ItemUseResult::none();
 	}
 
 	/**

src/pocketmine/item/ItemUseResult.php

@@ -0,0 +1,65 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+*/
+
+declare(strict_types=1);
+
+namespace pocketmine\item;
+
+final class ItemUseResult{
+	/** @var ItemUseResult */
+	private static $NONE;
+	/** @var ItemUseResult */
+	private static $FAILED;
+	/** @var ItemUseResult */
+	private static $SUCCEEDED;
+
+	/**
+	 * No action attempted to take place. Does nothing.
+	 *
+	 * @return ItemUseResult
+	 */
+	public static function none() : ItemUseResult{
+		return self::$NONE ?? (self::$NONE = new self());
+	}
+
+	/**
+	 * An action attempted to take place, but failed due to cancellation. This triggers rollback behaviour for a remote
+	 * player.
+	 *
+	 * @return ItemUseResult
+	 */
+	public static function fail() : ItemUseResult{
+		return self::$FAILED ?? (self::$FAILED = new self());
+	}
+
+	/**
+	 * An action took place successfully. Changes inventory contents if appropriate.
+	 *
+	 * @return ItemUseResult
+	 */
+	public static function success() : ItemUseResult{
+		return self::$SUCCEEDED ?? (self::$SUCCEEDED = new self());
+	}
+
+	private function __construct(){
+		//NOOP
+	}
+}

src/pocketmine/item/LiquidBucket.php

@@ -52,14 +52,14 @@ class LiquidBucket extends Item{
 		return 0;
 	}
 
-	public function onActivate(Player $player, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector) : bool{
+	public function onActivate(Player $player, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector) : ItemUseResult{
 		if(!$blockReplace->canBeReplaced()){
-			return false;
+			return ItemUseResult::none();
 		}
 
 		//TODO: move this to generic placement logic
 		$resultBlock = BlockFactory::get($this->liquidId);
-		if($resultBlock instanceof Liquid){
+		if($resultBlock instanceof Liquid){ //TODO: this should never be false
 			$ev = new PlayerBucketEmptyEvent($player, $blockReplace, $face, $this, ItemFactory::get(Item::BUCKET));
 			$ev->call();
 			if(!$ev->isCancelled()){
@@ -69,13 +69,12 @@ class LiquidBucket extends Item{
 				if($player->isSurvival()){
 					$player->getInventory()->setItemInHand($ev->getItem());
 				}
-			}else{
+				return ItemUseResult::success();
-				$player->getInventory()->sendContents($player);
 			}
 
-			return true;
+			return ItemUseResult::fail();
 		}
 
-		return false;
+		return ItemUseResult::none();
 	}
 }

src/pocketmine/item/PaintingItem.php

@@ -38,9 +38,9 @@ class PaintingItem extends Item{
 		parent::__construct(self::PAINTING, 0, "Painting");
 	}
 
-	public function onActivate(Player $player, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector) : bool{
+	public function onActivate(Player $player, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector) : ItemUseResult{
 		if(Facing::axis($face) === Facing::AXIS_Y){
-			return false;
+			return ItemUseResult::none();
 		}
 
 		$motives = [];

src/pocketmine/item/ProjectileItem.php

@@ -53,7 +53,7 @@ abstract class ProjectileItem extends Item{
 
 	}
 
-	public function onClickAir(Player $player, Vector3 $directionVector) : bool{
+	public function onClickAir(Player $player, Vector3 $directionVector) : ItemUseResult{
 		$nbt = EntityFactory::createBaseNBT($player->add(0, $player->getEyeHeight(), 0), $directionVector, $player->yaw, $player->pitch);
 		$this->addExtraTags($nbt);
 
@@ -68,14 +68,15 @@ abstract class ProjectileItem extends Item{
 		$projectileEv->call();
 		if($projectileEv->isCancelled()){
 			$projectile->flagForDespawn();
-		}else{
+			return ItemUseResult::fail();
+		}
+
 		$projectile->spawnToAll();
 
 		$player->getLevel()->broadcastLevelSoundEvent($player, LevelSoundEventPacket::SOUND_THROW, 0, EntityIds::PLAYER);
-		}
 
 		$this->pop();
 
-		return true;
+		return ItemUseResult::success();
 	}
 }

src/pocketmine/item/SpawnEgg.php

@@ -50,7 +50,7 @@ class SpawnEgg extends Item{
 		$this->entityClass = $entityClass;
 	}
 
-	public function onActivate(Player $player, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector) : bool{
+	public function onActivate(Player $player, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector) : ItemUseResult{
 		$nbt = EntityFactory::createBaseNBT($blockReplace->add(0.5, 0, 0.5), null, lcg_value() * 360, 0);
 
 		if($this->hasCustomName()){
@@ -60,6 +60,7 @@ class SpawnEgg extends Item{
 		$entity = EntityFactory::create($this->entityClass, $player->getLevel(), $nbt);
 		$this->pop();
 		$entity->spawnToAll();
-		return true;
+		//TODO: what if the entity was marked for deletion?
+		return ItemUseResult::success();
 	}
 }

src/pocketmine/level/Level.php

@@ -44,6 +44,7 @@ use pocketmine\event\level\SpawnChangeEvent;
 use pocketmine\event\player\PlayerInteractEvent;
 use pocketmine\item\Item;
 use pocketmine\item\ItemFactory;
+use pocketmine\item\ItemUseResult;
 use pocketmine\level\biome\Biome;
 use pocketmine\level\format\Chunk;
 use pocketmine\level\format\ChunkException;
@@ -1798,8 +1799,11 @@ class Level implements ChunkManager, Metadatable{
 					return true;
 				}
 
-				if(!$player->isSneaking() and $item->onActivate($player, $blockReplace, $blockClicked, $face, $clickVector)){
+				if(!$player->isSneaking()){
-					return true;
+					$result = $item->onActivate($player, $blockReplace, $blockClicked, $face, $clickVector);
+					if($result !== ItemUseResult::none()){
+						return $result === ItemUseResult::success();
+					}
 				}
 			}else{
 				return false;