From 4f9fac561df2f685b26d44d19efd2c04b5291f2e Mon Sep 17 00:00:00 2001 From: Kelsi Date: Fri, 31 Jul 2026 15:49:44 -0700 Subject: [PATCH] feat(bank): sort an individual bank bag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bank had one Sort button and it sorted everything, which pools every item into the main slots — the wrong tool for a bag being kept as a category, since sorting it empties it into the bank proper. Add sortBankBag and computeBankBagSortSwaps, which order one bag's contents by the same rule as everywhere else (quality desc, then item id, then stack size) and address only that bag's container, and put a Sort button on each bag's header in the grouped view. The existing button becomes "Sort All" so the difference in scope is legible before it is clicked rather than after. Both feed the bank's existing swap queue, so the server sees one CMSG_SWAP_ITEM per frame either way. --- include/game/inventory.hpp | 6 +++ src/game/inventory.cpp | 91 +++++++++++++++++++++++++++++++++++ src/ui/window_manager.cpp | 17 ++++++- tests/test_inventory_sort.cpp | 50 +++++++++++++++++++ 4 files changed, 163 insertions(+), 1 deletion(-) diff --git a/include/game/inventory.hpp b/include/game/inventory.hpp index 9c87790d..b13edfea 100644 --- a/include/game/inventory.hpp +++ b/include/game/inventory.hpp @@ -186,6 +186,11 @@ public: // sorting never spills items into slots the server doesn't have. void sortBank(int mainSlotCount); + // Sort one bank bag's contents in place, leaving the rest of the bank alone. + // Sorting the whole bank pools everything into the main slots, which is the + // wrong tool when a bag is being kept as a deliberate category. + void sortBankBag(int bagIndex); + // A single swap operation using WoW bag/slot addressing (for CMSG_SWAP_ITEM). struct SwapOp { uint8_t srcBag; @@ -198,6 +203,7 @@ public: // Does NOT modify the inventory — caller is responsible for sending packets. std::vector computeSortSwaps() const; std::vector computeBankSortSwaps(int mainSlotCount) const; + std::vector computeBankBagSortSwaps(int bagIndex) const; // WoW bag/slot addressing for bank storage (used by sort + drag-drop): // main bank slots live in bag 0xFF at slot BANK_SLOT_START + index; each bank bag's diff --git a/src/game/inventory.cpp b/src/game/inventory.cpp index 43983e48..0092260f 100644 --- a/src/game/inventory.cpp +++ b/src/game/inventory.cpp @@ -444,6 +444,97 @@ std::vector Inventory::computeBankSortSwaps(int mainSlotCount return swaps; } +// Sort one bank bag in place. sortBank() pools every item into the main slots, +// which destroys a bag being used as a deliberate category — a bag of herbs +// stays a bag of herbs, just in order. +void Inventory::sortBankBag(int bagIndex) { + if (bagIndex < 0 || bagIndex >= BANK_BAG_SLOTS) return; + BagData& bag = bankBags_[bagIndex]; + if (bag.special) return; // Restricted containers keep their contents in place + + std::vector items; + items.reserve(static_cast(bag.size)); + for (int s = 0; s < bag.size; ++s) { + if (!bag.slots[s].empty()) items.push_back(bag.slots[s].item); + } + + // Same ordering as sortBags(): quality desc → itemId asc → stackCount desc. + std::stable_sort(items.begin(), items.end(), [](const ItemDef& a, const ItemDef& b) { + if (a.quality != b.quality) + return static_cast(a.quality) > static_cast(b.quality); + if (a.itemId != b.itemId) + return a.itemId < b.itemId; + return a.stackCount > b.stackCount; + }); + + int idx = 0; + const int n = static_cast(items.size()); + for (int s = 0; s < bag.size; ++s) + bag.slots[s].item = (idx < n) ? items[idx++] : ItemDef{}; +} + +std::vector Inventory::computeBankBagSortSwaps(int bagIndex) const { + std::vector swaps; + if (bagIndex < 0 || bagIndex >= BANK_BAG_SLOTS) return swaps; + const BagData& bag = bankBags_[bagIndex]; + if (bag.special) return swaps; // must match sortBankBag(): never touch restricted bags + + struct Entry { + uint8_t slot; + uint32_t itemId; + ItemQuality quality; + uint32_t stackCount; + }; + + std::vector entries; + entries.reserve(static_cast(bag.size)); + for (int s = 0; s < bag.size; ++s) { + entries.push_back({static_cast(s), bag.slots[s].item.itemId, + bag.slots[s].item.quality, bag.slots[s].item.stackCount}); + } + + const int n = static_cast(entries.size()); + std::vector sortedIdx(n); + for (int i = 0; i < n; ++i) sortedIdx[i] = i; + + // Non-empty items sort by quality desc → itemId asc → stackCount desc; empties go to the end. + std::stable_sort(sortedIdx.begin(), sortedIdx.end(), [&](int a, int b) { + const bool aEmpty = (entries[a].itemId == 0); + const bool bEmpty = (entries[b].itemId == 0); + if (aEmpty != bEmpty) return bEmpty; + if (aEmpty) return false; + if (entries[a].quality != entries[b].quality) + return static_cast(entries[a].quality) > static_cast(entries[b].quality); + if (entries[a].itemId != entries[b].itemId) + return entries[a].itemId < entries[b].itemId; + return entries[a].stackCount > entries[b].stackCount; + }); + + const uint8_t bagAddr = static_cast(BANK_BAG_CONTAINER_START + bagIndex); + + std::vector posOf(n); + for (int i = 0; i < n; ++i) posOf[i] = i; + std::vector invPos(n); + for (int i = 0; i < n; ++i) invPos[i] = i; + + for (int target = 0; target < n; ++target) { + const int need = sortedIdx[target]; + const int cur = invPos[target]; + if (cur == need) continue; + if (entries[cur].itemId == 0 && entries[need].itemId == 0) continue; + + const int srcPos = posOf[need]; + swaps.push_back({bagAddr, entries[srcPos].slot, bagAddr, entries[target].slot}); + + posOf[cur] = srcPos; + posOf[need] = target; + invPos[srcPos] = cur; + invPos[target] = need; + } + + return swaps; +} + void Inventory::populateTestItems() { // Equipment { diff --git a/src/ui/window_manager.cpp b/src/ui/window_manager.cpp index 889f1535..4c53b865 100644 --- a/src/ui/window_manager.cpp +++ b/src/ui/window_manager.cpp @@ -3324,7 +3324,7 @@ bool WindowManager::renderBankWindow(game::GameHandler& gameHandler, // Toolbar: Sort button + contiguous-view toggle bool sorting = !bankSortQueue.empty(); if (sorting) ImGui::BeginDisabled(); - if (ImGui::SmallButton(sorting ? "Sorting..." : "Sort")) { + if (ImGui::SmallButton(sorting ? "Sorting..." : "Sort All")) { // Compute swaps before mutating local state, apply the local preview, then queue packets. auto swaps = inv.computeBankSortSwaps(bankSlotCount); inv.sortBank(bankSlotCount); @@ -3397,6 +3397,21 @@ bool WindowManager::renderBankWindow(game::GameHandler& gameHandler, ImGui::Spacing(); ImGui::Text("Bank Bag %d (%d slots)", bagIdx + 1, bagSize); + // Sorting the whole bank pools everything into the main slots, so a + // bag being kept as a category needs its own button to stay one. + ImGui::SameLine(); + ImGui::PushID(3000 + bagIdx); + if (sorting) ImGui::BeginDisabled(); + if (ImGui::SmallButton("Sort")) { + auto bagSwaps = inv.computeBankBagSortSwaps(bagIdx); + inv.sortBankBag(bagIdx); + for (auto& sw : bagSwaps) bankSortQueue.push_back(sw); + } + if (sorting) ImGui::EndDisabled(); + if (ImGui::IsItemHovered(ImGuiHoveredFlags_AllowWhenDisabled)) { + ImGui::SetTooltip("Sort just this bag, leaving the rest of the bank alone."); + } + ImGui::PopID(); for (int s = 0; s < bagSize; s++) { if (s % kBankCols != 0) ImGui::SameLine(); ImGui::PushID(3000 + bagIdx * 100 + s); diff --git a/tests/test_inventory_sort.cpp b/tests/test_inventory_sort.cpp index 272a9d17..a237c6f2 100644 --- a/tests/test_inventory_sort.cpp +++ b/tests/test_inventory_sort.cpp @@ -143,3 +143,53 @@ TEST_CASE("computeBankSortSwaps addresses bank slots and bags correctly", "[inve CHECK(dstOk); } } + +TEST_CASE("sortBankBag orders one bag without touching the rest of the bank", + "[inventory]") { + Inventory inv; + inv.setBankSlot(0, makeItem(900, ItemQuality::POOR)); + inv.setBankBagSize(0, 6); + inv.setBankBagSize(1, 6); + inv.setBankBagSlot(0, 0, makeItem(500, ItemQuality::COMMON)); + inv.setBankBagSlot(0, 4, makeItem(100, ItemQuality::EPIC)); + inv.setBankBagSlot(1, 2, makeItem(700, ItemQuality::RARE)); + + inv.sortBankBag(0); + + // Sorted and compacted to the front of its own bag. + CHECK(inv.getBankBagSlot(0, 0).item.itemId == 100); + CHECK(inv.getBankBagSlot(0, 1).item.itemId == 500); + CHECK(inv.getBankBagSlot(0, 4).empty()); + + // Nothing pooled into the main bank, and the other bag is untouched — which + // is the whole point of sorting one bag rather than the whole bank. + CHECK(inv.getBankSlot(0).item.itemId == 900); + CHECK(inv.getBankSlot(1).empty()); + CHECK(inv.getBankBagSlot(1, 2).item.itemId == 700); +} + +TEST_CASE("computeBankBagSortSwaps stays inside the one bag", "[inventory]") { + Inventory inv; + inv.setBankSlot(0, makeItem(900, ItemQuality::POOR)); + inv.setBankBagSize(2, 5); + inv.setBankBagSlot(2, 0, makeItem(500, ItemQuality::COMMON)); + inv.setBankBagSlot(2, 3, makeItem(100, ItemQuality::EPIC)); + + const auto swaps = inv.computeBankBagSortSwaps(2); + CHECK(!swaps.empty()); + + const uint8_t expected = + static_cast(Inventory::BANK_BAG_CONTAINER_START + 2); + for (const auto& op : swaps) { + CHECK(op.srcBag == expected); + CHECK(op.dstBag == expected); + } +} + +TEST_CASE("an out-of-range bank bag index is a no-op", "[inventory]") { + Inventory inv; + inv.sortBankBag(-1); + inv.sortBankBag(Inventory::BANK_BAG_SLOTS); + CHECK(inv.computeBankBagSortSwaps(-1).empty()); + CHECK(inv.computeBankBagSortSwaps(Inventory::BANK_BAG_SLOTS).empty()); +}