Container logic #290
Container logic #290
Conversation
| void Gui::handleScrollWheel(bool down) | ||
| { | ||
| int slot = m_pMinecraft->m_pLocalPlayer->m_pInventory->m_selectedHotbarSlot; | ||
| int slot = m_pMinecraft->m_pLocalPlayer->m_pInventory->m_selected; |
There was a problem hiding this comment.
Name this m_selectedSlot
| Inventory* pInv = getInventory(); | ||
|
|
||
| pInv->selectItem(m_selectedSlot, m_pMinecraft->m_pGui->getNumUsableSlots()); | ||
| std::swap(pInv->getItem(m_selectedSlot), pInv->getSelected()); |
There was a problem hiding this comment.
This looks odd, though I think I know what it's doing. It should really be in its own function.
|
|
||
| ItemInstance* pItem = pPlayer->getSelectedItem(); | ||
| if (pItem) | ||
| ItemInstance& pItem = pPlayer->getSelectedItem(); |
| ItemInstance* pItem = pPlayer->getSelectedItem(); | ||
| if (pItem) | ||
| ItemInstance& pItem = pPlayer->getSelectedItem(); | ||
| //why only if it's empty? |
There was a problem hiding this comment.
I don't understand the question, this was a null check before you changed it.
There was a problem hiding this comment.
The same is valid for the null check, why only if it isn't nullptr? Why the client would know if it's a null selected item before the player joined?
| PlayerEquipmentPacket() | ||
| { | ||
| m_playerID = 0; | ||
| m_itemID = 0; |
There was a problem hiding this comment.
Changing the meaning behind this will absolutely break compatibility with network protocol v2 and v3. b1.2_02 does not have a PlayerEquipmentPacket, but in its AddPlayerPacket, it still specifies an item ID rather than a slot ID. I understand that this makes more sense when being sent from client-to-server, but when sent server-to-client, this implies that the client is expected to know the entire hotbar contents (or more) of another player's inventory, which they shouldn't.
For compatibility sake, I would prefer that you not change this, however, if you're really committed to it, please put the changes behind a NETWORK_PROTOCOL_VERSION >= 4 preprocessor check, even though I'm not even sure if protocol v4 supports this.
| int getSelectedSlotNo() const { return m_selectedHotbarSlot; } | ||
| bool contains(const ItemInstance&) const; | ||
|
|
||
| int getSelectedSlotNo() const { return m_selected; } |
There was a problem hiding this comment.
Shouldn't this be unsigned?
| return "Inventory"; | ||
| } | ||
|
|
||
| void setChanged() override |
There was a problem hiding this comment.
Make this use one line.
|
|
||
| } | ||
|
|
||
| bool stillValid(Player* player) override |
There was a problem hiding this comment.
Make this use one line.
|
|
||
| public: | ||
| int m_selectedHotbarSlot; | ||
| int m_selected; |
|
|
||
| int m_hotbar[C_MAX_HOTBAR_ITEMS]; | ||
| std::vector<ItemInstance*> m_items; | ||
| mutable std::vector<ItemInstance> m_items; |
There was a problem hiding this comment.
What on earth does mutable do here? I've never seen it used before.
There was a problem hiding this comment.
to be able to access the vector in const methods, like Inventory::getItem
2 participants
Inventoryto useItemInstancereferences instead of pointers.Containerand implemented inInventory, together withContainerListener,ContainerMenuandSlotInventoryMenu,Player::m_pInventoryMenuandPlayer::m_pContainerMenustill needs to be implemented