Fixed bug preventing interaction with no item in-hand

 * Cleaned up ItemInstance null handling

Author: BrentDaMage

source/client/app/Minecraft.cpp

@@ -350,13 +350,7 @@ void Minecraft::handleBuildAction(const BuildActionIntention& action)
 		else if (action.isPlace() && canInteract)
 		{
 			ItemInstance* pItem = getSelectedItem();
-			if (pItem &&
-				m_pGameMode->useItemOn(
-					player,
-					m_pLevel,
-					pItem->m_itemID <= 0 ? nullptr : pItem,
-					m_hitResult.m_tilePos,
-					m_hitResult.m_hitSide))
+			if (m_pGameMode->useItemOn(player, m_pLevel, pItem, m_hitResult.m_tilePos, m_hitResult.m_hitSide))
 			{
 				bInteract = false;
 
@@ -366,7 +360,7 @@ void Minecraft::handleBuildAction(const BuildActionIntention& action)
 
 				if (isOnline())
 				{
-					if (pItem->m_itemID > C_MAX_TILES || pItem->m_itemID < 0)
+					if (ItemInstance::isNull(pItem) || pItem->m_itemID > C_MAX_TILES)
 						return;
 
 					TilePos tp(m_hitResult.m_tilePos);
@@ -480,7 +474,7 @@ void Minecraft::tickInput()
 			else if (getOptions()->isKey(KM_DROP, keyCode))
 			{
 				ItemInstance *item = m_pLocalPlayer->m_pInventory->getSelected();
-				if (item != nullptr)
+				if (!ItemInstance::isNull(item))
 				{
 					ItemInstance itemDrop = m_pLocalPlayer->isSurvival() ? item->remove(1) : ItemInstance(*item);
 					itemDrop.m_count = 1;
@@ -1181,22 +1175,19 @@ ItemInstance* Minecraft::getSelectedItem()
 {
 	ItemInstance* pInst = m_pLocalPlayer->getSelectedItem();
 
-	if (!pInst)
+	if (ItemInstance::isNull(pInst))
 		return nullptr;
 
-	if (m_pGameMode->isSurvivalType())
-		return pInst;
-
-    // Create new "unlimited" ItemInstance for Creative mode
-    
-	if (pInst->isNull())
-		return nullptr;
-
-	m_CurrItemInstance.m_itemID = pInst->m_itemID;
-	m_CurrItemInstance.m_count = 999;
-	m_CurrItemInstance.setAuxValue(pInst->getAuxValue());
+	if (m_pGameMode->isCreativeType())
+	{
+		// Create new "unlimited" ItemInstance for Creative mode
+		m_CurrItemInstance.m_itemID = pInst->m_itemID;
+		m_CurrItemInstance.m_count = 999;
+		m_CurrItemInstance.setAuxValue(pInst->getAuxValue());
+		return &m_CurrItemInstance;
+	}
 
-	return &m_CurrItemInstance;
+	return pInst;
 }
 
 int Minecraft::getFpsIntlCounter()

source/client/gui/Gui.cpp

@@ -417,7 +417,7 @@ void Gui::renderSlot(int slot, int x, int y, float f)
 	Inventory* pInv = m_pMinecraft->m_pLocalPlayer->m_pInventory;
 
 	ItemInstance* pInst = pInv->getQuickSlotItem(slot);
-	if (pInst == nullptr || pInst->m_itemID <= 0)
+	if (ItemInstance::isNull(pInst))
 		return;
 
     float var6 = ((float)pInst->m_popTime) - f;
@@ -442,10 +442,7 @@ void Gui::renderSlotOverlay(int slot, int x, int y, float f)
 	Inventory* pInv = m_pMinecraft->m_pLocalPlayer->m_pInventory;
 
 	ItemInstance* pInst = pInv->getQuickSlotItem(slot);
-	if (!pInst)
-		return;
-
-	if (!pInst->m_itemID)
+	if (ItemInstance::isNull(pInst))
 		return;
 
 	ItemRenderer::renderGuiItemOverlay(m_pMinecraft->m_pFont, m_pMinecraft->m_pTextures, pInst, x, y);

source/client/gui/screens/IngameBlockSelectionScreen.cpp

@@ -99,10 +99,7 @@ void IngameBlockSelectionScreen::init()
 void IngameBlockSelectionScreen::renderSlot(int index, int x, int y, float f)
 {
 	ItemInstance* pItem = getInventory()->getItem(index);
-	if (!pItem)
-		return;
-
-	if (!pItem->m_itemID)
+	if (ItemInstance::isNull(pItem))
 		return;
 
 	ItemRenderer::renderGuiItem(m_pMinecraft->m_pFont, m_pMinecraft->m_pTextures, pItem, x, y, true);

source/client/player/input/MouseBuildInput.cpp

@@ -9,6 +9,13 @@ MouseBuildInput::MouseBuildInput()
         m_lastButtonStates[i] = false;
 }
 
+void MouseBuildInput::_updateLastButtonStates()
+{
+    m_lastButtonStates[BUTTON_LEFT]   = Mouse::isButtonDown(BUTTON_LEFT);
+    m_lastButtonStates[BUTTON_RIGHT]  = Mouse::isButtonDown(BUTTON_RIGHT);
+    m_lastButtonStates[BUTTON_MIDDLE] = Mouse::isButtonDown(BUTTON_MIDDLE);
+}
+
 bool MouseBuildInput::tickBuild(Player* player, BuildActionIntention* buildActionIntention)
 {
     bool wroteIntention = false;
@@ -61,10 +68,7 @@ bool MouseBuildInput::tickBuild(Player* player, BuildActionIntention* buildActio
         *buildActionIntention = BuildActionIntention(intent);
     }
 
-    // Log last button states
-    m_lastButtonStates[BUTTON_LEFT] = Mouse::isButtonDown(BUTTON_LEFT);
-    m_lastButtonStates[BUTTON_RIGHT] = Mouse::isButtonDown(BUTTON_RIGHT);
-    m_lastButtonStates[BUTTON_MIDDLE] = Mouse::isButtonDown(BUTTON_MIDDLE);
+    _updateLastButtonStates();
 
     return wroteIntention;
 }

source/client/player/input/MouseBuildInput.hpp

@@ -12,6 +12,10 @@ class MouseBuildInput : public IBuildInput
 public:
 	MouseBuildInput();
 
+private:
+	void _updateLastButtonStates();
+
+public:
 	virtual bool tickBuild(Player* player, BuildActionIntention* buildActionIntention) override;
 };
 

source/client/renderer/ItemInHandRenderer.cpp

@@ -41,7 +41,7 @@ void ItemInHandRenderer::itemUsed()
 void ItemInHandRenderer::renderItem(ItemInstance* inst)
 {
 #ifndef ORIGINAL_CODE
-    if (inst->isNull())
+    if (ItemInstance::isNull(inst))
         return;
 #endif
 
@@ -197,7 +197,7 @@ void ItemInHandRenderer::render(float f)
     float fAnim = pLP->getAttackAnim(f);
     constexpr float d = 0.8f;
 
-	if (!pItem->isNull())
+	if (!ItemInstance::isNull(pItem))
 	{
         glTranslatef(-0.4f * Mth::sin(float(M_PI) * Mth::sqrt(fAnim)), 0.2f * Mth::sin(2.0f * float(M_PI) * Mth::sqrt(fAnim)), -0.2f * Mth::sin(float(M_PI) * fAnim));
         glTranslatef(0.7f * d, -0.65f * d - (1.0f - h) * 0.6f, -0.9f * d);

source/world/entity/Player.cpp

@@ -250,7 +250,7 @@ void Player::displayClientMessage(const std::string& msg)
 
 void Player::drop(const ItemInstance* pItemInstance, bool b)
 {
-	if (pItemInstance->isNull())
+	if (ItemInstance::isNull(pItemInstance))
 		return;
 
 	ItemEntity* pItemEntity = new ItemEntity(m_pLevel, Vec3(m_pos.x, m_pos.y - 0.3f + getHeadHeight(), m_pos.z), pItemInstance);

source/world/entity/Player.hpp

@@ -72,7 +72,7 @@ class Player : public Mob
 	bool isSurvival() const { return getPlayerGameType() == GAME_TYPE_SURVIVAL; }
 	bool isCreative() const { return getPlayerGameType() == GAME_TYPE_CREATIVE; }
 	ItemInstance* getSelectedItem() const;
-	bool isUsingItem() const { return false && !getSelectedItem()->isNull(); }
+	bool isUsingItem() const { return false /* && ItemInstance::isNull(getSelectedItem())*/; }
 
 	// QUIRK: Yes, I did mean it like that, as did Mojang.
 #pragma GCC diagnostic push

source/world/item/Inventory.cpp

@@ -226,7 +226,7 @@ int Inventory::getQuickSlotItemId(int slotNo)
 
 	int idx = m_hotbar[slotNo];
 	ItemInstance* pInst = getItem(idx);
-	if (!pInst)
+	if (ItemInstance::isNull(pInst))
 		return -1;
 
 	return pInst->m_itemID;
@@ -238,13 +238,8 @@ ItemInstance* Inventory::getQuickSlotItem(int slotNo)
 		return nullptr;
 
 	ItemInstance* pInst = getItem(m_hotbar[slotNo]);
-	if (!pInst)
-		return nullptr;
-
-	if (pInst->m_itemID == 0)
-		return nullptr;
 
-	return pInst;
+	return !ItemInstance::isNull(pInst) ? pInst : nullptr;
 }
 
 ItemInstance* Inventory::getSelectedItem()
@@ -324,7 +319,7 @@ void Inventory::selectItemById(int itemID, int maxHotBarSlot)
 int Inventory::getAttackDamage(Entity* pEnt)
 {
 	ItemInstance* pInst = getSelected();
-	if (!pInst)
+	if (ItemInstance::isNull(pInst))
 		return 1;
 
 	return pInst->getAttackDamage(pEnt);

source/world/item/ItemInstance.cpp

@@ -151,29 +151,6 @@ bool ItemInstance::isStackedByData()
 	return getItem()->isStackedByData();
 }
 
-bool ItemInstance::matches(ItemInstance* other) const
-{
-	return this->getAuxValue() == other->getAuxValue() &&
-           this->m_count == other->m_count &&
-           this->m_itemID == other->m_itemID;
-}
-
-bool ItemInstance::matches(ItemInstance* a1, ItemInstance* a2)
-{
-	if (a1 == a2 && a1 == nullptr)
-		return true;
-
-	if (a1 == nullptr || a2 == nullptr)
-		return false;
-
-	return a1->matches(a2);
-}
-
-int ItemInstance::getAttackDamage(Entity *pEnt)
-{
-	return getItem()->getAttackDamage(pEnt);
-}
-
 void ItemInstance::mineBlock(const TilePos& pos, Facing::Name face)
 {
 	return getItem()->mineBlock(this, pos, face);
@@ -212,18 +189,54 @@ bool ItemInstance::useOn(Player* player, Level* level, const TilePos& pos, Facin
 	return getItem()->useOn(this, player, level, pos, face);
 }
 
+int ItemInstance::getAttackDamage(Entity* pEnt)
+{
+	return getItem()->getAttackDamage(pEnt);
+}
+
 bool ItemInstance::isNull() const
 {
 	// 0.9.2
 	if (m_itemID <= 0) // m_field_10, assuming this is m_itemID
 		return true;
 
-	if (m_auxValue != 0)
-		return false;
-	if (m_count != 0)
-		return false;
-	if (m_popTime != 0)
+	if (m_auxValue != 0 ||
+		m_count    != 0 ||
+		m_popTime  != 0)
+	{
 		return false;
+	}
 
 	return true; // isNull
 }
+
+bool ItemInstance::isNull(const ItemInstance* item)
+{
+	return item == nullptr || item->isNull();
+}
+
+bool ItemInstance::matches(const ItemInstance* a1, const ItemInstance* a2)
+{
+	if (a1 == a2 && a1 == nullptr)
+		return true;
+
+	if (a1 == nullptr || a2 == nullptr)
+		return false;
+
+	return a1 == a2;
+}
+
+bool ItemInstance::operator==(const ItemInstance& other) const
+{
+	return this->getAuxValue() == other.getAuxValue() &&
+		   this->m_count       == other.m_count &&
+		   this->m_itemID      == other.m_itemID;
+}
+
+bool ItemInstance::operator!=(const ItemInstance& other) const
+{
+	// doing this is likely more efficient than inverting the result of == after the fact
+	return this->getAuxValue() != other.getAuxValue() ||
+		   this->m_count       != other.m_count ||
+		   this->m_itemID      != other.m_itemID;
+}