Many many changes related to inventory transactions, fixed item dropping, fixed creative menu

src/pocketmine/inventory/BaseInventory.php

@@ -140,7 +140,6 @@ abstract class BaseInventory implements Inventory{
 		if($holder instanceof Entity){
 			Server::getInstance()->getPluginManager()->callEvent($ev = new EntityInventoryChangeEvent($holder, $this->getItem($index), $item, $index));
 			if($ev->isCancelled()){
-				$this->sendSlot($index, $this->getViewers());
 				return false;
 			}
 			$item = $ev->getNewItem();

src/pocketmine/inventory/BaseTransaction.php

@@ -1,73 +0,0 @@
-<?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\inventory;
-
-use pocketmine\item\Item;
-
-class BaseTransaction implements Transaction{
-	/** @var Inventory */
-	protected $inventory;
-	/** @var int */
-	protected $slot;
-	/** @var Item */
-	protected $sourceItem;
-	/** @var Item */
-	protected $targetItem;
-	/** @var float */
-	protected $creationTime;
-
-	/**
-	 * @param Inventory $inventory
-	 * @param int       $slot
-	 * @param Item      $sourceItem
-	 * @param Item      $targetItem
-	 */
-	public function __construct(Inventory $inventory, int $slot, Item $sourceItem, Item $targetItem){
-		$this->inventory = $inventory;
-		$this->slot = $slot;
-		$this->sourceItem = clone $sourceItem;
-		$this->targetItem = clone $targetItem;
-		$this->creationTime = microtime(true);
-	}
-
-	public function getCreationTime() : float{
-		return $this->creationTime;
-	}
-
-	public function getInventory() : Inventory{
-		return $this->inventory;
-	}
-
-	public function getSlot() : int{
-		return $this->slot;
-	}
-
-	public function getSourceItem() : Item{
-		return clone $this->sourceItem;
-	}
-
-	public function getTargetItem() : Item{
-		return clone $this->targetItem;
-	}
-}

src/pocketmine/inventory/PlayerInventory.php

@@ -351,6 +351,12 @@ class PlayerInventory extends BaseInventory{
 	}
 
 	public function setItem(int $index, Item $item, bool $send = true) : bool{
+		if($item->isNull()){
+			$item = Item::get(Item::AIR, 0, 0);
+		}else{
+			$item = clone $item;
+		}
+
 		if($index >= $this->getSize()){ //Armor change
 			Server::getInstance()->getPluginManager()->callEvent($ev = new EntityArmorChangeEvent($this->getHolder(), $this->getItem($index), $item, $index));
 			if($ev->isCancelled() and $this->getHolder() instanceof Human){
@@ -367,9 +373,8 @@ class PlayerInventory extends BaseInventory{
 			$item = $ev->getNewItem();
 		}
 
-
 		$old = $this->getItem($index);
-		$this->slots[$index] = clone $item;
+		$this->slots[$index] = $item;
 		$this->onSlotChange($index, $old, $send);
 
 		return true;

src/pocketmine/inventory/transaction/InventoryTransaction.php

@@ -21,9 +21,12 @@
 
 declare(strict_types=1);
 
-namespace pocketmine\inventory;
+namespace pocketmine\inventory\transaction;
 
-interface TransactionGroup{
+use pocketmine\inventory\Inventory;
+use pocketmine\inventory\transaction\action\InventoryAction;
+
+interface InventoryTransaction{
 
 	/**
 	 * @return float
@@ -31,9 +34,9 @@ interface TransactionGroup{
 	public function getCreationTime() : float;
 
 	/**
-	 * @return Transaction[]
+	 * @return InventoryAction[]
 	 */
-	public function getTransactions() : array;
+	public function getActions() : array;
 
 	/**
 	 * @return Inventory[]
@@ -41,9 +44,9 @@ interface TransactionGroup{
 	public function getInventories() : array;
 
 	/**
-	 * @param Transaction $transaction
+	 * @param InventoryAction $action
 	 */
-	public function addTransaction(Transaction $transaction);
+	public function addAction(InventoryAction $action);
 
 	/**
 	 * @return bool

src/pocketmine/inventory/transaction/SimpleInventoryTransaction.php

@@ -21,17 +21,21 @@
 
 declare(strict_types=1);
 
-namespace pocketmine\inventory;
+namespace pocketmine\inventory\transaction;
 
 use pocketmine\event\inventory\InventoryTransactionEvent;
+use pocketmine\inventory\Inventory;
+use pocketmine\inventory\transaction\InventoryTransaction;
+use pocketmine\inventory\transaction\action\InventoryAction;
+use pocketmine\inventory\transaction\action\SlotChangeAction;
 use pocketmine\item\Item;
 use pocketmine\Player;
 use pocketmine\Server;
 
 /**
- * This TransactionGroup only allows doing Transaction between one / two inventories
+ * This InventoryTransaction only allows doing Transaction between one / two inventories
  */
-class SimpleTransactionGroup implements TransactionGroup{
+class SimpleInventoryTransaction implements InventoryTransaction{
 	/** @var float */
 	private $creationTime;
 	protected $hasExecuted = false;
@@ -41,8 +45,8 @@ class SimpleTransactionGroup implements TransactionGroup{
 	/** @var Inventory[] */
 	protected $inventories = [];
 
-	/** @var Transaction[] */
-	protected $transactions = [];
+	/** @var InventoryAction[] */
+	protected $actions = [];
 
 	/**
 	 * @param Player $source
@@ -71,27 +75,22 @@ class SimpleTransactionGroup implements TransactionGroup{
 	}
 
 	/**
-	 * @return Transaction[]
+	 * @return InventoryAction[]
 	 */
-	public function getTransactions() : array{
-		return $this->transactions;
+	public function getActions() : array{
+		return $this->actions;
 	}
 
-	public function addTransaction(Transaction $transaction){
-		if(isset($this->transactions[spl_object_hash($transaction)])){
+	public function addAction(InventoryAction $action){
+		if(isset($this->actions[spl_object_hash($action)])){
 			return;
 		}
-		foreach($this->transactions as $hash => $tx){
-			if($tx->getInventory() === $transaction->getInventory() and $tx->getSlot() === $transaction->getSlot()){
-				if($transaction->getCreationTime() >= $tx->getCreationTime()){
-					unset($this->transactions[$hash]);
-				}else{
-					return;
-				}
-			}
+
+		$this->actions[spl_object_hash($action)] = $action;
+		if($action instanceof SlotChangeAction){
+			$action->setInventoryFrom($this->source);
+			$this->inventories[spl_object_hash($action->getInventory())] = $action->getInventory();
 		}
-		$this->transactions[spl_object_hash($transaction)] = $transaction;
-		$this->inventories[spl_object_hash($transaction->getInventory())] = $transaction->getInventory();
 	}
 
 	/**
@@ -101,17 +100,17 @@ class SimpleTransactionGroup implements TransactionGroup{
 	 * @return bool
 	 */
 	protected function matchItems(array &$needItems, array &$haveItems) : bool{
-		foreach($this->transactions as $key => $ts){
-			if($ts->getTargetItem()->getId() !== Item::AIR){
-				$needItems[] = $ts->getTargetItem();
+		foreach($this->actions as $key => $action){
+			if($action->getTargetItem()->getId() !== Item::AIR){
+				$needItems[] = $action->getTargetItem();
 			}
-			$checkSourceItem = $ts->getInventory()->getItem($ts->getSlot());
-			$sourceItem = $ts->getSourceItem();
-			if(!$checkSourceItem->equals($sourceItem) or $sourceItem->getCount() !== $checkSourceItem->getCount()){
+
+			if(!$action->isValid($this->source)){
 				return false;
 			}
-			if($sourceItem->getId() !== Item::AIR){
-				$haveItems[] = $sourceItem;
+
+			if($action->getSourceItem()->getId() !== Item::AIR){
+				$haveItems[] = $action->getSourceItem();
 			}
 		}
 
@@ -139,7 +138,7 @@ class SimpleTransactionGroup implements TransactionGroup{
 		$haveItems = [];
 		$needItems = [];
 
-		return $this->matchItems($needItems, $haveItems) and count($this->transactions) > 0 and ((count($haveItems) === 0 and count($needItems) === 0) or $this->source->isCreative(true));
+		return $this->matchItems($needItems, $haveItems) and count($this->actions) > 0 and count($haveItems) === 0 and count($needItems) === 0;
 	}
 
 	/**
@@ -152,18 +151,15 @@ class SimpleTransactionGroup implements TransactionGroup{
 
 		Server::getInstance()->getPluginManager()->callEvent($ev = new InventoryTransactionEvent($this));
 		if($ev->isCancelled()){
-			foreach($this->inventories as $inventory){
-				if($inventory instanceof PlayerInventory){
-					$inventory->sendArmorContents($this->getSource());
-				}
-				$inventory->sendContents($this->getSource());
-			}
-
 			return false;
 		}
 
-		foreach($this->transactions as $transaction){
-			$transaction->getInventory()->setItem($transaction->getSlot(), $transaction->getTargetItem());
+		foreach($this->actions as $action){
+			if($action->execute($this->source)){
+				$action->onExecuteSuccess($this->source);
+			}else{
+				$action->onExecuteFail($this->source);
+			}
 		}
 
 		$this->hasExecuted = true;

src/pocketmine/inventory/transaction/action/CreativeInventoryAction.php

@@ -0,0 +1,85 @@
+<?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\inventory\transaction\action;
+
+use pocketmine\item\Item;
+use pocketmine\Player;
+
+class CreativeInventoryAction extends InventoryAction{
+
+	/**
+	 * Player put an item into the creative window to destroy it.
+	 */
+	const TYPE_DELETE_ITEM = 0;
+	/**
+	 * Player took an item from the creative window.
+	 */
+	const TYPE_CREATE_ITEM = 1;
+
+	protected $actionType;
+
+	public function __construct(Item $sourceItem, Item $targetItem, int $actionType){
+		parent::__construct($sourceItem, $targetItem);
+		$this->actionType = $actionType;
+	}
+
+	/**
+	 * Checks that the player is in creative, and (if creating an item) that the item exists in the creative inventory.
+	 *
+	 * @param Player $source
+	 *
+	 * @return bool
+	 */
+	public function isValid(Player $source) : bool{
+		return $source->isCreative(true) and
+			($this->actionType === self::TYPE_DELETE_ITEM or Item::getCreativeItemIndex($this->sourceItem) !== -1);
+	}
+
+	/**
+	 * Returns the type of the action.
+	 */
+	public function getActionType() : int{
+		return $this->actionType;
+	}
+
+	/**
+	 * No need to do anything extra here: this type just provides a place for items to disappear or appear from.
+	 *
+	 * @param Player $source
+	 *
+	 * @return bool
+	 */
+	public function execute(Player $source) : bool{
+		return true;
+	}
+
+	public function onExecuteSuccess(Player $source){
+
+	}
+
+	public function onExecuteFail(Player $source){
+
+	}
+
+}

src/pocketmine/inventory/transaction/action/DropItemAction.php

@@ -21,34 +21,41 @@
 
 declare(strict_types=1);
 
-namespace pocketmine\inventory;
+namespace pocketmine\inventory\transaction\action;
 
-use pocketmine\item\Item;
+use pocketmine\Player;
 
-interface Transaction{
+/**
+ * Represents an action involving dropping an item into the world.
+ */
+class DropItemAction extends InventoryAction{
 
 	/**
-	 * @return Inventory
+	 * Verifies that the source item of a drop-item action must be air. This is not strictly necessary, just a sanity
+	 * check.
+	 *
+	 * @param Player $source
+	 * @return bool
 	 */
-	public function getInventory() : Inventory;
+	public function isValid(Player $source) : bool{
+		return $this->sourceItem->isNull();
+	}
 
 	/**
-	 * @return int
+	 * Drops the target item in front of the player.
+	 *
+	 * @param Player $source
+	 * @return bool
 	 */
-	public function getSlot() : int;
+	public function execute(Player $source) : bool{
+		return $source->dropItem($this->targetItem);
+	}
 
-	/**
-	 * @return Item
-	 */
-	public function getSourceItem() : Item;
+	public function onExecuteSuccess(Player $source){
 
-	/**
-	 * @return Item
-	 */
-	public function getTargetItem() : Item;
+	}
 
-	/**
-	 * @return float
-	 */
-	public function getCreationTime() : float;
+	public function onExecuteFail(Player $source){
+
+	}
 }

src/pocketmine/inventory/transaction/action/InventoryAction.php

@@ -0,0 +1,102 @@
+<?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\inventory\transaction\action;
+
+use pocketmine\item\Item;
+use pocketmine\Player;
+
+/**
+ * Represents an action involving a change that applies in some way to an inventory or other item-source.
+ */
+abstract class InventoryAction{
+
+	/** @var float */
+	private $creationTime;
+	/** @var Item */
+	protected $sourceItem;
+	/** @var Item */
+	protected $targetItem;
+
+	public function __construct(Item $sourceItem, Item $targetItem){
+		$this->sourceItem = $sourceItem;
+		$this->targetItem = $targetItem;
+
+		$this->creationTime = microtime(true);
+	}
+
+	public function getCreationTime() : float{
+		return $this->creationTime;
+	}
+
+	/**
+	 * Returns the item that was present before the action took place.
+	 * @return Item
+	 */
+	public function getSourceItem() : Item{
+		return clone $this->sourceItem;
+	}
+
+	/**
+	 * Returns the item that the action attempted to replace the source item with.
+	 * @return Item
+	 */
+	public function getTargetItem() : Item{
+		return clone $this->targetItem;
+	}
+
+	/**
+	 * Returns whether this action is currently valid. This should perform any necessary sanity checks.
+	 *
+	 * @param Player $source
+	 *
+	 * @return bool
+	 */
+	abstract public function isValid(Player $source) : bool;
+
+	/**
+	 * Performs actions needed to complete the inventory-action server-side. Returns if it was successful. Will return
+	 * false if plugins cancelled events. This will only be called if the transaction which it is part of is considered
+	 * valid.
+	 *
+	 * @param Player $source
+	 *
+	 * @return bool
+	 */
+	abstract public function execute(Player $source) : bool;
+
+	/**
+	 * Performs additional actions when this inventory-action completed successfully.
+	 *
+	 * @param Player $source
+	 */
+	abstract public function onExecuteSuccess(Player $source);
+
+	/**
+	 * Performs additional actions when this inventory-action did not complete successfully.
+	 *
+	 * @param Player $source
+	 */
+	abstract public function onExecuteFail(Player $source);
+
+}

src/pocketmine/inventory/transaction/action/SlotChangeAction.php

@@ -0,0 +1,126 @@
+<?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\inventory\transaction\action;
+
+use pocketmine\inventory\Inventory;
+use pocketmine\item\Item;
+use pocketmine\Player;
+
+/**
+ * Represents an action causing a change in an inventory slot.
+ */
+class SlotChangeAction extends InventoryAction{
+
+	/** @var Inventory|null */
+	protected $inventory;
+	/** @var int */
+	private $inventorySlot;
+	/** @var int */
+	private $containerId;
+
+	/**
+	 * @param Item $sourceItem
+	 * @param Item $targetItem
+	 * @param int  $containerId
+	 * @param int  $inventorySlot
+	 */
+	public function __construct(Item $sourceItem, Item $targetItem, int $containerId, int $inventorySlot){
+		parent::__construct($sourceItem, $targetItem);
+		$this->inventorySlot = $inventorySlot;
+		$this->containerId = $containerId;
+	}
+
+	public function getContainerId() : int{
+		return $this->containerId;
+	}
+
+	/**
+	 * Returns the inventory involved in this action. Will return null if the action has not yet been fully initialized.
+	 *
+	 * @return Inventory|null
+	 */
+	public function getInventory(){
+		return $this->inventory;
+	}
+
+	public function setInventoryFrom(Player $player){
+		$inventory = $player->getWindow($this->containerId);
+		if($inventory === null){
+			throw new \InvalidStateException("Player " . $player->getName() . " has no open container with ID " . $this->containerId);
+		}
+
+		$this->inventory = $inventory;
+	}
+
+	/**
+	 * Returns the slot in the inventory which this action modified.
+	 * @return int
+	 */
+	public function getSlot() : int{
+		return $this->inventorySlot;
+	}
+
+	/**
+	 * Checks if the item in the inventory at the specified slot is the same as this action's source item.
+	 *
+	 * @param Player $source
+	 *
+	 * @return bool
+	 */
+	public function isValid(Player $source) : bool{
+		$check = $this->inventory->getItem($this->inventorySlot);
+		return $check->equals($this->sourceItem) and $check->getCount() === $this->sourceItem->getCount();
+	}
+
+	/**
+	 * Sets the item into the target inventory.
+	 *
+	 * @param Player $source
+	 *
+	 * @return bool
+	 */
+	public function execute(Player $source) : bool{
+		return $this->inventory->setItem($this->inventorySlot, $this->targetItem, false);
+	}
+
+	/**
+	 * Sends slot changes to other viewers of the inventory. This will not send any change back to the source Player.
+	 *
+	 * @param Player $source
+	 */
+	public function onExecuteSuccess(Player $source){
+		$viewers = $this->inventory->getViewers();
+		unset($viewers[spl_object_hash($source)]);
+		$this->inventory->sendSlot($this->inventorySlot, $viewers);
+	}
+
+	/**
+	 * Sends the original slot contents to the source player to revert the action.
+	 *
+	 * @param Player $source
+	 */
+	public function onExecuteFail(Player $source){
+		$this->inventory->sendSlot($this->inventorySlot, $source);
+	}
+}