ClearCommand: Cleanup logic & fix vanilla disparities (#4619)

changelogs/4.0.md

@@ -836,7 +836,7 @@ However, if we add `src-namespace-prefix: pmmp\TesterPlugin` to the `plugin.yml`
   - `Item::clearCreativeItems()` -> `CreativeInventory::clear()`
   - `Item::getCreativeItemIndex()` -> `CreativeInventory::getItemIndex()`
   - `Item::getCreativeItems()` -> `CreativeInventory::getAll()`
-  - `Item::initCreativeItems()` -> `CreativeInventory::init()`
+  - `Item::initCreativeItems()` -> `CreativeInventory::reset()`
   - `Item::isCreativeItem()` -> `CreativeInventory::contains()`
   - `Item::removeCreativeItem()` -> `CreativeInventory::remove()`
 - The following classes have been added:

src/command/defaults/ClearCommand.php

@@ -26,6 +26,8 @@ namespace pocketmine\command\defaults;
 use pocketmine\command\Command;
 use pocketmine\command\CommandSender;
 use pocketmine\command\utils\InvalidCommandSyntaxException;
+use pocketmine\inventory\Inventory;
+use pocketmine\item\Item;
 use pocketmine\item\LegacyStringToItemParser;
 use pocketmine\item\LegacyStringToItemParserException;
 use pocketmine\item\StringToItemParser;
@@ -33,9 +35,9 @@ use pocketmine\lang\KnownTranslationFactory;
 use pocketmine\permission\DefaultPermissionNames;
 use pocketmine\player\Player;
 use pocketmine\utils\TextFormat;
-use function array_merge;
 use function count;
 use function implode;
+use function min;
 
 class ClearCommand extends VanillaCommand{
 
@@ -57,7 +59,6 @@ class ClearCommand extends VanillaCommand{
 			throw new InvalidCommandSyntaxException();
 		}
 
-		$target = null;
 		if(isset($args[0])){
 			$target = $sender->getServer()->getPlayerByPrefix($args[0]);
 			if($target === null){
@@ -77,14 +78,14 @@ class ClearCommand extends VanillaCommand{
 			throw new InvalidCommandSyntaxException();
 		}
 
-		$item = null;
+		$targetItem = null;
 		$maxCount = -1;
 		if(isset($args[1])){
 			try{
-				$item = StringToItemParser::getInstance()->parse($args[1]) ?? LegacyStringToItemParser::getInstance()->parse($args[1]);
+				$targetItem = StringToItemParser::getInstance()->parse($args[1]) ?? LegacyStringToItemParser::getInstance()->parse($args[1]);
 
 				if(isset($args[2])){
-					$item->setCount($maxCount = $this->getInteger($sender, $args[2], 0));
+					$targetItem->setCount($maxCount = $this->getInteger($sender, $args[2], -1));
 				}
 			}catch(LegacyStringToItemParserException $e){
 				//vanilla checks this at argument parsing layer, can't come up with a better alternative
@@ -93,14 +94,18 @@ class ClearCommand extends VanillaCommand{
 			}
 		}
 
-		//checking players inventory for all the items matching the criteria
+		/**
-		if($item !== null and $maxCount === 0){
+		 * @var Inventory[] $inventories - This is the order that vanilla would clear items in.
-			$count = 0;
+		 */
-			$contents = array_merge($target->getInventory()->all($item), $target->getArmorInventory()->all($item));
+		$inventories = [
-			foreach($contents as $content){
+			$target->getInventory(),
-				$count += $content->getCount();
+			$target->getCursorInventory(),
-			}
+			$target->getArmorInventory()
+		];
 
+		// Checking player's inventory for all the items matching the criteria
+		if($targetItem !== null and $maxCount === 0){
+			$count = $this->countItems($inventories, $targetItem);
 			if($count > 0){
 				$sender->sendMessage(KnownTranslationFactory::commands_clear_testing($target->getName(), (string) $count));
 			}else{
@@ -110,65 +115,59 @@ class ClearCommand extends VanillaCommand{
 			return true;
 		}
 
-		$cleared = 0;
+		$clearedCount = 0;
-
+		if($targetItem === null){
-		//clear everything from the targets inventory
+			// Clear all items from the inventories
-		if($item === null){
+			$clearedCount += $this->countItems($inventories, null);
-			$contents = array_merge($target->getInventory()->getContents(), $target->getArmorInventory()->getContents());
+			foreach($inventories as $inventory){
-			foreach($contents as $content){
+				$inventory->clearAll();
-				$cleared += $content->getCount();
 			}
-
-			$target->getInventory()->clearAll();
-			$target->getArmorInventory()->clearAll();
-			//TODO: should the cursor inv be cleared?
 		}else{
-			//clear the item from targets inventory irrelevant of the count
+			// Clear the item from target's inventory irrelevant of the count
 			if($maxCount === -1){
-				if(($slot = $target->getArmorInventory()->first($item)) !== -1){
+				$clearedCount += $this->countItems($inventories, $targetItem);
-					$cleared++;
+				foreach($inventories as $inventory){
-					$target->getArmorInventory()->clear($slot);
+					$inventory->remove($targetItem);
-				}
-
-				foreach($target->getInventory()->all($item) as $index => $i){
-					$cleared += $i->getCount();
-					$target->getInventory()->clear($index);
 				}
 			}else{
-				//clear only the given amount of that particular item from targets inventory
+				// Clear the item from target's inventory up to maxCount
-				if(($slot = $target->getArmorInventory()->first($item)) !== -1){
+				foreach($inventories as $inventory){
-					$cleared++;
+					foreach($inventory->all($targetItem) as $index => $item){
-					$maxCount--;
+						// The count to reduce from the item and max count
-					$target->getArmorInventory()->clear($slot);
+						$reductionCount = min($item->getCount(), $maxCount);
-				}
+						$item->pop($reductionCount);
-
+						$clearedCount += $reductionCount;
-				if($maxCount > 0){
+						$inventory->setItem($index, $item);
-					foreach($target->getInventory()->all($item) as $index => $i){
-						if($i->getCount() >= $maxCount){
-							$i->pop($maxCount);
-							$cleared += $maxCount;
-							$target->getInventory()->setItem($index, $i);
-							break;
-						}
 
+						$maxCount -= $reductionCount;
 						if($maxCount <= 0){
-							break;
+							break 2;
 						}
-
-						$cleared += $i->getCount();
-						$maxCount -= $i->getCount();
-						$target->getInventory()->clear($index);
 					}
 				}
 			}
 		}
 
-		if($cleared > 0){
+		if($clearedCount > 0){
-			Command::broadcastCommandMessage($sender, KnownTranslationFactory::commands_clear_success($target->getName(), (string) $cleared));
+			Command::broadcastCommandMessage($sender, KnownTranslationFactory::commands_clear_success($target->getName(), (string) $clearedCount));
 		}else{
 			$sender->sendMessage(KnownTranslationFactory::commands_clear_failure_no_items($target->getName())->prefix(TextFormat::RED));
 		}
 
 		return true;
 	}
-}
+
+	/**
+	 * @param Inventory[] $inventories
+	 */
+	protected function countItems(array $inventories, ?Item $target) : int{
+		$count = 0;
+		foreach($inventories as $inventory){
+			$contents = $target !== null ? $inventory->all($target) : $inventory->getContents();
+			foreach($contents as $item){
+				$count += $item->getCount();
+			}
+		}
+		return $count;
+	}
+}