From 8585723656cda8287c7c42311b316d8d7d472fd0 Mon Sep 17 00:00:00 2001 From: Bryce! Date: Sun, 1 Nov 2026 18:45:16 -0500 Subject: [PATCH] Fix deck export and mana group ordering --- .../src/interface/deck_loader/deck_loader.cpp | 9 ++++-- .../cards/deck_card_zone_display_widget.cpp | 4 +++ .../libcockatrice/deck_list/deck_list.cpp | 12 ++++++- .../models/deck_list/deck_list_model.cpp | 6 ++-- .../deck_list_model_custom_zones.cpp | 30 ++++++++++++++++-- .../deck_list/deck_list_model_custom_zones.h | 9 ++++-- .../deck_list_model_custom_zones_test.cpp | 31 +++++++++++++++++++ .../deck_list_zones/deck_list_zones_test.cpp | 22 +++++++++++++ 8 files changed, 113 insertions(+), 10 deletions(-) diff --git a/cockatrice/src/interface/deck_loader/deck_loader.cpp b/cockatrice/src/interface/deck_loader/deck_loader.cpp index d413a6c04..d7a042cd4 100644 --- a/cockatrice/src/interface/deck_loader/deck_loader.cpp +++ b/cockatrice/src/interface/deck_loader/deck_loader.cpp @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -371,8 +372,12 @@ bool DeckLoader::saveToStream_Plain(QTextStream &out, saveToStream_DeckHeader(out, deckList); } - // loop zones - for (auto zoneNode : deckList.getZoneNodes()) { + // Plain deck readers expect the main deck before the sideboard. Reorder a + // copy so native saves and the relative order of other zones stay intact. + auto zones = deckList.getZoneNodes(); + std::stable_partition(zones.begin(), zones.end(), + [](const auto *zone) { return zone->getName() == DECK_ZONE_MAIN; }); + for (auto zoneNode : zones) { saveToStream_DeckZone(out, zoneNode, addComments, addSetNameAndNumber); // end of zone diff --git a/cockatrice/src/interface/widgets/cards/deck_card_zone_display_widget.cpp b/cockatrice/src/interface/widgets/cards/deck_card_zone_display_widget.cpp index d7eef6c18..3ec2053b1 100644 --- a/cockatrice/src/interface/widgets/cards/deck_card_zone_display_widget.cpp +++ b/cockatrice/src/interface/widgets/cards/deck_card_zone_display_widget.cpp @@ -69,6 +69,7 @@ DeckCardZoneDisplayWidget::DeckCardZoneDisplayWidget(QWidget *parent, updateZoneCardCount(); connect(deckListModel, &QAbstractItemModel::rowsInserted, this, &DeckCardZoneDisplayWidget::onCategoryAddition); + connect(deckListModel, &QAbstractItemModel::layoutChanged, this, &DeckCardZoneDisplayWidget::displayCards); if (selectionModel) { connect(selectionModel, &QItemSelectionModel::selectionChanged, this, &DeckCardZoneDisplayWidget::onSelectionChanged); @@ -174,6 +175,9 @@ void DeckCardZoneDisplayWidget::displayCards() for (const QPersistentModelIndex &persistent : rows) { constructAppropriateWidget(persistent); + // Reuse existing widgets when a model sort moves their persistent rows. + // This preserves collapsed groups and card selection during live edits. + cardGroupLayout->insertWidget(persistent.row(), indexToWidgetMap.value(persistent)); } } diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp index 821b53dff..71703b32a 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp @@ -340,7 +340,17 @@ bool DeckList::saveToStream_Plain(QTextStream &stream, bool prefixSideboardCards } }; - forEachCard(writeToStream); + // Export the main deck first without changing the saved tree's zone order. + forEachCard([&](const auto node, const auto card) { + if (node->getName() == DECK_ZONE_MAIN) { + writeToStream(node, card); + } + }); + forEachCard([&](const auto node, const auto card) { + if (node->getName() != DECK_ZONE_MAIN) { + writeToStream(node, card); + } + }); return true; } diff --git a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp index 4b4088972..c7a0d909e 100644 --- a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp +++ b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp @@ -739,7 +739,8 @@ QModelIndex DeckListModel::nodeToIndex(AbstractDecklistNode *node) const void DeckListModel::sortShadowTree(InnerDecklistNode *node, Qt::SortOrder order) { // The mapping is not needed: fresh shadow nodes have no persistent indices yet. - (void)DeckListModelCustomZones::sortWithCustomZonesLast(root, node, order); + (void)DeckListModelCustomZones::sortWithCustomZonesLast( + root, node, order, activeGroupCriteria == DeckListModelGroupCriteria::MANA_COST && lastKnownColumn != 0); for (int i = node->size() - 1; i >= 0; --i) { if (auto *subNode = dynamic_cast(node->at(i))) { @@ -752,7 +753,8 @@ void DeckListModel::sortHelper(InnerDecklistNode *node, Qt::SortOrder order) { // Sort children (custom zones always sorted after groups within a board) and // use the movement mapping to update the list of persistent indices. - const auto mapping = DeckListModelCustomZones::sortWithCustomZonesLast(root, node, order); + const auto mapping = DeckListModelCustomZones::sortWithCustomZonesLast( + root, node, order, activeGroupCriteria == DeckListModelGroupCriteria::MANA_COST && lastKnownColumn != 0); QModelIndexList from, to; int columns = columnCount(); diff --git a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model_custom_zones.cpp b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model_custom_zones.cpp index 419c38b04..523708cbd 100644 --- a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model_custom_zones.cpp +++ b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model_custom_zones.cpp @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include namespace DeckListModelCustomZones @@ -111,7 +113,7 @@ QList> plainSort(InnerDecklistNode *node, Qt::SortOrder order) * * @return The (preSortRow, finalRow) mapping covering both the sort and the shift. */ -QList> boardSort(InnerDecklistNode *node, Qt::SortOrder order) +QList> boardSort(InnerDecklistNode *node, Qt::SortOrder order, bool numericGroups) { const QVector> sortResult = node->sort(order); @@ -132,6 +134,27 @@ QList> boardSort(InnerDecklistNode *node, Qt::SortOrder order) } } + if (numericGroups) { + // Only criteria groups are numeric. Mixing numeric and lexical fallback + // comparisons with custom zone names would not define a strict ordering. + std::sort(groups.begin(), groups.end(), [order](const auto *left, const auto *right) { + const QString leftName = left->getName(); + const QString rightName = right->getName(); + bool leftIsNumber, rightIsNumber; + const double leftValue = leftName.toDouble(&leftIsNumber); + const double rightValue = rightName.toDouble(&rightIsNumber); + leftIsNumber = leftIsNumber && std::isfinite(leftValue); + rightIsNumber = rightIsNumber && std::isfinite(rightValue); + if (leftIsNumber != rightIsNumber) { + return leftIsNumber; + } + if (leftIsNumber && leftValue != rightValue) { + return order == Qt::AscendingOrder ? leftValue < rightValue : leftValue > rightValue; + } + return order == Qt::AscendingOrder ? leftName < rightName : leftName > rightName; + }); + } + QVector ordered = groups + customZones; for (int i = 0; i < ordered.size(); ++i) { node->replace(i, ordered[i]); @@ -147,10 +170,11 @@ QList> boardSort(InnerDecklistNode *node, Qt::SortOrder order) } // namespace -QList> sortWithCustomZonesLast(InnerDecklistNode *root, InnerDecklistNode *node, Qt::SortOrder order) +QList> +sortWithCustomZonesLast(InnerDecklistNode *root, InnerDecklistNode *node, Qt::SortOrder order, bool numericGroups) { const bool isBoardZone = (node != root) && (node->getParent() == root); - return isBoardZone ? boardSort(node, order) : plainSort(node, order); + return isBoardZone ? boardSort(node, order, numericGroups) : plainSort(node, order); } } // namespace DeckListModelCustomZones diff --git a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model_custom_zones.h b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model_custom_zones.h index 53bdeaa1e..d3f477450 100644 --- a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model_custom_zones.h +++ b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model_custom_zones.h @@ -91,10 +91,15 @@ void mirrorCustomZones(const InnerDecklistNode *deckBoardZone, InnerDecklistNode * @param root Root of the shadow tree (used to classify board zones). * @param node The shadow node whose children are reordered. * @param order Sort order to apply. + * @param numericGroups Compare criteria-group names as mana values. Custom zone + * names and card names retain their normal comparison; nonnumeric groups follow + * numeric groups in either sort direction. * @return A list of (preSortRow, finalRow) pairs describing how each node moved. */ -[[nodiscard]] QList> -sortWithCustomZonesLast(InnerDecklistNode *root, InnerDecklistNode *node, Qt::SortOrder order); +[[nodiscard]] QList> sortWithCustomZonesLast(InnerDecklistNode *root, + InnerDecklistNode *node, + Qt::SortOrder order, + bool numericGroups = false); } // namespace DeckListModelCustomZones diff --git a/tests/deck_list_model/deck_list_model_custom_zones_test.cpp b/tests/deck_list_model/deck_list_model_custom_zones_test.cpp index 95df74702..d4e44125c 100644 --- a/tests/deck_list_model/deck_list_model_custom_zones_test.cpp +++ b/tests/deck_list_model/deck_list_model_custom_zones_test.cpp @@ -278,6 +278,37 @@ TEST(DeckListModelCustomZones, SortPlainNodeDoesNotReorderCustomZones) EXPECT_EQ(mapping[1].second, 1); } +TEST(DeckListModelCustomZones, ManaGroupsUseNumericOrderWithoutChangingOtherNodes) +{ + InnerDecklistNode root; + auto *board = new InnerDecklistNode(DECK_ZONE_MAIN, &root); + const QStringList original = {"10", "2", "0.5", "1000000", "1", "unknown", "nan"}; + for (const auto &name : original) { + new InnerDecklistNode(name, board); + } + auto *custom = new DecklistModelSubZoneNode("1x", board); + new InnerDecklistNode("10", custom); + new InnerDecklistNode("2", custom); + root.setSortMethod(DeckSortMethod::ByName); + + const auto mapping = DeckListModelCustomZones::sortWithCustomZonesLast(&root, board, Qt::AscendingOrder, true); + EXPECT_EQ(childNames(board), (QStringList{"0.5", "1", "2", "10", "1000000", "nan", "unknown", "1x"})); + QStringList before = original; + before.append("1x"); + for (const auto &move : mapping) { + EXPECT_EQ(board->at(move.second)->getName(), before.at(move.first)); + } + + (void)DeckListModelCustomZones::sortWithCustomZonesLast(&root, board, Qt::DescendingOrder, true); + EXPECT_EQ(childNames(board), (QStringList{"1000000", "10", "2", "1", "0.5", "unknown", "nan", "1x"})); + + // The numeric flag only affects groups directly under a board. + (void)DeckListModelCustomZones::sortWithCustomZonesLast(&root, custom, Qt::AscendingOrder, true); + EXPECT_EQ(childNames(custom), (QStringList{"10", "2"})); + (void)DeckListModelCustomZones::sortWithCustomZonesLast(&root, board, Qt::AscendingOrder); + EXPECT_EQ(childNames(board), (QStringList{"0.5", "1", "10", "1000000", "2", "nan", "unknown", "1x"})); +} + int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv); diff --git a/tests/deck_list_zones/deck_list_zones_test.cpp b/tests/deck_list_zones/deck_list_zones_test.cpp index 1903b70df..a67e689cf 100644 --- a/tests/deck_list_zones/deck_list_zones_test.cpp +++ b/tests/deck_list_zones/deck_list_zones_test.cpp @@ -393,6 +393,28 @@ TEST(DeckListZones, MaybeboardCustomZoneCardsAreExcludedFromHash) EXPECT_EQ(deck.getDeckHash(), expected.getDeckHash()); } +TEST(DeckListZones, PlainExportPutsMainFirstWithoutReorderingSavedZones) +{ + DeckList deck; + auto *tree = deck.getTree(); + ASSERT_NE(tree->addCustomZone(DECK_ZONE_SIDE, "Side Tech"), nullptr); + tree->addCard("Side Card", 2, "Side Tech", -1); + tree->addCard("Token Card", 1, DECK_ZONE_TOKENS, -1); + ASSERT_NE(tree->addCustomZone(DECK_ZONE_MAIN, "Removal"), nullptr); + tree->addCard("Main Card", 3, "Removal", -1); + ASSERT_NE(tree->addCustomZone(DECK_ZONE_MAYBEBOARD, "Candidates"), nullptr); + tree->addCard("Maybe Card", 4, "Candidates", -1); + + const QString native = deck.writeToString_Native(); + EXPECT_EQ(deck.writeToString_Plain(true, false), QString("3 Main Card\nSB: 2 Side Card\n1 Token Card\n")); + EXPECT_EQ(deck.writeToString_Plain(false, false), QString("3 Main Card\n2 Side Card\n1 Token Card\n")); + EXPECT_EQ(deck.writeToString_Native(), native); + + DeckList reopened(native); + EXPECT_EQ(reopened.getZoneNodes().first()->getName(), DECK_ZONE_SIDE); + EXPECT_EQ(reopened.writeToString_Plain(true, false), deck.writeToString_Plain(true, false)); +} + int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv);