diff --git a/.ci/compile.sh b/.ci/compile.sh index 569ae9f6b..1db0cacd1 100755 --- a/.ci/compile.sh +++ b/.ci/compile.sh @@ -185,6 +185,9 @@ if [[ $MAKE_COVERAGE ]]; then fi if [[ $USE_CCACHE ]]; then flags+=("-DUSE_CCACHE=1") + # PCH-aware caching is required or ccache refuses to cache any TU that + # consumes a precompiled header, silently recompiling everything on every run. + ccache --set-config sloppiness=pch_defines,time_macros if [[ $CCACHE_SIZE ]]; then # note, this setting persists after running the script ccache --max-size "$CCACHE_SIZE" @@ -375,21 +378,6 @@ echo "Running cmake with flags: ${flags[*]}" cmake .. "${flags[@]}" echo "::endgroup::" -if [[ $USE_CCACHE ]]; then - # PCH-aware caching is required or ccache refuses to cache any TU that consumes a - # precompiled header, silently recompiling everything on every run. The mtime and ctime - # entries cover files created during the build itself: the moc/uic/rcc output and the - # protobuf headers are generated after ccache starts, so without them every TU including - # one is read as a race and never cached. Checked-out sources are older than the compile - # that follows them and do not trip the check. - # - # This has to run after configure: CMakeLists.txt sets sloppiness=pch_defines,time_macros - # at configure time, and --set-config replaces the whole list rather than extending it, so - # setting it earlier would leave the build below with two entries. Re-asserting all four - # here is what actually reaches the compiler; the order is the point. - ccache --set-config sloppiness=pch_defines,time_macros,include_file_mtime,include_file_ctime -fi - echo "::group::Build project" cpuinfo echo "Running cmake --build with flags: ${buildflags[*]}" diff --git a/.github/workflows/desktop-build.yml b/.github/workflows/desktop-build.yml index 4ac28d3a5..d72cb881e 100644 --- a/.github/workflows/desktop-build.yml +++ b/.github/workflows/desktop-build.yml @@ -157,13 +157,11 @@ jobs: package: DEB server_only: yes test: skip - ccache_size: 400M # server only, no client to compile - distro: Debian version: 13 package: DEB - ccache_size: 850M # the Release build plus the suite it runs - distro: Fedora version: 44 @@ -174,7 +172,6 @@ jobs: # only the Debug configuration passes -Werror, so these two are the legs where a # new warning in either compiler still fails the build. debug_test: yes - ccache_size: 900M # Debug for the suite and Release for the package - distro: Ubuntu version: 26.04 @@ -196,7 +193,6 @@ jobs: package: skip sanitize: yes - ccache_size: 800M # instrumented objects are larger name: ${{ matrix.distro }} ${{ matrix.version }}${{ matrix.variant && format(' {0}', matrix.variant) || '' }} needs: configure @@ -209,27 +205,7 @@ jobs: env: CACHE: ${{ github.workspace }}/.cache/${{ matrix.distro }}${{ matrix.version }}${{ matrix.variant }} # directory for caching docker image and ccache CCACHE_EVICTION_AGE: 7d - # The bound is sized against the repository's 10 GB cache quota - # (https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#usage-limits-and-eviction-policy), - # not against what one leg would like. The quota currently holds about 1.1 GB of - # CodeQL trap, 1.1 GB of vcpkg, 0.7 GB of Qt and 2.1 GB of docker buildkit layers - # (mode=min in docker-release.yml, added here, brings the last one down to roughly - # 0.3 GB - the one input here that is an expectation rather than a measurement, since - # mode=min still writes one layer per pushed binary) plus the compiler caches of the nine legs a pull request runs. The five - # nightly-only legs keep no compiler cache (see PERSIST_CCACHE below), which is what - # makes these per-leg bounds affordable. Accounting for all of it: 6.45G of ccache - # bounds + 3.2G of the fixed entries = 9.65G against the 10G quota. - # - # A configuration needs roughly 500-850 MB on Linux, so the old uniform 600M bound sat - # below what a leg writes: ccache spent each build deleting the entries the next run - # asked for (measured: 2624-3720 cleanup passes per build, 8% of calls served). Each - # leg states the size it needs instead: one configuration for most, two for Fedora 44 - # (Debug for the suite, Release for the package), the server alone for Servatrice. - # Raising these further means caching fewer legs, since GitHub evicts a whole entry - # when the total is exceeded. - # - # Every key listed in LIVE_CCACHE_KEYS in prune-caches below must match this matrix. - CCACHE_SIZE: ${{ matrix.ccache_size || '700M' }} + CCACHE_SIZE: 600M # space of all repo is 10Gi: https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#usage-limits-and-eviction-policy CMAKE_GENERATOR: 'Ninja' NAME: ${{ matrix.distro }}${{ matrix.version }} IMAGE_SUFFIX: ${{ matrix.variant }} @@ -242,11 +218,6 @@ jobs: # also package, so testing and packaging cost one compile instead of two. DEBUG_TEST: ${{ matrix.sanitize == 'yes' || matrix.coverage == 'yes' || matrix.debug_test == 'yes' }} RUN_TESTS: ${{ matrix.test != 'skip' && !(matrix.sanitize == 'yes' || matrix.coverage == 'yes' || matrix.debug_test == 'yes') }} - # Whether this leg restores and saves its compiler cache. Every leg a pull request - # runs keeps one; the nightly-only legs do not, so that the quota is spent on the - # builds that review a change. A tag build runs both sets, and the nightly legs are - # cold there - accepted, because a tag is rare and only the pull-request legs gate. - PERSIST_CCACHE: 'true' # Shared with build-linux-nightly so the two leg sets run the same steps. steps: &linux-steps @@ -255,7 +226,6 @@ jobs: - name: "Restore compiler cache (ccache)" id: restore_ccache - if: env.PERSIST_CCACHE == 'true' uses: actions/cache/restore@v6 with: key: ccache-${{ matrix.distro }}${{ matrix.version }}${{ matrix.variant }} @@ -401,7 +371,7 @@ jobs: # Delete used cache to emulate a ccache update. See https://github.com/actions/cache/issues/342 - name: "Delete remote compiler cache (ccache)" - if: github.ref == 'refs/heads/master' && env.PERSIST_CCACHE == 'true' && steps.restore_ccache.outputs.cache-hit + if: github.ref == 'refs/heads/master' && steps.restore_ccache.outputs.cache-hit continue-on-error: true env: CACHE_PRIMARY_KEY: ${{ steps.restore_ccache.outputs.cache-primary-key }} @@ -412,7 +382,7 @@ jobs: fi - name: "Save updated compiler cache (ccache)" - if: github.ref == 'refs/heads/master' && env.PERSIST_CCACHE == 'true' + if: github.ref == 'refs/heads/master' uses: actions/cache/save@v6 with: key: ${{ steps.restore_ccache.outputs.cache-primary-key }} @@ -509,12 +479,7 @@ jobs: env: CACHE: ${{ github.workspace }}/.cache/${{ matrix.distro }}${{ matrix.version }}${{ matrix.variant }} # directory for caching docker image and ccache CCACHE_EVICTION_AGE: 7d - # These legs do not keep a compiler cache: they run once a day (and on tags) and gate - # nothing, so their share of the 10 GB quota goes to the pull-request legs instead. - # A cold nightly build costs minutes on a runner nobody waits for; a cold pull request - # build costs a review. CCACHE_SIZE is still passed so ccache trims its scratch dir. - PERSIST_CCACHE: 'false' - CCACHE_SIZE: 600M + CCACHE_SIZE: 600M # space of all repo is 10Gi: https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#usage-limits-and-eviction-policy CMAKE_GENERATOR: 'Ninja' NAME: ${{ matrix.distro }}${{ matrix.version }} IMAGE_SUFFIX: ${{ matrix.variant }} @@ -526,8 +491,8 @@ jobs: matrix: include: - os: macOS - target: 14 # EOL 2026-09-14 - runner: xcode-27 # https://github.com/actions/runner-images/blob/main/images/macos/xcode-27-arm64-Readme.md + target: 14 # EOL 2026-?? + runner: macos-26 # https://github.com/actions/runner-images/blob/main/images/macos/macos-26-arm64-Readme.md ccache_eviction_age: 7d cmake_generator: Ninja @@ -542,10 +507,11 @@ jobs: vcpkg_deployment_target: 14 vcpkg_scope: 'vcpkg/macos14-arm64-xcode26.6/' vcpkg_upload: 1 # only one leg per vcpkg_scope hands its archives over + xcode: "26.6" - os: macOS - target: 15 # EOL 2027-?? - runner: xcode-27 # https://github.com/actions/runner-images/blob/main/images/macos/xcode-27-arm64-Readme.md + target: 15 + runner: macos-26 # https://github.com/actions/runner-images/blob/main/images/macos/macos-26-arm64-Readme.md ccache_eviction_age: 7d cmake_generator: Ninja @@ -559,10 +525,11 @@ jobs: use_ccache: 1 vcpkg_deployment_target: 14 vcpkg_scope: 'vcpkg/macos14-arm64-xcode26.6/' + xcode: "26.6" - os: macOS - target: 27 - runner: xcode-27 # https://github.com/actions/runner-images/blob/main/images/macos/xcode-27-arm64-Readme.md + target: 26 + runner: macos-26 # https://github.com/actions/runner-images/blob/main/images/macos/macos-26-arm64-Readme.md ccache_eviction_age: 7d cmake_generator: Ninja @@ -573,6 +540,7 @@ jobs: use_ccache: 1 vcpkg_deployment_target: 14 vcpkg_scope: 'vcpkg/macos14-arm64-xcode26.6/' + xcode: "26.6" - os: Windows target: 10 @@ -603,13 +571,7 @@ jobs: timeout-minutes: 100 env: CCACHE_DIR: ${{ github.workspace }}/.cache/ - # macOS configurations compile about twice the TUs of a Linux one, so each of these - # three legs gets a bound above what it has been measured to write. The four macOS - # legs used to be bounded at 600M from the same quota as the Linux ones; the three - # pull-request legs now get 700M each while the nightly Intel leg keeps none, and - # that is what fits (see CCACHE_SIZE in build-linux for the full arithmetic). - CCACHE_SIZE: 700M - PERSIST_CCACHE: 'true' + CCACHE_SIZE: 600M # space of all repo is 10Gi: https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#usage-limits-and-eviction-policy # Shared with build-vcpkg-nightly so the two leg sets run the same steps. steps: &vcpkg-steps @@ -629,11 +591,11 @@ jobs: run: brew install ccache - name: "[macOS] Restore compiler cache (ccache)" - if: matrix.os == 'macOS' && matrix.use_ccache == 1 && env.PERSIST_CCACHE == 'true' + if: matrix.os == 'macOS' && matrix.use_ccache == 1 id: restore_ccache uses: actions/cache/restore@v6 with: - key: ccache-${{ matrix.os }}${{ matrix.override_target }}-${{ matrix.runner }} + key: ccache-${{ matrix.runner }}_${{ matrix.override_target }}-Xcode${{ matrix.xcode }} path: ${{ env.CCACHE_DIR }} - name: "[macOS] Install aqtinstall" @@ -748,6 +710,7 @@ jobs: # configure.outputs.tag is set for ANY tag, so beta (Development) tags get the DSN # and the symbol upload below just like full releases. COCKATRICE_SENTRY_DSN: ${{ needs.configure.outputs.tag != null && secrets.SENTRY_DSN || '' }} + DEVELOPER_DIR: '/Applications/Xcode_${{ matrix.xcode }}.app/Contents/Developer' GITHUB_TOKEN: ${{ github.token }} # needed for vcpkg dependency graph updates, see VCPKG_FEATURE_FLAGS MACOS_CERTIFICATE: ${{ secrets.PROD_MACOS_CERTIFICATE }} MACOS_CERTIFICATE_NAME: ${{ secrets.PROD_MACOS_CERTIFICATE_NAME }} @@ -959,7 +922,7 @@ jobs: # Delete used cache to emulate a ccache update. See https://github.com/actions/cache/issues/342 - name: "[macOS] Delete remote compiler cache (ccache)" - if: matrix.os == 'macOS' && matrix.use_ccache == 1 && env.PERSIST_CCACHE == 'true' && github.ref == 'refs/heads/master' && steps.restore_ccache.outputs.cache-hit + if: matrix.os == 'macOS' && matrix.use_ccache == 1 && github.ref == 'refs/heads/master' && steps.restore_ccache.outputs.cache-hit continue-on-error: true env: CACHE_PRIMARY_KEY: ${{ steps.restore_ccache.outputs.cache-primary-key }} @@ -970,7 +933,7 @@ jobs: fi - name: "[macOS] Cache updated compiler cache (ccache)" - if: matrix.os == 'macOS' && matrix.use_ccache == 1 && env.PERSIST_CCACHE == 'true' && github.ref == 'refs/heads/master' + if: matrix.os == 'macOS' && matrix.use_ccache == 1 && github.ref == 'refs/heads/master' uses: actions/cache/save@v6 with: key: ${{ steps.restore_ccache.outputs.cache-primary-key }} @@ -1108,6 +1071,7 @@ jobs: use_ccache: 1 vcpkg_scope: 'vcpkg/macos13-x64-xcode26.3/' vcpkg_upload: 1 # only one leg per vcpkg_scope hands its archives over + xcode: "26.3" name: ${{ matrix.os }} ${{ matrix.target }}${{ matrix.soc == 'Intel' && ' Intel' || '' }}${{ matrix.type == 'Debug' && ' Debug' || '' }} needs: configure @@ -1118,10 +1082,7 @@ jobs: timeout-minutes: 100 env: CCACHE_DIR: ${{ github.workspace }}/.cache/ - # Like build-linux-nightly: this leg keeps no compiler cache, so the quota stays with - # the pull-request legs (see PERSIST_CCACHE there). - PERSIST_CCACHE: 'false' - CCACHE_SIZE: 600M + CCACHE_SIZE: 600M # space of all repo is 10Gi: https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#usage-limits-and-eviction-policy steps: *vcpkg-steps prune-caches: @@ -1132,23 +1093,12 @@ jobs: continue-on-error: true # freeing space must never fail a build steps: - # The repository holds 10 GB of cache against entries that want more, and GitHub evicts - # the least recently used entry whenever that is exceeded. The vcpkg entries are the - # reason that never happens: the restore above touches every entry under the prefix on - # every run, so a dead ABI generation keeps its access date fresh forever and age based - # eviction can never reach it. Deleting by age alone cannot fix that, only knowing which - # scope an entry belongs to can. The same is true of a compiler cache of a leg that no - # longer keeps one: its restore would keep touching it, so the live key list below is - # what retires it. - # - # docker-release's image builds keep their layer cache in the same quota: one blob per - # layer, keyed buildkit-blob-1-, plus one index--1- entry per image - # scope that lists them. They are written with mode=min, so only the final image stage - # is kept and the set stays small. A layer a build actually needs is downloaded, which - # refreshes its access date, so an old date means the builds have stopped using it and - # age is the right test. Dropping a blob an index still lists costs the next image build - # one rebuilt layer; docker-release gates nothing, so that is cheaper than keeping the - # entries forever. + # The repository holds 10Gi of cache against entries that want more, and GitHub + # evicts the least recently used entry whenever that is exceeded. The vcpkg + # entries are the reason that never happens: the restore above touches every + # entry under the prefix on every run, so a dead ABI generation keeps its access + # date fresh forever and age based eviction can never reach it. Deleting by age + # alone cannot fix that, only knowing which scope an entry belongs to can. - name: "Delete unusable cache entries" shell: bash env: @@ -1157,39 +1107,25 @@ jobs: # matrices above, or the entries of a scope that is still in use get deleted # on the next run. LIVE_VCPKG_SCOPES: 'vcpkg/macos13-x64-xcode26.3/ vcpkg/macos14-arm64-xcode26.6/ vcpkg/windows10-x64-vs2026/' - # Must list the ccache key of every leg that keeps a compiler cache - # (PERSIST_CCACHE in build-linux and build-vcpkg), or a live entry is deleted. - # The nightly legs keep none, so their leftover entries are reclaimed here - # instead of waiting out the age rule. - LIVE_CCACHE_KEYS: 'ccache-Arch ccache-Servatrice_Debian12 ccache-Debian13 ccache-Fedora44 ccache-Ubuntu26.04 ccache-Ubuntu26.04sanitize ccache-macOS14-xcode-27 ccache-macOS15-xcode-27 ccache-macOS-xcode-27' run: | gh api --paginate "repos/$GITHUB_REPOSITORY/actions/caches?per_page=100" \ --jq '.actions_caches[] | [.id, .ref, .key, .last_accessed_at] | @tsv' >"$RUNNER_TEMP/caches.tsv" cutoff="$(( $(date -u +%s) - 7 * 24 * 3600 ))" live=" ${LIVE_VCPKG_SCOPES} " - live_ccache=" ${LIVE_CCACHE_KEYS} " - deleted=0 while IFS=$'\t' read -r id ref key accessed; do [[ -n $id ]] || continue - # Only ever touch the key shapes our workflows create (buildkit-* and index-* - # come from docker-release.yml), so that an entry of some other workflow can - # never be deleted by accident + # Only ever touch the key shapes this workflow creates, so that an entry of + # some other workflow can never be deleted by accident case "$key" in - vcpkg/* | ccache-* | Qt* | buildkit-* | index-*) ;; + vcpkg/* | ccache-* | Qt*) ;; *) continue ;; esac reason="" if [[ $ref != refs/heads/master ]]; then reason="scoped to $ref" - elif [[ $key == ccache-* ]]; then - # An exact token compare: ccache-Ubuntu26.04 is a prefix of - # ccache-Ubuntu26.04sanitize, and both keys are live - if [[ $live_ccache != *" $key "* ]]; then - reason="no pull-request leg keeps it" - fi elif [[ $key == vcpkg/* ]]; then # "/", cut the hash off the end to get the scope scope="vcpkg/${key#vcpkg/}" diff --git a/.github/workflows/docker-release.yml b/.github/workflows/docker-release.yml index 1537d6d4d..937386940 100644 --- a/.github/workflows/docker-release.yml +++ b/.github/workflows/docker-release.yml @@ -84,18 +84,12 @@ jobs: password: ${{ github.token }} # Don't push for non-publishing triggers - # - # The layer cache shares the repository's 10 GB Actions cache quota with the compiler - # caches (see CCACHE_SIZE in desktop-build.yml). mode=min keeps only the final image - # stage: the build stage sits behind `COPY . .`, so its compile layer is invalidated by - # every source change and was never read back, yet it was the bulk of the ~2 GB this - # cache held. The final runtime stage is the part that still pays off. - name: "Build image" if: steps.login.outcome != 'success' uses: docker/build-push-action@v7 with: cache-from: type=gha,scope=${{ env.CACHE_SCOPE }} - cache-to: ${{ case(github.ref == 'refs/heads/master', format('type=gha,mode=min,scope={0}', env.CACHE_SCOPE), '') }} + cache-to: ${{ case(github.ref == 'refs/heads/master', format('type=gha,mode=max,scope={0}', env.CACHE_SCOPE), '') }} context: . platforms: ${{ matrix.platform }} push: false @@ -107,7 +101,7 @@ jobs: uses: docker/build-push-action@v7 with: cache-from: type=gha,scope=${{ env.CACHE_SCOPE }} - cache-to: type=gha,mode=min,scope=${{ env.CACHE_SCOPE }} + cache-to: type=gha,mode=max,scope=${{ env.CACHE_SCOPE }} context: . labels: | org.opencontainers.image.description=${{ env.OCI_DESCRIPTION }} diff --git a/CMakeLists.txt b/CMakeLists.txt index 9044bc005..563f174e8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -65,10 +65,6 @@ if(USE_CCACHE AND NOT WIN32) # PCH-aware caching, matching .ci/compile.sh: without this ccache refuses # to cache any TU that consumes a precompiled header, so every PCH-backed # target recompiles from scratch on each build. - # The include_file_mtime/include_file_ctime entries that CI also sets are - # deliberately not set here: --set-config writes the user's global - # ccache.conf, and those two would turn off the modified-during-compile - # check for every project on the machine, not just this one. execute_process(COMMAND ${CCACHE_PROGRAM} --set-config sloppiness=pch_defines,time_macros) message(STATUS "Found CCache ${CCACHE_PROGRAM}") endif() diff --git a/Testing/deck_editor_open_test.xml b/Testing/deck_editor_open_test.xml index ab74b1c63..437c4bd23 100644 --- a/Testing/deck_editor_open_test.xml +++ b/Testing/deck_editor_open_test.xml @@ -1,4 +1,4 @@ - + diff --git a/Testing/deck_editor_search_test.xml b/Testing/deck_editor_search_test.xml index 6641ac3b2..0b9f88fa1 100644 --- a/Testing/deck_editor_search_test.xml +++ b/Testing/deck_editor_search_test.xml @@ -1,6 +1,6 @@ - + diff --git a/Testing/integration_test.xml b/Testing/integration_test.xml index 33191d1ac..5fc7a971a 100644 --- a/Testing/integration_test.xml +++ b/Testing/integration_test.xml @@ -36,7 +36,7 @@ - + diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index b18846079..9a3172aa8 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -56,7 +56,6 @@ set(cockatrice_SOURCES src/interface/widgets/dialogs/dlg_settings.cpp src/interface/widgets/dialogs/dlg_startup_card_check.cpp src/interface/widgets/dialogs/dlg_tip_of_the_day.cpp - src/interface/widgets/dialogs/dlg_turn_order.cpp src/interface/widgets/dialogs/dlg_update.cpp src/interface/widgets/dialogs/dlg_view_log.cpp src/interface/widgets/dialogs/override_printing_warning.cpp @@ -496,15 +495,7 @@ if(APPLE) set(cockatrice_SOURCES ${cockatrice_SOURCES} ${CMAKE_CURRENT_SOURCE_DIR}/resources/appicon.icns) endif(APPLE) -# The fonts and card artwork produce a large C++ byte array with qt_add_resources. -# Embed the data in the compiled resource object instead, keeping compiler memory -# independent of that payload. Qt's two-pass resource build is unsupported on iOS. -if(IOS) - qt_add_resources(cockatrice_RESOURCES_RCC ${cockatrice_RESOURCES}) -else() - qt_add_big_resources(cockatrice_RESOURCES_RCC ${cockatrice_RESOURCES}) -endif() -set_source_files_properties(${cockatrice_RESOURCES} PROPERTIES SKIP_AUTORCC ON) +qt_add_resources(cockatrice_RESOURCES_RCC ${cockatrice_RESOURCES}) # Declare path variables set(ICONDIR @@ -530,7 +521,6 @@ qt_add_executable( WIN32 MACOSX_BUNDLE ${cockatrice_SOURCES} - ${cockatrice_RESOURCES} ${cockatrice_RESOURCES_RCC} ${cockatrice_MOC_SRCS} MANUAL_FINALIZATION diff --git a/cockatrice/src/game_graphics/game_scene.cpp b/cockatrice/src/game_graphics/game_scene.cpp index 128899a13..1f780b795 100644 --- a/cockatrice/src/game_graphics/game_scene.cpp +++ b/cockatrice/src/game_graphics/game_scene.cpp @@ -36,7 +36,6 @@ #include #include #include -#include #include #include #include @@ -442,37 +441,6 @@ QList GameScene::collectActivePlayers(int &firstPlayerIndex) cons } } } - - // Lay out the table in seat order rather than id order. Players missing - // from the tracked order keep id order at the end. - QList seatOrder; - for (auto *pgItem : playerViews.values()) { - if (PlayerLogic *p = pgItem->getLogic()) { - seatOrder = p->getGame()->getGameState()->getSeatOrder(); - break; - } - } - if (!seatOrder.isEmpty()) { - std::sort(activePlayers.begin(), activePlayers.end(), [&](PlayerLogic *a, PlayerLogic *b) { - int indexA = seatOrder.indexOf(a->getPlayerInfo()->getId()); - int indexB = seatOrder.indexOf(b->getPlayerInfo()->getId()); - if (indexA == -1) { - indexA = seatOrder.size() + a->getPlayerInfo()->getId(); - } - if (indexB == -1) { - indexB = seatOrder.size() + b->getPlayerInfo()->getId(); - } - return indexA < indexB; - }); - // Recompute the local-player index after sorting. - firstPlayerIndex = 0; - for (int i = 0; i < activePlayers.size(); ++i) { - if (activePlayers[i]->getPlayerInfo()->getLocal()) { - firstPlayerIndex = i; - break; - } - } - } return activePlayers; } diff --git a/cockatrice/src/game_graphics/log/message_log_widget.cpp b/cockatrice/src/game_graphics/log/message_log_widget.cpp index 8b351e022..046a1ae90 100644 --- a/cockatrice/src/game_graphics/log/message_log_widget.cpp +++ b/cockatrice/src/game_graphics/log/message_log_widget.cpp @@ -605,20 +605,6 @@ void MessageLogWidget::logReverseTurn(PlayerLogic *player, bool reversed) .arg(reversed ? tr("reversed") : tr("normal"))); } -void MessageLogWidget::logTurnOrderChanged(QStringList playerNames, bool randomized) -{ - QStringList sanitizedNames; - for (const QString &name : playerNames) { - sanitizedNames.append(sanitizeHtml(name)); - } - const QString order = sanitizedNames.join(" -> "); - if (randomized) { - appendHtmlServerMessage(tr("Randomized turn order: %1.").arg(order)); - } else { - appendHtmlServerMessage(tr("New turn order set: %1.").arg(order)); - } -} - void MessageLogWidget::logRollDie(PlayerLogic *player, int sides, const QList &rolls) { if (rolls.length() == 1) { diff --git a/cockatrice/src/game_graphics/log/message_log_widget.h b/cockatrice/src/game_graphics/log/message_log_widget.h index 5632f5af8..5642a76e9 100644 --- a/cockatrice/src/game_graphics/log/message_log_widget.h +++ b/cockatrice/src/game_graphics/log/message_log_widget.h @@ -94,7 +94,6 @@ public slots: int amount, bool isLentToAnotherPlayer); void logReverseTurn(PlayerLogic *player, bool reversed); - void logTurnOrderChanged(QStringList playerNames, bool randomized); void logRollDie(PlayerLogic *player, int sides, const QList &rolls); void logSay(PlayerLogic *player, QString message); void logSetActivePhase(int phase); diff --git a/cockatrice/src/interface/deck_loader/deck_loader.cpp b/cockatrice/src/interface/deck_loader/deck_loader.cpp index c8fa2a6f8..daa5dc98a 100644 --- a/cockatrice/src/interface/deck_loader/deck_loader.cpp +++ b/cockatrice/src/interface/deck_loader/deck_loader.cpp @@ -39,7 +39,6 @@ #include #include #include -#include #include #include #include @@ -373,12 +372,8 @@ bool DeckLoader::saveToStream_Plain(QTextStream &out, saveToStream_DeckHeader(out, deckList); } - // 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) { + // loop zones + for (auto zoneNode : deckList.getZoneNodes()) { saveToStream_DeckZone(out, zoneNode, addComments, addSetNameAndNumber); // end of zone diff --git a/cockatrice/src/interface/widgets/cards/card_art_utils.cpp b/cockatrice/src/interface/widgets/cards/card_art_utils.cpp index a7e633ad2..ebf79282e 100644 --- a/cockatrice/src/interface/widgets/cards/card_art_utils.cpp +++ b/cockatrice/src/interface/widgets/cards/card_art_utils.cpp @@ -2,8 +2,6 @@ #include "libcockatrice/card/card_info.h" -#include -#include #include #include #include @@ -20,23 +18,4 @@ QPixmap rotateSidewaysLayoutArt(const QPixmap &art, const ExactCard &card) transform.rotate(90); return art.transformed(transform, Qt::SmoothTransformation); } - -QPixmap cropCardArt(const QPixmap &art, const ExactCard &card) -{ - if (art.isNull()) { - return {}; - } - - const QPixmap source = rotateSidewaysLayoutArt(art, card); - const bool landscape = card.getInfo().getUiAttributes().landscapeOrientation; - const QSize size = source.size(); - const int marginX = size.width() * 0.07; - // Landscape name boxes extend below 5% of the upright image. Remove the - // whole box here, rather than relying on the banner's narrow viewport to - // hide it: the same saved offset is also used by the taller profile popup. - const int topMargin = size.height() * (landscape ? 0.15 : 0.11); - const int bottomMargin = size.height() * 0.45; - const QRect crop(marginX, topMargin, size.width() - 2 * marginX, size.height() - topMargin - bottomMargin); - return source.copy(crop.intersected(source.rect())); -} -} // namespace CardArtUtils +} // namespace CardArtUtils \ No newline at end of file diff --git a/cockatrice/src/interface/widgets/cards/card_art_utils.h b/cockatrice/src/interface/widgets/cards/card_art_utils.h index ac3b82380..5c331a12c 100644 --- a/cockatrice/src/interface/widgets/cards/card_art_utils.h +++ b/cockatrice/src/interface/widgets/cards/card_art_utils.h @@ -20,13 +20,6 @@ namespace CardArtUtils * @return @p art rotated 90° clockwise for sideways-layout cards, else @p art. */ QPixmap rotateSidewaysLayoutArt(const QPixmap &art, const ExactCard &card); - -/** - * Return the upright artwork used by banner previews, user rows and profile - * popups. The crop excludes conventional name and rules boxes before callers apply - * their saved pan and zoom, so a taller popup cannot expose the card frame. - */ -QPixmap cropCardArt(const QPixmap &art, const ExactCard &card); } // namespace CardArtUtils -#endif // CARD_ART_UTILS_H +#endif // CARD_ART_UTILS_H \ No newline at end of file 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 3ec2053b1..d7eef6c18 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,7 +69,6 @@ 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); @@ -175,9 +174,6 @@ 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/cockatrice/src/interface/widgets/dialogs/dlg_create_game.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_create_game.cpp index ada4237f7..104b695e6 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_create_game.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_create_game.cpp @@ -116,17 +116,14 @@ void DlgCreateGame::sharedCtor() shareDecklistsOnLoadCheckBox = new QCheckBox(tr("Open decklists in lobby")); - shufflePlayersCheckBox = new QCheckBox(tr("Randomize turn order (seats) on each game start")); - createGameAsJudgeCheckBox = new QCheckBox(tr("Create game as judge")); auto *gameSetupOptionsLayout = new QGridLayout; gameSetupOptionsLayout->addWidget(startingLifeTotalLabel, 0, 0); gameSetupOptionsLayout->addWidget(startingLifeTotalEdit, 0, 1); - gameSetupOptionsLayout->addWidget(shareDecklistsOnLoadCheckBox, 1, 0, 1, 2); - gameSetupOptionsLayout->addWidget(shufflePlayersCheckBox, 2, 0, 1, 2); + gameSetupOptionsLayout->addWidget(shareDecklistsOnLoadCheckBox, 1, 0); if (room && room->getUserInfo()->user_level() & ServerInfo_User::IsJudge) { - gameSetupOptionsLayout->addWidget(createGameAsJudgeCheckBox, 3, 0, 1, 2); + gameSetupOptionsLayout->addWidget(createGameAsJudgeCheckBox, 2, 0); } else { createGameAsJudgeCheckBox->setChecked(false); createGameAsJudgeCheckBox->setHidden(true); @@ -187,7 +184,6 @@ DlgCreateGame::DlgCreateGame(TabRoom *_room, const QMap &_gameType createGameAsSpectatorCheckBox->setChecked(SettingsCache::instance().game().getCreateGameAsSpectator()); startingLifeTotalEdit->setValue(SettingsCache::instance().game().getDefaultStartingLifeTotal()); shareDecklistsOnLoadCheckBox->setChecked(SettingsCache::instance().game().getShareDecklistsOnLoad()); - shufflePlayersCheckBox->setChecked(SettingsCache::instance().game().getShufflePlayers()); if (!rememberGameSettings->isChecked()) { actReset(); @@ -221,7 +217,6 @@ DlgCreateGame::DlgCreateGame(const ServerInfo_Game &gameInfo, const QMapsetEnabled(false); startingLifeTotalEdit->setEnabled(false); shareDecklistsOnLoadCheckBox->setEnabled(false); - shufflePlayersCheckBox->setEnabled(false); descriptionEdit->setText(QString::fromStdString(gameInfo.description())); maxPlayersEdit->setValue(gameInfo.max_players()); @@ -232,7 +227,6 @@ DlgCreateGame::DlgCreateGame(const ServerInfo_Game &gameInfo, const QMapsetChecked(gameInfo.spectators_can_chat()); spectatorsSeeEverythingCheckBox->setChecked(gameInfo.spectators_omniscient()); shareDecklistsOnLoadCheckBox->setChecked(gameInfo.share_decklists_on_load()); - shufflePlayersCheckBox->setChecked(gameInfo.shuffle_players()); QSet types; for (int i = 0; i < gameInfo.game_types_size(); ++i) { @@ -270,7 +264,6 @@ void DlgCreateGame::actReset() startingLifeTotalEdit->setValue(20); shareDecklistsOnLoadCheckBox->setChecked(false); - shufflePlayersCheckBox->setChecked(false); createGameAsJudgeCheckBox->setChecked(false); QMapIterator gameTypeCheckBoxIterator(gameTypeCheckBoxes); @@ -301,7 +294,6 @@ void DlgCreateGame::actOK() cmd.set_join_as_spectator(createGameAsSpectatorCheckBox->isChecked()); cmd.set_starting_life_total(startingLifeTotalEdit->value()); cmd.set_share_decklists_on_load(shareDecklistsOnLoadCheckBox->isChecked()); - cmd.set_shuffle_players(shufflePlayersCheckBox->isChecked()); auto _gameTypes = QString(); QMapIterator gameTypeCheckBoxIterator(gameTypeCheckBoxes); @@ -326,7 +318,6 @@ void DlgCreateGame::actOK() SettingsCache::instance().game().setCreateGameAsSpectator(createGameAsSpectatorCheckBox->isChecked()); SettingsCache::instance().game().setDefaultStartingLifeTotal(startingLifeTotalEdit->value()); SettingsCache::instance().game().setShareDecklistsOnLoad(shareDecklistsOnLoadCheckBox->isChecked()); - SettingsCache::instance().game().setShufflePlayers(shufflePlayersCheckBox->isChecked()); SettingsCache::instance().game().setGameTypes(_gameTypes); } PendingCommand *pend = room->prepareRoomCommand(cmd); diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_create_game.h b/cockatrice/src/interface/widgets/dialogs/dlg_create_game.h index 58356c9b6..71b09997c 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_create_game.h +++ b/cockatrice/src/interface/widgets/dialogs/dlg_create_game.h @@ -51,7 +51,6 @@ private: QCheckBox *spectatorsAllowedCheckBox, *spectatorsNeedPasswordCheckBox, *spectatorsCanTalkCheckBox, *spectatorsSeeEverythingCheckBox, *createGameAsJudgeCheckBox, *createGameAsSpectatorCheckBox; QCheckBox *shareDecklistsOnLoadCheckBox; - QCheckBox *shufflePlayersCheckBox; QDialogButtonBox *buttonBox; QPushButton *clearButton; QCheckBox *rememberGameSettings; diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_turn_order.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_turn_order.cpp deleted file mode 100644 index 8814f89db..000000000 --- a/cockatrice/src/interface/widgets/dialogs/dlg_turn_order.cpp +++ /dev/null @@ -1,136 +0,0 @@ -#include "dlg_turn_order.h" - -#include -#include -#include -#include -#include -#include -#include - -DlgTurnOrder::DlgTurnOrder(const QStringList &_playerNames, QWidget *parent) - : QDialog(parent), randomizeRequested(false) -{ - listWidget = new QListWidget(this); - listWidget->addItems(_playerNames); - listWidget->setCurrentRow(0); - connect(listWidget, &QListWidget::currentRowChanged, this, &DlgTurnOrder::selectionChanged); - - moveUpButton = new QPushButton(this); - connect(moveUpButton, &QPushButton::clicked, this, &DlgTurnOrder::actMoveUp); - moveDownButton = new QPushButton(this); - connect(moveDownButton, &QPushButton::clicked, this, &DlgTurnOrder::actMoveDown); - randomizeButton = new QPushButton(this); - connect(randomizeButton, &QPushButton::clicked, this, &DlgTurnOrder::actRandomize); - - // Shown in place of the list while the server is asked to randomize: the - // order is chosen server side, so there is nothing to preview here. - hintLabel = new QLabel(this); - hintLabel->setWordWrap(true); - hintLabel->setAlignment(Qt::AlignCenter); - - auto *buttonColumn = new QVBoxLayout; - buttonColumn->addWidget(moveUpButton); - buttonColumn->addWidget(moveDownButton); - buttonColumn->addWidget(randomizeButton); - buttonColumn->addStretch(); - - auto *listLayout = new QHBoxLayout; - listLayout->addWidget(listWidget); - listLayout->addWidget(hintLabel, 1); - listLayout->addLayout(buttonColumn); - - auto *buttonBox = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel); - connect(buttonBox, &QDialogButtonBox::accepted, this, &DlgTurnOrder::actOk); - connect(buttonBox, &QDialogButtonBox::rejected, this, &DlgTurnOrder::reject); - - auto *mainLayout = new QVBoxLayout; - mainLayout->addLayout(listLayout); - mainLayout->addWidget(buttonBox); - setLayout(mainLayout); - - updateButtons(); - - updateMode(); - setMinimumWidth(300); -} - -void DlgTurnOrder::retranslateUi() -{ - setWindowTitle(tr("Set turn order")); - moveUpButton->setText(tr("Move up")); - moveDownButton->setText(tr("Move down")); - randomizeButton->setText(randomizeRequested ? tr("Set order manually") : tr("Randomize")); - hintLabel->setText(tr("The server will randomize the turn order.")); -} - -QStringList DlgTurnOrder::order() const -{ - QStringList playerNames; - for (int i = 0; i < listWidget->count(); ++i) { - playerNames.append(listWidget->item(i)->text()); - } - return playerNames; -} - -void DlgTurnOrder::moveSelected(int delta) -{ - const int row = listWidget->currentRow(); - const int newRow = row + delta; - if (row < 0 || newRow < 0 || newRow >= listWidget->count()) { - return; - } - - QListWidgetItem *item = listWidget->takeItem(row); - listWidget->insertItem(newRow, item); - listWidget->setCurrentRow(newRow); - updateButtons(); -} - -void DlgTurnOrder::updateMode() -{ - listWidget->setVisible(!randomizeRequested); - hintLabel->setVisible(randomizeRequested); - moveUpButton->setVisible(!randomizeRequested); - moveDownButton->setVisible(!randomizeRequested); - retranslateUi(); -} - -void DlgTurnOrder::updateButtons() -{ - const int row = listWidget->currentRow(); - const int count = listWidget->count(); - const bool manualOrder = !randomizeRequested; - moveUpButton->setEnabled(manualOrder && row > 0); - moveDownButton->setEnabled(manualOrder && row >= 0 && row < count - 1); -} - -void DlgTurnOrder::actMoveUp() -{ - moveSelected(-1); -} - -void DlgTurnOrder::actMoveDown() -{ - moveSelected(1); -} - -void DlgTurnOrder::actRandomize() -{ - // Randomizing is a request to the server, not a local shuffle: showing a - // preview would let the host re-open the dialog until the order looks - // favourable for them. - randomizeRequested = !randomizeRequested; - updateMode(); - updateButtons(); -} - -void DlgTurnOrder::actOk() -{ - accept(); -} - -void DlgTurnOrder::selectionChanged() -{ - updateButtons(); -} diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_turn_order.h b/cockatrice/src/interface/widgets/dialogs/dlg_turn_order.h deleted file mode 100644 index 44d8203b9..000000000 --- a/cockatrice/src/interface/widgets/dialogs/dlg_turn_order.h +++ /dev/null @@ -1,51 +0,0 @@ -/** - * @file dlg_turn_order.h - * @ingroup GameDialogs - */ -//! \todo Document this file. - -#ifndef DLG_TURN_ORDER_H -#define DLG_TURN_ORDER_H - -#include -#include - -class QLabel; -class QListWidget; -class QPushButton; - -class DlgTurnOrder : public QDialog -{ - Q_OBJECT -private: - QListWidget *listWidget; - QLabel *hintLabel; - QPushButton *moveUpButton; - QPushButton *moveDownButton; - QPushButton *randomizeButton; - bool randomizeRequested; - - void moveSelected(int delta); - void updateMode(); - void updateButtons(); - -private slots: - void actMoveUp(); - void actMoveDown(); - void actRandomize(); - void actOk(); - void selectionChanged(); - -public: - explicit DlgTurnOrder(const QStringList &_playerNames, QWidget *parent = nullptr); - - void retranslateUi(); - - [[nodiscard]] bool isRandomizeRequested() const - { - return randomizeRequested; - } - [[nodiscard]] QStringList order() const; -}; - -#endif diff --git a/cockatrice/src/interface/widgets/general/home_styled_button.cpp b/cockatrice/src/interface/widgets/general/home_styled_button.cpp index a2ac7912d..9c31d3815 100644 --- a/cockatrice/src/interface/widgets/general/home_styled_button.cpp +++ b/cockatrice/src/interface/widgets/general/home_styled_button.cpp @@ -21,8 +21,8 @@ class QWidget; HomeStyledButton::HomeStyledButton(const QString &text, QPair _gradientColors, QWidget *parent) : QPushButton(text, parent), gradientColors(_gradientColors) { - // Keep the font/style-derived height even when the home page has ample space. - setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Fixed); + setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding); + setMinimumHeight(25); setStyleSheet(generateButtonStylesheet(gradientColors)); applyInterfaceDefaultFont(); connect(themeManager, &ThemeManager::fontsChanged, this, &HomeStyledButton::applyInterfaceDefaultFont); diff --git a/cockatrice/src/interface/widgets/general/home_widget.cpp b/cockatrice/src/interface/widgets/general/home_widget.cpp index db4501efc..e6b5bf255 100644 --- a/cockatrice/src/interface/widgets/general/home_widget.cpp +++ b/cockatrice/src/interface/widgets/general/home_widget.cpp @@ -27,11 +27,10 @@ #include #include #include -#include +#include #include #include #include -#include #include #include #include @@ -45,9 +44,9 @@ #include #include #include -#include #include #include +#include #include #include #include @@ -61,7 +60,6 @@ #include #include #include -#include #include #include #include @@ -71,25 +69,20 @@ HomeWidget::HomeWidget(QWidget *parent, TabSupervisor *_tabSupervisor) : QWidget(parent), tabSupervisor(_tabSupervisor), background(themePixmap(QStringLiteral("backgrounds/home"))) { setObjectName("homeWidget"); + layout = new QGridLayout(this); backgroundSourceCard = new CardInfoPictureArtCropWidget(this); gradientColors = determineButtonColor(); - // The launcher keeps its natural size on a large display and stays centred in the - // viewport. On a small window it scrolls, instead of its combined minimum height - // becoming the minimum height of the entire application. - auto *scrollArea = QtUtils::makeScrollArea(this, Qt::ScrollBarAsNeeded, Qt::ScrollBarAsNeeded); - scrollArea->setObjectName("homeActionsScrollArea"); - scrollArea->setFrameShape(QFrame::NoFrame); - scrollArea->setWidgetResizable(false); - scrollArea->setAlignment(Qt::AlignCenter); - // The home background is painted by this widget, so let it show through the viewport. - scrollArea->viewport()->setAutoFillBackground(false); - scrollArea->setWidget(createButtons()); + layout->addWidget(createButtons(), 1, 1, Qt::AlignVCenter | Qt::AlignHCenter); - auto *layout = new QVBoxLayout(this); - layout->addWidget(scrollArea); + layout->setRowStretch(0, 1); + layout->setRowStretch(2, 1); + layout->setColumnStretch(0, 1); + layout->setColumnStretch(2, 1); + + setLayout(layout); cardChangeTimer = new QTimer(this); connect(cardChangeTimer, &QTimer::timeout, this, &HomeWidget::updateRandomCard); @@ -296,10 +289,18 @@ QGroupBox *HomeWidget::createButtons() { QGroupBox *box = new QGroupBox(this); box->setObjectName("homeButtonsBox"); - // The box only stacks the logo and the actions: no frame, and no background of its - // own, so the home background art stays visible. It never shows a title. The rule is - // load-bearing: QScrollArea::setWidget() turns autoFillBackground on for its widget. - box->setStyleSheet("QGroupBox { background: transparent; border: none; }"); + box->setStyleSheet(R"( + QGroupBox { + color: white; /* Title text color */ + background: transparent; + } + + QGroupBox::title { + color: white; + subcontrol-origin: margin; + subcontrol-position: top center; /* or top left / right */ + } +)"); // Family comes from the InterfaceDefault theme role; the 20px size stays caller-side. auto applyBoxFont = [box]() { QFont boxFont = themeManager->appFont(AppFont::InterfaceDefault); @@ -308,12 +309,9 @@ QGroupBox *HomeWidget::createButtons() }; applyBoxFont(); connect(themeManager, &ThemeManager::fontsChanged, this, applyBoxFont); + box->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding); QVBoxLayout *boxLayout = new QVBoxLayout; boxLayout->setAlignment(Qt::AlignHCenter); - // The scroll area is not resizable, so the box has to size itself: SetFixedSize keeps - // it at its content size and re-fixes it whenever the labels or fonts change, which is - // what makes the scroll area's scroll bars follow the content. - boxLayout->setSizeConstraint(QLayout::SetFixedSize); logoLabel = new QLabel; logoLabel->setAlignment(Qt::AlignCenter); diff --git a/cockatrice/src/interface/widgets/general/home_widget.h b/cockatrice/src/interface/widgets/general/home_widget.h index fec1db950..aa4745c82 100644 --- a/cockatrice/src/interface/widgets/general/home_widget.h +++ b/cockatrice/src/interface/widgets/general/home_widget.h @@ -16,6 +16,7 @@ #include #include +class QGridLayout; class QLabel; class CardInfoPictureArtCropWidget; class ExactCard; @@ -45,6 +46,7 @@ public slots: void updateConnectButton(const ClientStatus status); private: + QGridLayout *layout; QTimer *cardChangeTimer; TabSupervisor *tabSupervisor; QPixmap background; diff --git a/cockatrice/src/interface/widgets/server/games_model.cpp b/cockatrice/src/interface/widgets/server/games_model.cpp index 8fed95833..315d51d9e 100644 --- a/cockatrice/src/interface/widgets/server/games_model.cpp +++ b/cockatrice/src/interface/widgets/server/games_model.cpp @@ -185,9 +185,6 @@ QVariant GamesModel::data(const QModelIndex &index, int role) const if (gameentry.share_decklists_on_load()) { result.append(tr("open decklists")); } - if (gameentry.shuffle_players()) { - result.append(tr("random turn order")); - } return result.join(", "); } case Qt::DecorationRole: { diff --git a/cockatrice/src/interface/widgets/server/user/user_card_art_provider.cpp b/cockatrice/src/interface/widgets/server/user/user_card_art_provider.cpp index 6e8fd771a..d84bd5804 100644 --- a/cockatrice/src/interface/widgets/server/user/user_card_art_provider.cpp +++ b/cockatrice/src/interface/widgets/server/user/user_card_art_provider.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -65,6 +66,27 @@ void UserCardArtProvider::requestCardArt(const QString &userName, const QString processQueue(); } +QPixmap UserCardArtProvider::cropCardArt(const QPixmap &fullRes, const ExactCard &card) +{ + QPixmap source = fullRes; + + // Sideways-layout cards (plane, siege/battle, split) store their landscape + // artwork rotated 90° inside a portrait frame. Rotate it upright first so + // the crop below lands on the horizontal art, mirroring the way + // CardInfoPictureWidget displays these cards. + const bool landscape = card.getInfo().getUiAttributes().landscapeOrientation; + source = CardArtUtils::rotateSidewaysLayoutArt(source, card); + + const QSize sz = source.size(); + const int marginX = sz.width() * 0.07; + const int topMargin = landscape ? sz.height() * 0.05 : sz.height() * 0.11; + const int bottomMargin = landscape ? sz.height() * 0.42 : sz.height() * 0.45; + + const QRect artRect(marginX, topMargin, sz.width() - 2 * marginX, sz.height() - topMargin - bottomMargin); + + return source.copy(artRect.intersected(source.rect())); +} + void UserCardArtProvider::insertIntoCache(const QString &key, const QPixmap &pixmap) { if (pixmap.isNull()) { @@ -112,7 +134,7 @@ void UserCardArtProvider::processQueue() // Synchronous hit (already loaded/on disk) if (!fullRes.isNull()) { - insertIntoCache(key, CardArtUtils::cropCardArt(fullRes, card)); + insertIntoCache(key, cropCardArt(fullRes, card)); pending.remove(key); emit cardArtUpdated(userName); @@ -136,7 +158,7 @@ void UserCardArtProvider::processQueue() CardPictureLoader::getPixmap(fullRes, card, QSize(745, 1040)); if (!fullRes.isNull()) { - self->insertIntoCache(key, CardArtUtils::cropCardArt(fullRes, card)); + self->insertIntoCache(key, self->cropCardArt(fullRes, card)); } self->pending.remove(key); @@ -150,4 +172,4 @@ void UserCardArtProvider::processQueue() // Stop here. We'll continue when the async load finishes. return; } -} +} \ No newline at end of file diff --git a/cockatrice/src/interface/widgets/server/user/user_card_art_provider.h b/cockatrice/src/interface/widgets/server/user/user_card_art_provider.h index a7844ca88..ce9225b00 100644 --- a/cockatrice/src/interface/widgets/server/user/user_card_art_provider.h +++ b/cockatrice/src/interface/widgets/server/user/user_card_art_provider.h @@ -10,6 +10,8 @@ #include #include +class ExactCard; + class UserCardArtProvider : public QObject { Q_OBJECT @@ -19,6 +21,7 @@ public: void requestCardArt(const QString &userName, const QString &cardName, const QString &providerId); const QMap &cache() const; + static QPixmap cropCardArt(const QPixmap &fullRes, const ExactCard &card); signals: void cardArtUpdated(const QString &userName); diff --git a/cockatrice/src/interface/widgets/server/user/user_card_settings_dialog.cpp b/cockatrice/src/interface/widgets/server/user/user_card_settings_dialog.cpp index 67fb57e78..f8b57cfc2 100644 --- a/cockatrice/src/interface/widgets/server/user/user_card_settings_dialog.cpp +++ b/cockatrice/src/interface/widgets/server/user/user_card_settings_dialog.cpp @@ -2,7 +2,6 @@ #include "../../../card_picture_loader/card_picture_loader.h" #include "../../cards/art_crop_attribution.h" -#include "../../cards/card_art_utils.h" #include "../../utility/completer_utils.h" #include "card_database_display_model.h" #include "card_database_model.h" @@ -11,6 +10,7 @@ #include "libcockatrice/card/printing/exact_card.h" #include "libcockatrice/card/printing/printing_info.h" #include "libcockatrice/card/set/card_set.h" +#include "user_card_art_provider.h" #include "user_list_painter.h" #include @@ -580,7 +580,7 @@ void UserCardArtSettingsDialog::reloadPreview() return; } - currentPixmap = CardArtUtils::cropCardArt(fullRes, card); + currentPixmap = UserCardArtProvider::cropCardArt(fullRes, card); preview->setPixmap(currentPixmap); preview->setParams(currentParams); diff --git a/cockatrice/src/interface/widgets/tabs/tab_game.cpp b/cockatrice/src/interface/widgets/tabs/tab_game.cpp index 34d6f8a51..cbff850a4 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_game.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_game.cpp @@ -20,7 +20,6 @@ #include "../interface/widgets/cards/card_info_frame_widget.h" #include "../interface/widgets/dialogs/dlg_create_game.h" #include "../interface/widgets/dialogs/dlg_invite_to_game.h" -#include "../interface/widgets/dialogs/dlg_turn_order.h" #include "../interface/widgets/server/game_link.h" #include "../interface/widgets/server/user/user_list_manager.h" #include "../interface/widgets/utility/completer_utils.h" @@ -196,7 +195,6 @@ void TabGame::connectToGameState() { connect(game->getGameState(), &GameState::gameStarted, this, &TabGame::startGame); connect(game->getGameState(), &GameState::gameStopped, this, &TabGame::stopGame); - connect(game->getGameState(), &GameState::hostChanged, this, &TabGame::updateTurnOrderAction); connect(game->getGameState(), &GameState::activePhaseChanged, this, &TabGame::setActivePhase); connect(game->getGameState(), &GameState::activePlayerChanged, this, &TabGame::setActivePlayer); } @@ -265,8 +263,6 @@ void TabGame::connectMessageLogToGameEventHandler() connect(game->getGameEventHandler(), &GameEventHandler::logTurnReversed, messageLog, &MessageLogWidget::logReverseTurn); - connect(game->getGameEventHandler(), &GameEventHandler::logTurnOrderChanged, messageLog, - &MessageLogWidget::logTurnOrderChanged); connect(game->getGameEventHandler(), &GameEventHandler::logConcede, messageLog, &MessageLogWidget::logConcede); connect(game->getGameEventHandler(), &GameEventHandler::logUnconcede, messageLog, &MessageLogWidget::logUnconcede); @@ -377,9 +373,6 @@ void TabGame::retranslateUi() if (aReverseTurn) { aReverseTurn->setText(tr("Reverse turn order")); } - if (aTurnOrder) { - aTurnOrder->setText(tr("&Set turn order...")); - } if (aRemoveLocalArrows) { aRemoveLocalArrows->setText(tr("&Remove all local arrows")); } @@ -636,73 +629,6 @@ void TabGame::actConcede() } } -void TabGame::actTurnOrder() -{ - if (!canSetTurnOrder()) { - return; - } - - QStringList playerNames; - const QList seatOrder = game->getGameState()->getSeatOrder(); - const auto &players = game->getPlayerManager()->getPlayers(); - if (!seatOrder.isEmpty()) { - for (int playerId : seatOrder) { - if (PlayerLogic *player = players.value(playerId, nullptr)) { - playerNames.append(player->getPlayerInfo()->getName()); - } - } - // Include any players missing from the tracked order rather than dropping them. - for (auto it = players.constBegin(); it != players.constEnd(); ++it) { - const QString name = it.value()->getPlayerInfo()->getName(); - if (!playerNames.contains(name)) { - playerNames.append(name); - } - } - } else { - for (PlayerLogic *player : players) { - playerNames.append(player->getPlayerInfo()->getName()); - } - } - - DlgTurnOrder dlg(playerNames, this); - if (dlg.exec() != QDialog::Accepted) { - return; - } - - game->getGameEventHandler()->handleSetPlayerOrder(dlg.order(), dlg.isRandomizeRequested()); -} - -/** - * Whether the local user may change the seating order: only while the game has - * not started, and only as the host or a moderator - the server allows both - * (Server_AbstractParticipant::cmdSetPlayerOrder). - */ -bool TabGame::canSetTurnOrder() const -{ - if (game->getGameMetaInfo()->started()) { - return false; - } - if (game->isHost()) { - return true; - } - const ServerInfo_User *userInfo = tabSupervisor->getUserInfo(); - return userInfo != nullptr && (userInfo->user_level() & ServerInfo_User::IsModerator); -} - -/** - * Shows the turn order action only when it can actually change anything: the - * player must be allowed to set the order and the game must not have started. - * Called whenever either condition may have changed (game start/stop, host - * change). - */ -void TabGame::updateTurnOrderAction() -{ - if (aTurnOrder == nullptr) { - return; - } - aTurnOrder->setVisible(canSetTurnOrder()); -} - /** * Confirms the leave game and sends the leave game command, if applicable. * @@ -914,15 +840,6 @@ void TabGame::processPlayerLeave(PlayerLogic *leavingPlayer) } } - // A shuffled local player has their seat changed, so the deck view container - // keyed by their previous id must be cleaned up alongside the leaving player. - int leavingPlayerId = leavingPlayer->getPlayerInfo()->getId(); - if (deckViewContainers.contains(leavingPlayerId)) { - TabbedDeckViewContainer *deckView = deckViewContainers.take(leavingPlayerId); - deckViewContainerLayout->removeWidget(deckView); - deckView->deleteLater(); - } - scene->removePlayer(leavingPlayer); } @@ -1031,7 +948,6 @@ void TabGame::startGame(bool _resuming) aConcede->setText(tr("&Concede")); aConcede->setEnabled(true); } - updateTurnOrderAction(); } void TabGame::stopGame() @@ -1054,7 +970,6 @@ void TabGame::stopGame() aConcede->setText(tr("&Concede")); aConcede->setEnabled(false); } - updateTurnOrderAction(); } void TabGame::closeGame() @@ -1179,9 +1094,6 @@ void TabGame::createMenuItems() connect(aNextTurn, &QAction::triggered, game->getGameEventHandler(), &GameEventHandler::handleNextTurn); aReverseTurn = new QAction(this); connect(aReverseTurn, &QAction::triggered, game->getGameEventHandler(), &GameEventHandler::handleReverseTurn); - aTurnOrder = new QAction(this); - connect(aTurnOrder, &QAction::triggered, this, &TabGame::actTurnOrder); - updateTurnOrderAction(); aRemoveLocalArrows = new QAction(this); connect(aRemoveLocalArrows, &QAction::triggered, this, &TabGame::actRemoveLocalArrows); aRotateViewCW = new QAction(this); @@ -1232,7 +1144,6 @@ void TabGame::createMenuItems() gameMenu->addAction(aNextTurn); aNextTurn->setObjectName("aNextTurn"); gameMenu->addAction(aReverseTurn); - gameMenu->addAction(aTurnOrder); gameMenu->addSeparator(); gameMenu->addAction(aRemoveLocalArrows); gameMenu->addAction(aRotateViewCW); @@ -1261,7 +1172,6 @@ void TabGame::createReplayMenuItems() aNextPhaseAction = nullptr; aNextTurn = nullptr; aReverseTurn = nullptr; - aTurnOrder = nullptr; aRemoveLocalArrows = nullptr; aRotateViewCW = nullptr; aRotateViewCCW = nullptr; @@ -1392,7 +1302,6 @@ void TabGame::createPlayAreaWidget(bool bReplay) scene = new GameScene(phasesToolbar, this); connect(game->getPlayerManager(), &PlayerManager::playerConceded, scene, &GameScene::rearrange); connect(game->getPlayerManager(), &PlayerManager::playerCountChanged, scene, &GameScene::rearrange); - connect(game->getGameState(), &GameState::seatOrderChanged, scene, &GameScene::rearrange); connect(scene, &GameScene::arrowDeletionRequested, game->getGameEventHandler(), &GameEventHandler::handleArrowDeletion); connect(game->getGameEventHandler(), &GameEventHandler::arrowDeleted, scene, &GameScene::deleteArrow); diff --git a/cockatrice/src/interface/widgets/tabs/tab_game.h b/cockatrice/src/interface/widgets/tabs/tab_game.h index 412529dc2..7bc52202f 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_game.h +++ b/cockatrice/src/interface/widgets/tabs/tab_game.h @@ -94,8 +94,7 @@ private: QMenu *gameMenu, *viewMenu; TearOffMenu *phasesMenu; QAction *aGameInfo, *aConcede, *aCopyGameLink, *aLeaveGame, *aNextPhase, *aNextPhaseAction, *aNextTurn, - *aReverseTurn, *aTurnOrder, *aRemoveLocalArrows, *aRotateViewCW, *aRotateViewCCW, *aResetLayout, - *aResetReplayLayout; + *aReverseTurn, *aRemoveLocalArrows, *aRotateViewCW, *aRotateViewCCW, *aResetLayout, *aResetReplayLayout; QAction *aFocusChat; QAction *aInviteToGame = nullptr; QList phaseActions; @@ -130,7 +129,6 @@ private: PlayerLogic *setActivePlayer(int id); void setActivePhase(int phase); - bool canSetTurnOrder() const; void createMenuItems(); void createReplayMenuItems(); void createViewMenuItems(); @@ -164,8 +162,6 @@ private slots: void actInviteToGame(); void actConcede(); void actCopyGameLink(); - void actTurnOrder(); - void updateTurnOrderAction(); void actRemoveLocalArrows(); void actRotateViewCW(); void actRotateViewCCW(); diff --git a/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_3.cpp b/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_3.cpp index 8d5735db7..d40c1c063 100644 --- a/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_3.cpp +++ b/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_3.cpp @@ -18,7 +18,6 @@ #include #include #include -#include #include #include #include @@ -500,7 +499,7 @@ bool CockatriceXml3Parser::saveToFile(FormatRulesNameMap _formats, { Q_UNUSED(_formats); - QSaveFile file(fileName); + QFile file(fileName); if (!file.open(QIODevice::WriteOnly)) { return false; } @@ -540,5 +539,5 @@ bool CockatriceXml3Parser::saveToFile(FormatRulesNameMap _formats, xml.writeEndElement(); // cockatrice_carddatabase xml.writeEndDocument(); - return !xml.hasError() && file.commit(); + return true; } diff --git a/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.cpp b/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.cpp index 3965b80f0..3ee1e808e 100644 --- a/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.cpp +++ b/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.cpp @@ -18,7 +18,6 @@ #include #include #include -#include #include #include #include @@ -683,7 +682,7 @@ bool CockatriceXml4Parser::saveToFile(FormatRulesNameMap _formats, const QString &sourceUrl, const QString &sourceVersion) { - QSaveFile file(fileName); + QFile file(fileName); if (!file.open(QIODevice::WriteOnly)) { return false; } @@ -731,5 +730,5 @@ bool CockatriceXml4Parser::saveToFile(FormatRulesNameMap _formats, xml.writeEndElement(); // cockatrice_carddatabase xml.writeEndDocument(); - return !xml.hasError() && file.commit(); + return true; } diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp index b357b5801..2cd5ef914 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp @@ -340,17 +340,7 @@ bool DeckList::saveToStream_Plain(QTextStream &stream, bool prefixSideboardCards } }; - // 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); - } - }); + forEachCard(writeToStream); return true; } diff --git a/libcockatrice_game/libcockatrice/game/game_event_handler.cpp b/libcockatrice_game/libcockatrice/game/game_event_handler.cpp index 7ef502400..856475dfa 100644 --- a/libcockatrice_game/libcockatrice/game/game_event_handler.cpp +++ b/libcockatrice_game/libcockatrice/game/game_event_handler.cpp @@ -21,7 +21,6 @@ #include #include #include -#include #include #include #include @@ -39,7 +38,6 @@ #include #include #include -#include #include #include #include @@ -52,7 +50,6 @@ #include #include #include -#include #include #include #include @@ -192,9 +189,6 @@ void GameEventHandler::processGameEventContainer(const GameEventContainer &cont, case GameEvent::REVERSE_TURN: eventReverseTurn(event.GetExtension(Event_ReverseTurn::ext), playerId, context); break; - case GameEvent::PLAYER_ORDER_CHANGED: - eventPlayerOrderChanged(event.GetExtension(Event_PlayerOrderChanged::ext), playerId, context); - break; default: { PlayerLogic *player = game->getPlayerManager()->getPlayers().value(playerId, 0); @@ -221,16 +215,6 @@ void GameEventHandler::handleReverseTurn() sendGameCommand(Command_ReverseTurn()); } -void GameEventHandler::handleSetPlayerOrder(const QList &playerNames, bool randomize) -{ - Command_SetPlayerOrder command; - command.set_randomize(randomize); - for (const QString &name : playerNames) { - command.add_player_names(name.toStdString()); - } - sendGameCommand(command); -} - void GameEventHandler::handleActiveLocalPlayerConceded() { sendGameCommand(Command_Concede()); @@ -393,27 +377,6 @@ void GameEventHandler::eventGameStateChanged(const Event_GameStateChanged &event { const int playerListSize = event.player_list_size(); - if (event.has_local_player_id()) { - game->getPlayerManager()->setLocalPlayerId(event.local_player_id()); - } - - QSet authoritativePlayerIds; - for (int i = 0; i < playerListSize; ++i) { - const ServerInfo_PlayerProperties &prop = event.player_list(i).properties(); - if (!prop.spectator()) { - authoritativePlayerIds.insert(prop.player_id()); - } - } - - // Players whose seats were shuffled away from the previous game are no longer - // part of the authoritative list and must be removed. - for (int stalePlayerId : game->getPlayerManager()->getPlayers().keys()) { - if (!authoritativePlayerIds.contains(stalePlayerId)) { - game->getPlayerManager()->removePlayer(stalePlayerId); - emit playerLeft(stalePlayerId); - } - } - QVector>> opponentDecksToDisplay; for (int i = 0; i < playerListSize; ++i) { @@ -458,19 +421,6 @@ void GameEventHandler::eventGameStateChanged(const Event_GameStateChanged &event emit remotePlayersDecksSelected(opponentDecksToDisplay); - // The server emits player_list in seat order, so record it to lay out the - // table by seating rather than by player id. - QList seatOrder; - for (int i = 0; i < playerListSize; ++i) { - const ServerInfo_PlayerProperties &prop = event.player_list(i).properties(); - if (!prop.spectator()) { - seatOrder.append(prop.player_id()); - } - } - if (!seatOrder.isEmpty()) { - game->getGameState()->setSeatOrder(seatOrder); - } - game->getGameState()->setGameTime(event.seconds_elapsed()); if (event.game_started() && !game->getGameMetaInfo()->started()) { @@ -675,33 +625,6 @@ void GameEventHandler::eventReverseTurn(const Event_ReverseTurn &event, emit logTurnReversed(player, event.reversed()); } -void GameEventHandler::eventPlayerOrderChanged(const Event_PlayerOrderChanged &event, - int /*eventPlayerId*/, - const GameEventContext & /*context*/) -{ - QStringList playerNames; - for (const auto &name : event.player_names()) { - playerNames.append(QString::fromStdString(name)); - } - - // Track the seating by stable player id so the table layout and the turn - // order dialog follow the shuffled order instead of id order. - QList seatOrder; - const auto &players = game->getPlayerManager()->getPlayers(); - for (const QString &name : playerNames) { - for (auto it = players.constBegin(); it != players.constEnd(); ++it) { - if (it.value() && it.value()->getPlayerInfo()->getName() == name) { - seatOrder.append(it.key()); - break; - } - } - } - if (!seatOrder.isEmpty()) { - game->getGameState()->setSeatOrder(seatOrder); - } - emit logTurnOrderChanged(playerNames, event.randomized()); -} - void GameEventHandler::eventGameHostChanged(const Event_GameHostChanged & /*event*/, int eventPlayerId, const GameEventContext & /*context*/) diff --git a/libcockatrice_game/libcockatrice/game/game_event_handler.h b/libcockatrice_game/libcockatrice/game/game_event_handler.h index bfcf355b0..f7b11780b 100644 --- a/libcockatrice_game/libcockatrice/game/game_event_handler.h +++ b/libcockatrice_game/libcockatrice/game/game_event_handler.h @@ -51,7 +51,6 @@ class Event_GameSay; class Event_Kicked; class Event_ReverseTurn; class Event_Ping; -class Event_PlayerOrderChanged; class ServerInfo_PlayerProperties; namespace google { @@ -114,16 +113,6 @@ public: /** @brief Request reversing the current turn order. */ void handleReverseTurn(); - /** - * @brief Request a new seating / turn order. - * - * Only the game host (or a moderator) may do this before the game starts. - * - * @param playerNames Ordered list of player names. Ignored when randomize is true. - * @param randomize Ask the server to shuffle the seats instead. - */ - void handleSetPlayerOrder(const QList &playerNames, bool randomize); - /** @brief Concede the game for the currently active local player. */ void handleActiveLocalPlayerConceded(); @@ -327,10 +316,6 @@ public: /** @brief Handle a turn reversal event. */ void eventReverseTurn(const Event_ReverseTurn &event, int eventPlayerId, const GameEventContext &context); - /** @brief Handle a change of the seating / turn order. */ - void - eventPlayerOrderChanged(const Event_PlayerOrderChanged &event, int eventPlayerId, const GameEventContext &context); - /** @brief Handle ping / latency updates. */ void eventPing(const Event_Ping &event, int eventPlayerId, const GameEventContext &context); @@ -458,7 +443,6 @@ signals: void logLeave(PlayerLogic *player, QString reason); void logKicked(); void logTurnReversed(PlayerLogic *player, bool reversed); - void logTurnOrderChanged(QStringList playerNames, bool randomized); void logGameClosed(); void logActivePlayer(PlayerLogic *activePlayer); void logActivePhaseChanged(int activePhase); diff --git a/libcockatrice_game/libcockatrice/game/game_state.h b/libcockatrice_game/libcockatrice/game/game_state.h index 4d02019ce..55784fb91 100644 --- a/libcockatrice_game/libcockatrice/game/game_state.h +++ b/libcockatrice_game/libcockatrice/game/game_state.h @@ -33,11 +33,7 @@ public: void setHostId(int _hostId) { - if (hostId == _hostId) { - return; - } hostId = _hostId; - emit hostChanged(hostId); } QList getClients() const @@ -116,19 +112,6 @@ public: return hostId; } - void setSeatOrder(const QList &order) - { - if (seatOrder != order) { - seatOrder = order; - emit seatOrderChanged(seatOrder); - } - } - - QList getSeatOrder() const - { - return seatOrder; - } - int getSecondsElapsed() const { return secondsElapsed; @@ -140,8 +123,6 @@ signals: void gameStopped(); void activePhaseChanged(int activePhase); void activePlayerChanged(int playerId); - void hostChanged(int hostId); - void seatOrderChanged(const QList &seatOrder); public slots: void incrementGameTime(); @@ -161,9 +142,6 @@ private: int currentPhase; int activePlayer; bool gameClosed; - //! Player ids in seating (turn) order, first to play first. Empty until the - //! server sends an order, in which case players are laid out in id order. - QList seatOrder; }; #endif // COCKATRICE_GAME_STATE_H diff --git a/libcockatrice_game/libcockatrice/game/player/player_manager.h b/libcockatrice_game/libcockatrice/game/player/player_manager.h index 950574112..c95bbbb96 100644 --- a/libcockatrice_game/libcockatrice/game/player/player_manager.h +++ b/libcockatrice_game/libcockatrice/game/player/player_manager.h @@ -57,11 +57,6 @@ public: return localPlayerId; } - void setLocalPlayerId(int _localPlayerId) - { - localPlayerId = _localPlayerId; - } - [[nodiscard]] const QMap &getPlayers() const { return players; diff --git a/libcockatrice_interfaces/libcockatrice/interfaces/interface_game_settings_provider.h b/libcockatrice_interfaces/libcockatrice/interfaces/interface_game_settings_provider.h index 00763d031..2b1ddf8ec 100644 --- a/libcockatrice_interfaces/libcockatrice/interfaces/interface_game_settings_provider.h +++ b/libcockatrice_interfaces/libcockatrice/interfaces/interface_game_settings_provider.h @@ -20,7 +20,6 @@ public: [[nodiscard]] virtual bool getCreateGameAsSpectator() const = 0; [[nodiscard]] virtual int getDefaultStartingLifeTotal() const = 0; [[nodiscard]] virtual bool getShareDecklistsOnLoad() const = 0; - [[nodiscard]] virtual bool getShufflePlayers() const = 0; [[nodiscard]] virtual bool getRememberGameSettings() const = 0; [[nodiscard]] virtual bool getLocalGameRememberSettings() const = 0; [[nodiscard]] virtual int getLocalGameMaxPlayers() const = 0; 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 f3c4545d7..617324fe3 100644 --- a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp +++ b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp @@ -746,8 +746,7 @@ 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, activeGroupCriteria == DeckListModelGroupCriteria::MANA_COST && lastKnownColumn != 0); + (void)DeckListModelCustomZones::sortWithCustomZonesLast(root, node, order); for (int i = node->size() - 1; i >= 0; --i) { if (auto *subNode = dynamic_cast(node->at(i))) { @@ -760,8 +759,7 @@ 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, activeGroupCriteria == DeckListModelGroupCriteria::MANA_COST && lastKnownColumn != 0); + const auto mapping = DeckListModelCustomZones::sortWithCustomZonesLast(root, node, order); 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 523708cbd..419c38b04 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,8 +8,6 @@ #include #include #include -#include -#include #include namespace DeckListModelCustomZones @@ -113,7 +111,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, bool numericGroups) +QList> boardSort(InnerDecklistNode *node, Qt::SortOrder order) { const QVector> sortResult = node->sort(order); @@ -134,27 +132,6 @@ QList> boardSort(InnerDecklistNode *node, Qt::SortOrder order, b } } - 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]); @@ -170,11 +147,10 @@ QList> boardSort(InnerDecklistNode *node, Qt::SortOrder order, b } // namespace -QList> -sortWithCustomZonesLast(InnerDecklistNode *root, InnerDecklistNode *node, Qt::SortOrder order, bool numericGroups) +QList> sortWithCustomZonesLast(InnerDecklistNode *root, InnerDecklistNode *node, Qt::SortOrder order) { const bool isBoardZone = (node != root) && (node->getParent() == root); - return isBoardZone ? boardSort(node, order, numericGroups) : plainSort(node, order); + return isBoardZone ? boardSort(node, order) : 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 d3f477450..53bdeaa1e 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,15 +91,10 @@ 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, - bool numericGroups = false); +[[nodiscard]] QList> +sortWithCustomZonesLast(InnerDecklistNode *root, InnerDecklistNode *node, Qt::SortOrder order); } // namespace DeckListModelCustomZones diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/game_config.h b/libcockatrice_network/libcockatrice/network/server/remote/game/game_config.h index 0661bd2aa..baf946632 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/game_config.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/game_config.h @@ -21,7 +21,6 @@ struct GameConfig bool spectatorsSeeEverything = true; int startingLifeTotal = 20; bool shareDecklistsOnLoad = true; - bool shufflePlayers = false; }; #endif diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_participant.cpp b/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_participant.cpp index dcd14d14c..ff8ea14f3 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_participant.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_participant.cpp @@ -43,7 +43,6 @@ #include #include #include -#include #include #include #include @@ -435,33 +434,6 @@ Response::ResponseCode Server_AbstractParticipant::cmdReverseTurn(const Command_ return Response::RespOk; } -Response::ResponseCode Server_AbstractParticipant::cmdSetPlayerOrder(const Command_SetPlayerOrder &cmd, - ResponseContainer & /*rc*/, - GameEventStorage & /*ges*/) -{ - if ((game->getHostId() != playerId) && !(userInfo->user_level() & ServerInfo_User::IsModerator)) { - return Response::RespFunctionNotAllowed; - } - - if (game->getGameStarted()) { - return Response::RespContextError; - } - - if (cmd.randomize()) { - game->shufflePlayerSeats(); - } else { - QList playerNames; - for (int i = 0; i < cmd.player_names_size(); ++i) { - playerNames.append(QString::fromStdString(cmd.player_names(i))); - } - game->reorderPlayerSeats(playerNames); - } - - // Clients learn the new seating from the Event_PlayerOrderChanged that - // reseatPlayers broadcasts; no full game state resend is needed for it. - return Response::RespOk; -} - Response::ResponseCode Server_AbstractParticipant::processGameCommand(const GameCommand &command, ResponseContainer &rc, GameEventStorage &ges) { @@ -571,9 +543,6 @@ Server_AbstractParticipant::processGameCommand(const GameCommand &command, Respo case GameCommand::SET_PLAYMAT: return cmdSetPlaymat(command.GetExtension(Command_SetPlaymat::ext), rc, ges); break; - case GameCommand::SET_PLAYER_ORDER: - return cmdSetPlayerOrder(command.GetExtension(Command_SetPlayerOrder::ext), rc, ges); - break; default: return Response::RespInvalidCommand; } diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_participant.h b/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_participant.h index 316b9727c..b6d4a8025 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_participant.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_participant.h @@ -53,7 +53,6 @@ class Command_DeckSelect; class Command_SetSideboardLock; class Command_ChangeZoneProperties; class Command_SetPlaymat; -class Command_SetPlayerOrder; class Server_AbstractParticipant : public Server_ArrowTarget, public ServerInfo_User_Container { @@ -90,10 +89,6 @@ public: { return playerId; } - void setPlayerId(int _playerId) - { - playerId = _playerId; - } bool isSpectator() const { return roleInfo.isSpectator; @@ -178,8 +173,6 @@ public: virtual Response::ResponseCode cmdReverseTurn(const Command_ReverseTurn & /*cmd*/, ResponseContainer & /*rc*/, GameEventStorage &ges); virtual Response::ResponseCode - cmdSetPlayerOrder(const Command_SetPlayerOrder &cmd, ResponseContainer &rc, GameEventStorage &ges); - virtual Response::ResponseCode cmdChangeZoneProperties(const Command_ChangeZoneProperties &cmd, ResponseContainer &rc, GameEventStorage &ges); Response::ResponseCode processGameCommand(const GameCommand &command, ResponseContainer &rc, GameEventStorage &ges); diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.cpp b/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.cpp index e2724523e..49db999f6 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.cpp @@ -73,13 +73,11 @@ #include #include #include -#include #include #include #include #include #include -#include #include #include #include @@ -93,8 +91,8 @@ Server_Game::Server_Game(const GameConfig &config, Server_Room *_room) spectatorsAllowed(config.spectatorsAllowed), spectatorsNeedPassword(config.spectatorsNeedPassword), spectatorsCanTalk(config.spectatorsCanTalk), spectatorsSeeEverything(config.spectatorsSeeEverything), startingLifeTotal(config.startingLifeTotal), shareDecklistsOnLoad(config.shareDecklistsOnLoad), - shufflePlayers(config.shufflePlayers), inactivityCounter(0), startTimeOfThisGame(0), secondsElapsed(0), - firstGameStarted(false), turnOrderReversed(false), startTime(QDateTime::currentDateTime()), pingClock(nullptr), + inactivityCounter(0), startTimeOfThisGame(0), secondsElapsed(0), firstGameStarted(false), + turnOrderReversed(false), startTime(QDateTime::currentDateTime()), pingClock(nullptr), deckValidationStrategy(new Server_DefaultDeckValidationStrategy), lifecycleStrategy(new Server_DefaultLifecycleStrategy), matchResultStrategy(new Server_NullMatchResultStrategy), gameMutex() @@ -294,32 +292,6 @@ int Server_Game::getSpectatorCount() const return result; } -QList Server_Game::getParticipantsInSeatOrder() const -{ - // Players in seat order first (fall back to id order for seats predating - // seatOrder), then spectators. Player ids are stable; only the ordering - // changes on shuffle/reorder. - QList ordered; - ordered.reserve(participants.size()); - for (int id : seatOrder) { - auto *participant = participants.value(id, nullptr); - if (participant && !participant->isSpectator() && !ordered.contains(participant)) { - ordered.append(participant); - } - } - for (auto *participant : participants.values()) { - if (!participant->isSpectator() && !ordered.contains(participant)) { - ordered.append(participant); - } - } - for (auto *participant : participants.values()) { - if (participant->isSpectator()) { - ordered.append(participant); - } - } - return ordered; -} - void Server_Game::createGameStateChangedEvent(Event_GameStateChanged *event, Server_AbstractParticipant *recipient, bool omniscient, @@ -334,24 +306,16 @@ void Server_Game::createGameStateChangedEvent(Event_GameStateChanged *event, event->set_game_started(false); } - // Emit the player list in seat order so every client (including old ones - // that ignore Event_PlayerOrderChanged) sees the current seating. Player - // ids are stable; only this ordering changes on shuffle/reorder. - QList ordered = getParticipantsInSeatOrder(); - for (Server_AbstractParticipant *participant : ordered) { + for (Server_AbstractParticipant *participant : participants.values()) { participant->getInfo(event->add_player_list(), recipient, omniscient, withUserInfo); } - - if (recipient != nullptr) { - event->set_local_player_id(recipient->getPlayerId()); - } } void Server_Game::sendGameStateToPlayers() { // game state information for replay and omniscient spectators Event_GameStateChanged omniscientEvent; - createGameStateChangedEvent(&omniscientEvent, nullptr, true, true); + createGameStateChangedEvent(&omniscientEvent, nullptr, true, false); GameEventContainer *replayCont = prepareGameEvent(omniscientEvent, -1); replayCont->set_seconds_elapsed(secondsElapsed - startTimeOfThisGame); @@ -363,7 +327,7 @@ void Server_Game::sendGameStateToPlayers() // the data we used for the replay. All spectators are equal, so we don't need to make a createGameStateChangedEvent // call for each one. Event_GameStateChanged spectatorNormalEvent; - createGameStateChangedEvent(&spectatorNormalEvent, nullptr, false, true); + createGameStateChangedEvent(&spectatorNormalEvent, nullptr, false, false); // send game state info to clients according to their role in the game for (auto *participant : participants.values()) { @@ -376,7 +340,7 @@ void Server_Game::sendGameStateToPlayers() } } else { Event_GameStateChanged event; - createGameStateChangedEvent(&event, participant, participant->isJudge(), true); + createGameStateChangedEvent(&event, participant, participant->isJudge(), false); gec = prepareGameEvent(event, -1); } @@ -416,11 +380,6 @@ void Server_Game::doStartGameIfReady(bool forceStartGame) return; } - if (shufflePlayers && players.size() > 1) { - shufflePlayerSeats(); - players = getPlayers(); - } - for (Server_AbstractPlayer *player : players.values()) { player->setupZones(); } @@ -474,116 +433,6 @@ void Server_Game::startGameIfReady(bool forceStartGame) emit sigStartGameIfReady(forceStartGame); } -void Server_Game::shufflePlayerSeats() -{ - QMutexLocker locker(&gameMutex); - - // Collect players in current seat order (fall back to id order for seats - // predating seatOrder). Spectators keep their seats. - QList players; - for (int id : seatOrder) { - auto *participant = participants.value(id, nullptr); - if (participant && !participant->isSpectator()) { - players.append(participant); - } - } - for (auto *participant : participants.values()) { - if (!participant->isSpectator() && !players.contains(participant)) { - players.append(participant); - } - } - QList spectators; - for (auto *participant : participants.values()) { - if (participant->isSpectator()) { - spectators.append(participant); - } - } - - // Fisher-Yates shuffle of the players. - QList shuffledPlayers; - while (!players.isEmpty()) { - const int index = rng->rand(0, players.size() - 1); - shuffledPlayers.append(players.takeAt(index)); - } - - reseatPlayers(shuffledPlayers + spectators, true); -} - -void Server_Game::reorderPlayerSeats(const QList &orderedNames) -{ - QMutexLocker locker(&gameMutex); - - // Collect players in current seat order (fall back to id order for seats - // predating seatOrder). Spectators keep their seats. - QList players; - for (int id : seatOrder) { - auto *participant = participants.value(id, nullptr); - if (participant && !participant->isSpectator()) { - players.append(participant); - } - } - for (auto *participant : participants.values()) { - if (!participant->isSpectator() && !players.contains(participant)) { - players.append(participant); - } - } - QList spectators; - for (auto *participant : participants.values()) { - if (participant->isSpectator()) { - spectators.append(participant); - } - } - - // Order the players according to the given names, appending any player that - // was not listed (should not happen) in their current seat order. - QList orderedPlayers; - QSet placed; - for (const QString &name : orderedNames) { - for (auto *participant : players) { - if (!placed.contains(participant) && (QString::fromStdString(participant->getUserInfo()->name()) == name)) { - orderedPlayers.append(participant); - placed.insert(participant); - break; - } - } - } - for (auto *participant : players) { - if (!placed.contains(participant)) { - orderedPlayers.append(participant); - } - } - - reseatPlayers(orderedPlayers + spectators, false); -} - -void Server_Game::reseatPlayers(const QList &orderedParticipants, bool randomized) -{ - QMutexLocker locker(&gameMutex); - - // Keep player ids stable and only reorder the seating/turn order. Changing - // ids would break old clients (stale localPlayerId from Event_GameJoined - // duplicates their own seat as an opponent), invalidate the home-server - // per-user game/seat mapping used for ISL command routing, and orphan the - // host id. All of those maps stay valid because ids never change here. - // The current host therefore keeps its id and needs no re-key tracking. - seatOrder.clear(); - for (auto *participant : orderedParticipants) { - if (!participant->isSpectator()) { - seatOrder.append(participant->getPlayerId()); - } - } - - // Broadcast the new seating order so that every client can display it. - Event_PlayerOrderChanged event; - for (auto *participant : orderedParticipants) { - if (!participant->isSpectator()) { - event.add_player_names(QString::fromStdString(participant->getUserInfo()->name()).toStdString()); - } - } - event.set_randomized(randomized); - sendGameEventContainer(prepareGameEvent(event, -1)); -} - void Server_Game::stopGameIfFinished() { QMutexLocker locker(&gameMutex); @@ -714,7 +563,6 @@ void Server_Game::addPlayer(Server_AbstractUserInterface *userInterface, allSpectatorsEver.insert(playerName); } else { allPlayersEver.insert(playerName); - seatOrder.append(newParticipant->getPlayerId()); // if the original creator of the game joins, give them host status back //! \todo Transferring host to spectators has side effects. @@ -744,7 +592,6 @@ void Server_Game::removeParticipant(Server_AbstractParticipant *participant, Eve room->getServer()->removePersistentPlayer(QString::fromStdString(participant->getUserInfo()->name()), room->getId(), gameId, participant->getPlayerId()); participants.remove(participant->getPlayerId()); - seatOrder.removeAll(participant->getPlayerId()); bool spectator = participant->isSpectator(); GameEventStorage ges; @@ -764,20 +611,10 @@ void Server_Game::removeParticipant(Server_AbstractParticipant *participant, Eve participant->prepareDestroy(); if (playerHost) { - // Pass host to the next player in seat order so the game always keeps - // a host while players remain. int newHostId = -1; - for (int id : seatOrder) { - if (participants.contains(id)) { - newHostId = id; - break; - } - } - if (newHostId == -1) { - for (auto *otherPlayer : getPlayers().values()) { - newHostId = otherPlayer->getPlayerId(); - break; - } + for (auto *otherPlayer : getPlayers().values()) { + newHostId = otherPlayer->getPlayerId(); + break; } if (newHostId != -1) { hostId = newHostId; @@ -934,20 +771,7 @@ void Server_Game::nextTurn() } auto players = getPlayers(); - // Advance in seat order, not id order, so shuffles/reorders change the - // turn sequence while ids stay stable. - QList keys; - keys.reserve(players.size()); - for (int id : seatOrder) { - if (players.contains(id)) { - keys.append(id); - } - } - for (int id : players.keys()) { - if (!keys.contains(id)) { - keys.append(id); - } - } + const QList keys = players.keys(); int listPos = -1; if (activePlayer != -1) { listPos = keys.indexOf(activePlayer); @@ -997,7 +821,7 @@ void Server_Game::createGameJoinedEvent(Server_AbstractParticipant *joiningParti event2.set_active_phase(activePhase); bool omniscient = (joiningParticipant->isSpectator() && spectatorsSeeEverything) || joiningParticipant->isJudge(); - for (auto *participant : getParticipantsInSeatOrder()) { + for (auto *participant : participants.values()) { participant->getInfo(event2.add_player_list(), joiningParticipant, omniscient, true); } @@ -1126,7 +950,6 @@ void Server_Game::getInfo(ServerInfo_Game &result) const result.set_spectators_can_chat(spectatorsCanTalk); result.set_spectators_omniscient(spectatorsSeeEverything); result.set_share_decklists_on_load(shareDecklistsOnLoad); - result.set_shuffle_players(shufflePlayers); result.set_spectators_count(getSpectatorCount()); result.set_start_time(startTime.toSecsSinceEpoch()); } diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.h b/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.h index 23ba96598..363c0ca94 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.h @@ -74,11 +74,6 @@ private: int hostId; ServerInfo_User *creatorInfo; QMap participants; - // Seating/turn order as an ordered list of player ids. Player ids themselves - // are stable for the lifetime of the participant; shuffling or reordering - // only permutes this list so old clients, ISL routes and persistent - // references (all keyed by player id) keep working. - QList seatOrder; QSet allPlayersEver, allSpectatorsEver; bool gameStarted; bool gameClosed; @@ -95,7 +90,6 @@ private: bool spectatorsSeeEverything; int startingLifeTotal; bool shareDecklistsOnLoad; - bool shufflePlayers; int inactivityCounter; int startTimeOfThisGame, secondsElapsed; bool firstGameStarted; @@ -115,8 +109,6 @@ private: Server_AbstractParticipant *recipient, bool omniscient, bool withUserInfo); - QList getParticipantsInSeatOrder() const; - void reseatPlayers(const QList &orderedParticipants, bool randomized); void storeGameInformation(); bool canSeePrivateEventsFor(const Server_AbstractParticipant *participant, int privatePlayerId) const; signals: @@ -157,10 +149,6 @@ public: { return participants; } - QList getSeatOrder() const - { - return seatOrder; - } int getGameId() const { return gameId; @@ -201,10 +189,6 @@ public: { return shareDecklistsOnLoad; } - bool getShufflePlayers() const - { - return shufflePlayers; - } Response::ResponseCode checkJoin(ServerInfo_User *user, const QString &_password, const RoleInfo &roleInfo, bool overrideRestrictions); bool containsUser(const QString &userName) const; @@ -218,8 +202,6 @@ public: bool kickParticipant(int playerId); void startGameIfReady(bool forceStartGame); void stopGameIfFinished(); - void shufflePlayerSeats(); - void reorderPlayerSeats(const QList &orderedNames); int getActivePlayer() const { return activePlayer; diff --git a/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp b/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp index 78b9d955f..f9cadc9aa 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp @@ -1116,7 +1116,6 @@ Server_ProtocolHandler::cmdCreateGame(const Command_CreateGame &cmd, Server_Room int startingLifeTotal = cmd.has_starting_life_total() ? cmd.starting_life_total() : 20; bool shareDecklistsOnLoad = cmd.has_share_decklists_on_load() ? cmd.share_decklists_on_load() : false; - bool shufflePlayers = cmd.has_shuffle_players() ? cmd.shuffle_players() : false; const int gameId = databaseInterface->getNextGameId(); if (gameId == -1) { @@ -1138,8 +1137,7 @@ Server_ProtocolHandler::cmdCreateGame(const Command_CreateGame &cmd, Server_Room .spectatorsCanTalk = cmd.spectators_can_talk(), .spectatorsSeeEverything = cmd.spectators_see_everything(), .startingLifeTotal = startingLifeTotal, - .shareDecklistsOnLoad = shareDecklistsOnLoad, - .shufflePlayers = shufflePlayers}; + .shareDecklistsOnLoad = shareDecklistsOnLoad}; auto *game = new Server_Game(config, room); diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt b/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt index 5fba2631d..059e83096 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt @@ -66,7 +66,6 @@ set(PROTO_FILES command_set_card_attr.proto command_set_card_counter.proto command_set_counter.proto - command_set_player_order.proto command_set_playmat.proto command_set_sideboard_lock.proto command_set_sideboard_plan.proto @@ -111,7 +110,6 @@ set(PROTO_FILES event_list_rooms.proto event_move_card.proto event_notify_user.proto - event_player_order_changed.proto event_player_properties_changed.proto event_remove_from_list.proto event_remove_messages.proto diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/command_set_player_order.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/command_set_player_order.proto deleted file mode 100644 index 97e27038f..000000000 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/command_set_player_order.proto +++ /dev/null @@ -1,15 +0,0 @@ -syntax = "proto2"; -import "game_commands.proto"; -message Command_SetPlayerOrder { - extend GameCommand { - optional Command_SetPlayerOrder ext = 1206; - } - - // The desired seating order, expressed as player names. Only used when - // randomize is false. - repeated string player_names = 1; - - // When set, the server shuffles the seating order instead of using - // player_names. - optional bool randomize = 2; -} diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/event_game_state_changed.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/event_game_state_changed.proto index bbf2d0f98..5c8aee3fe 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/event_game_state_changed.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/event_game_state_changed.proto @@ -24,9 +24,4 @@ message Event_GameStateChanged { // the amount of seconds since the game started optional uint32 seconds_elapsed = 5; - - // the id of the player this event is addressed to. Used to re-identify the - // local player after seats have been shuffled. Only set for events sent to - // a specific participant, never for omniscient/replay events. - optional sint32 local_player_id = 6; } diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/event_player_order_changed.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/event_player_order_changed.proto deleted file mode 100644 index d119398b7..000000000 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/event_player_order_changed.proto +++ /dev/null @@ -1,13 +0,0 @@ -syntax = "proto2"; -import "game_event.proto"; -message Event_PlayerOrderChanged { - extend GameEvent { - optional Event_PlayerOrderChanged ext = 2023; - } - - // The new seating order, expressed as player names. - repeated string player_names = 1; - - // Whether the order was obtained by randomizing rather than set explicitly. - optional bool randomized = 2; -} diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/game_commands.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/game_commands.proto index eb399ffb9..2e5b88978 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/game_commands.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/game_commands.proto @@ -180,11 +180,6 @@ message GameCommand { /// Server: Server_Player::cmdSetPlaymat /// Client: reflected via player properties changed event SET_PLAYMAT = 1035; - - /// Set the seating (turn) order of the players. - /// Server: Server_AbstractParticipant::cmdSetPlayerOrder - /// Client: reflected via Event_PlayerOrderChanged - SET_PLAYER_ORDER = 1206; } extensions 100 to max; diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/game_event.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/game_event.proto index 573d26453..7d3147701 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/game_event.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/game_event.proto @@ -34,7 +34,6 @@ message GameEvent { CHANGE_ZONE_PROPERTIES = 2020; REVERSE_TURN = 2021; GAME_LOG_NOTICE = 2022; - PLAYER_ORDER_CHANGED = 2023; } optional sint32 player_id = 1 [default = -1]; extensions 100 to max; diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/room_commands.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/room_commands.proto index 3be7e4e64..9d9c6d277 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/room_commands.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/room_commands.proto @@ -78,9 +78,6 @@ message Command_CreateGame { // share decklists with all players when selected optional bool share_decklists_on_load = 14; - - // shuffle the player order (seats) before each game starts - optional bool shuffle_players = 15; } message Command_JoinGame { diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/serverinfo_game.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/serverinfo_game.proto index 39ec79914..9989ae18a 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/serverinfo_game.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/serverinfo_game.proto @@ -48,9 +48,6 @@ message ServerInfo_Game { // decklists are sent to all players when loaded optional bool share_decklists_on_load = 15; - // players are shuffled into new seats before each game starts - optional bool shuffle_players = 16; - // the current player count optional uint32 player_count = 30; diff --git a/libcockatrice_settings/libcockatrice/settings/game_settings.cpp b/libcockatrice_settings/libcockatrice/settings/game_settings.cpp index 89fe7b85e..c8c6463e3 100644 --- a/libcockatrice_settings/libcockatrice/settings/game_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/game_settings.cpp @@ -71,11 +71,6 @@ bool GameSettings::getShareDecklistsOnLoad() const return getValue("shareDecklistsOnLoad", "game").toBool(); } -bool GameSettings::getShufflePlayers() const -{ - return getValue("shuffleplayers", "game").toBool(); -} - bool GameSettings::getRememberGameSettings() const { return getValue("rememberGameSettings", "game", QString(), true).toBool(); @@ -156,11 +151,6 @@ void GameSettings::setShareDecklistsOnLoad(bool _shareDecklistsOnLoad) setValue(_shareDecklistsOnLoad, "shareDecklistsOnLoad", "game"); } -void GameSettings::setShufflePlayers(bool _shufflePlayers) -{ - setValue(_shufflePlayers, "shuffleplayers", "game"); -} - void GameSettings::setRememberGameSettings(bool _rememberGameSettings) { setValue(_rememberGameSettings, "rememberGameSettings", "game"); diff --git a/libcockatrice_settings/libcockatrice/settings/game_settings.h b/libcockatrice_settings/libcockatrice/settings/game_settings.h index 45ec8554b..3e64234fc 100644 --- a/libcockatrice_settings/libcockatrice/settings/game_settings.h +++ b/libcockatrice_settings/libcockatrice/settings/game_settings.h @@ -27,7 +27,6 @@ public: [[nodiscard]] bool getCreateGameAsSpectator() const override; [[nodiscard]] int getDefaultStartingLifeTotal() const override; [[nodiscard]] bool getShareDecklistsOnLoad() const override; - [[nodiscard]] bool getShufflePlayers() const override; [[nodiscard]] bool getRememberGameSettings() const override; [[nodiscard]] bool getLocalGameRememberSettings() const override; [[nodiscard]] int getLocalGameMaxPlayers() const override; @@ -45,7 +44,6 @@ public: void setCreateGameAsSpectator(bool _createGameAsSpectator); void setDefaultStartingLifeTotal(int _defaultStartingLifeTotal); void setShareDecklistsOnLoad(bool _shareDecklistsOnLoad); - void setShufflePlayers(bool _shufflePlayers); void setRememberGameSettings(bool _rememberGameSettings); void setLocalGameRememberSettings(bool value); void setLocalGameMaxPlayers(int value); diff --git a/oracle/CMakeLists.txt b/oracle/CMakeLists.txt index f68f51f23..587463572 100644 --- a/oracle/CMakeLists.txt +++ b/oracle/CMakeLists.txt @@ -17,7 +17,6 @@ set(ORACLE_WIN32_QM_INSTALL_DIR "translations") # Sources # ------------------------ set(oracle_SOURCES - src/database_writer.cpp src/main.cpp src/oraclewizard.cpp src/oracleimporter.cpp diff --git a/oracle/src/database_writer.cpp b/oracle/src/database_writer.cpp deleted file mode 100644 index e80aa853a..000000000 --- a/oracle/src/database_writer.cpp +++ /dev/null @@ -1,52 +0,0 @@ -#include "database_writer.h" - -#include -#include -#include -#include - -namespace OracleDatabase -{ -QString validateXml(const QByteArray &data) -{ - QXmlStreamReader xml(data); - if (!xml.readNextStartElement() || xml.name() != QLatin1String("cockatrice_carddatabase")) { - return QCoreApplication::translate("OracleDatabase", "The download is not a Cockatrice card database."); - } - const auto version = xml.attributes().value("version"); - if (version != QLatin1String("3") && version != QLatin1String("4")) { - return QCoreApplication::translate("OracleDatabase", "The downloaded card database version is not supported."); - } - while (!xml.atEnd()) { - xml.readNext(); - } - if (xml.hasError()) { - return QCoreApplication::translate("OracleDatabase", - "The downloaded card database is incomplete or invalid: %1") - .arg(xml.errorString()); - } - return {}; -} - -bool saveXml(const QString &fileName, const QByteArray &data, QString *error) -{ - const QString validationError = validateXml(data); - if (!validationError.isEmpty()) { - if (error) { - *error = validationError; - } - return false; - } - QSaveFile file(fileName); - if (!file.open(QIODevice::WriteOnly) || file.write(data) != data.size() || !file.commit()) { - if (error) { - *error = file.errorString(); - } - return false; - } - if (error) { - error->clear(); - } - return true; -} -} // namespace OracleDatabase diff --git a/oracle/src/database_writer.h b/oracle/src/database_writer.h deleted file mode 100644 index 173fd23ab..000000000 --- a/oracle/src/database_writer.h +++ /dev/null @@ -1,17 +0,0 @@ -#ifndef ORACLE_DATABASE_WRITER_H -#define ORACLE_DATABASE_WRITER_H - -#include -#include - -namespace OracleDatabase -{ -// Validate the complete downloaded document without allocating a card database. -// Empty string means valid; otherwise returns a diagnostic suitable for the wizard. -QString validateXml(const QByteArray &data); -// Never truncate the installed database: validate, write a sibling temporary file, -// check the complete write and atomically commit it. -bool saveXml(const QString &fileName, const QByteArray &data, QString *error = nullptr); -} // namespace OracleDatabase - -#endif diff --git a/oracle/src/oracleimporter.cpp b/oracle/src/oracleimporter.cpp index c83c962e3..477c54d21 100644 --- a/oracle/src/oracleimporter.cpp +++ b/oracle/src/oracleimporter.cpp @@ -100,10 +100,6 @@ static CardSet::Priority getSetPriority(const QString &setType, const QString &s bool OracleImporter::readSetsFromByteArray(QByteArray data) { - importSuccessful = false; - importError.clear(); - allSets.clear(); - rawSetsData.clear(); const RawJson::ScanProgressCallback progress = progressReporting ? [this]( @@ -768,15 +764,7 @@ int OracleImporter::startImport() { static ICardSetPriorityController *noOpController = new NoopCardSetPriorityController(); - importSuccessful = false; - importError.clear(); - sets.clear(); - cards.clear(); - localizedEntries.clear(); - splitLocalizedTexts.clear(); - if (allSets.isEmpty()) { - return failImport(tr("No set has been selected for import.")); - } + importCancelled.storeRelease(0); // Pre-allocate the cards hash to avoid rehashing during import. Keys are // distinct card names while raw ranges only count printings (AllPrintings @@ -798,7 +786,9 @@ int OracleImporter::startImport() for (const SetToDownload &curSetToParse : allSets) { if (importCancelled.loadAcquire()) { - return failImport(tr("The card database import was cancelled.")); + // The wizard was closed mid-import: stop at the next set boundary so + // the caller can wait for this future without processing every set. + break; } CardSetPtr newSet = CardSet::newInstance(noOpController, curSetToParse.getShortName(), @@ -808,20 +798,30 @@ int OracleImporter::startImport() // parse only this set's slice of the raw document so the whole JSON tree is // never kept in memory at once const RawJson::SetDataRange &rawRange = curSetToParse.getRawRange(); - if (rawRange.start < 0 || rawRange.length <= 0 || rawRange.start > rawSetsData.size() || - rawRange.length > rawSetsData.size() - rawRange.start) { - return failImport(tr("The source data for set %1 is unavailable. Load the sets file again.") - .arg(curSetToParse.getShortName())); + const qsizetype rangeEnd = rawRange.start + rawRange.length; + if (rawRange.start < 0 || rawRange.length <= 0 || rangeEnd > rawSetsData.size()) { + // rawSetsData is cleared by releaseSetData() while SetToDownload copies + // taken from getSets() keep their ranges, and nothing else enforces the + // pairing — so never index past the buffer on stale/mismatched ranges. + qWarning() << "error: out-of-bounds raw range for set" << curSetToParse.getShortName() << "skipping"; + ++setIndex; + emit setIndexChanged(0, setIndex, curSetToParse.getLongName()); + continue; } + // sliced() shares the buffer instead of deep-copying the slice; the largest + // sets in AllPrintings are tens of MB, so the copy is worth avoiding here. const QByteArray setBytes = rawSetsData.sliced(rawRange.start, rawRange.length); QJsonParseError parseError; const QJsonDocument setDoc = QJsonDocument::fromJson(setBytes, &parseError); if (parseError.error != QJsonParseError::NoError) { - return failImport( - tr("Could not import set %1: %2").arg(curSetToParse.getShortName(), parseError.errorString())); - } - if (!setDoc.isObject() || !setDoc.object().value("cards").isArray()) { - return failImport(tr("Set %1 does not contain a card list.").arg(curSetToParse.getShortName())); + qWarning() << "error: parsing card data for set" << curSetToParse.getShortName() << ":" + << parseError.errorString(); + ++setIndex; + // Keep the progress accounting honest: a set that failed to parse + // still advanced the index, so report it (with zero imported cards) + // rather than letting SaveSetsPage's bar stall per failed set. + emit setIndexChanged(0, setIndex, curSetToParse.getLongName()); + continue; } // Only add the set to the database once its slice parsed cleanly; @@ -838,14 +838,7 @@ int OracleImporter::startImport() emit setIndexChanged(numCardsInSet, setIndex, curSetToParse.getLongName()); } - if (importCancelled.loadAcquire()) { - return failImport(tr("The card database import was cancelled.")); - } - if (cards.isEmpty()) { - return failImport(tr("No cards were imported. The existing card database has been kept.")); - } applyLocalizedData(); - importSuccessful = true; emit setIndexChanged(0, setIndex, QString()); @@ -853,23 +846,8 @@ int OracleImporter::startImport() return setIndex; } -int OracleImporter::failImport(const QString &message) -{ - importSuccessful = false; - importError = message; - sets.clear(); - cards.clear(); - localizedEntries.clear(); - splitLocalizedTexts.clear(); - qWarning() << message; - return 0; -} - bool OracleImporter::saveToFile(const QString &fileName, const QString &sourceUrl, const QString &sourceVersion) { - if (!importSuccessful) { - return false; - } CockatriceXml4Parser parser(new NoopCardSetPriorityController()); return parser.saveToFile(createDefaultMagicFormats(), sets, cards, fileName, sourceUrl, sourceVersion); @@ -883,7 +861,6 @@ void OracleImporter::releaseSetData() void OracleImporter::clear() { - prepareImport(); sets.clear(); cards.clear(); allSets.clear(); diff --git a/oracle/src/oracleimporter.h b/oracle/src/oracleimporter.h index 5ebe9cf36..d872e184e 100644 --- a/oracle/src/oracleimporter.h +++ b/oracle/src/oracleimporter.h @@ -198,11 +198,6 @@ private: * the whole import (or racing it). */ QAtomicInt importCancelled; - bool importSuccessful = false; - QString importError; - - // Discard the incomplete in-memory database; the installed file is untouched. - int failImport(const QString &message); /** * The ISO-639 language code whose foreignData is imported; "en" by default. @@ -283,24 +278,7 @@ public: * the wizard can hand over its decompressed buffer without copying it. */ bool readSetsFromByteArray(QByteArray data); - // Returns the complete selected-set count on success, zero on failure or cancellation. int startImport(); - bool hasSuccessfulImport() const - { - return importSuccessful; - } - const QString &getImportError() const - { - return importError; - } - // Call on the owning thread before scheduling a worker, so a cancellation - // arriving before that worker starts is not lost. - void prepareImport() - { - importCancelled.storeRelease(0); - importSuccessful = false; - importError.clear(); - } /** * @brief Requests an in-flight startImport() to stop at the next set boundary. * diff --git a/oracle/src/oraclewizard.cpp b/oracle/src/oraclewizard.cpp index dc8c7ecb3..c33d821a5 100644 --- a/oracle/src/oraclewizard.cpp +++ b/oracle/src/oraclewizard.cpp @@ -1,7 +1,6 @@ #include "oraclewizard.h" #include "client/settings/cache_settings.h" -#include "database_writer.h" #include "main.h" #include "oracleimporter.h" #include "pages.h" @@ -142,13 +141,6 @@ void OracleWizard::reject() } QWizard::reject(); - // A hidden wizard does not close the application's last visible window. - // Stop background refreshes explicitly so the hosting client receives a verdict, - // using the same rule as main(): a reject after the database was written is still - // a successful update, only a reject that wrote nothing counts as a failure. - if (backgroundMode) { - QCoreApplication::exit(hasSavedDatabase() ? 0 : 1); - } } void OracleWizard::runInBackground() @@ -220,10 +212,17 @@ void OracleWizard::disableButtons() bool OracleWizard::saveTokensToFile(const QString &fileName) { - QString error; - if (!OracleDatabase::saveXml(fileName, tokensData, &error)) { - qWarning() << error; + QFile file(fileName); + if (!file.open(QIODevice::WriteOnly)) { + qDebug() << "File open (w) failed for" << fileName; return false; } + + if (file.write(tokensData) == -1) { + qDebug() << "File write (w) failed for" << fileName; + return false; + } + + file.close(); return true; } diff --git a/oracle/src/pages.cpp b/oracle/src/pages.cpp index f5c38be1c..4c664a38c 100644 --- a/oracle/src/pages.cpp +++ b/oracle/src/pages.cpp @@ -1,7 +1,6 @@ #include "pages.h" #include "client/settings/cache_settings.h" -#include "database_writer.h" #include "main.h" #include "oracleimporter.h" #include "oraclewizard.h" @@ -44,7 +43,6 @@ #include #include #include -#include #include #include #include @@ -171,16 +169,7 @@ LoadSetsResult loadSetsData(const QPointer &importer, QByteArray break; } - // Test the first non-space byte in place: trimmed() would duplicate the whole - // payload, which on the plain-XML path is the largest download we handle. - const auto firstNonSpace = std::find_if_not(data.cbegin(), data.cend(), - [](char c) { return c == ' ' || c == '\t' || c == '\n' || c == '\r'; }); - const bool looksLikeXml = firstNonSpace != data.cend() && *firstNonSpace == '<'; - if (looksLikeXml) { - result.errorMessage = OracleDatabase::validateXml(data); - if (!result.errorMessage.isEmpty()) { - return result; - } + if (data.startsWith("<")) { result.ok = true; result.plainXml = true; result.xmlData = std::move(data); @@ -662,10 +651,6 @@ void LoadSetsPage::importFinished() } else if (!result.ok && !result.plainXml) { qWarning() << tr("The file was retrieved successfully, but it does not contain any sets data."); } - if (!result.ok) { - wizard()->reject(); - return; - } emit readyToContinue(); return; } @@ -732,7 +717,7 @@ SaveSetsPage::SaveSetsPage(QWidget *parent) : OracleWizardPage(parent) bool SaveSetsPage::isComplete() const { - return !importActive && importReady; + return !importActive; } void SaveSetsPage::cleanupPage() @@ -745,11 +730,8 @@ void SaveSetsPage::cleanupPage() void SaveSetsPage::initializePage() { - importReady = false; retranslateUi(); if (wizard()->downloadedPlainXml) { - importReady = true; - emit completeChanged(); messageLog->hide(); progressBar->hide(); if (wizard()->backgroundMode) { @@ -775,9 +757,6 @@ void SaveSetsPage::initializePage() emit completeChanged(); const QPointer importer = wizard()->importer; - if (auto *activeImporter = importer.data()) { - activeImporter->prepareImport(); - } importFuture = QtConcurrent::run([importer] { return importer ? importer->startImport() : 0; }); importWatcher.setFuture(importFuture); } @@ -809,28 +788,22 @@ void SaveSetsPage::importFinished() const int setsImported = importWatcher.result(); const QPointer importer = wizard()->importer; - importReady = importer && importer->hasSuccessfulImport() && setsImported == totalSets; - const QString error = importer ? importer->getImportError() : tr("The card database importer is unavailable."); if (importer) { importer->releaseSetData(); } - wizard()->button(QWizard::NextButton)->setEnabled(importReady); - emit completeChanged(); - if (!importReady) { - const QString message = error.isEmpty() ? tr("No set has been imported.") : error; - if (wizard()->backgroundMode) { - qWarning() << message; - wizard()->reject(); - } else { - QMessageBox::critical(this, tr("Error"), message); + if (wizard()->backgroundMode) { + if (setsImported == 0) { + qWarning() << tr("No set has been imported."); } + emit readyToContinue(); return; } progressBar->setValue(progressBar->maximum()); - if (wizard()->backgroundMode) { - emit readyToContinue(); + + if (setsImported == 0) { + QMessageBox::critical(this, tr("Error"), tr("No set has been imported.")); } } @@ -881,18 +854,6 @@ void SaveSetsPage::updateTotalProgress(int cardsImported, int setIndex, const QS bool SaveSetsPage::validatePage() { - if (!isComplete()) { - return false; - } - const auto failSave = [this](const QString &message) { - qWarning() << message; - if (wizard()->backgroundMode) { - wizard()->reject(); - } else { - QMessageBox::critical(this, tr("Error"), message); - } - return false; - }; QString defaultPath = SettingsCache::instance().getCardDatabasePath(); QString windowName = tr("Save card database"); QString fileType = tr("XML; card database (*.xml)"); @@ -911,21 +872,24 @@ bool SaveSetsPage::validatePage() QFileInfo fi(fileName); QDir fileDir(fi.path()); if (!fileDir.exists() && !fileDir.mkpath(fileDir.absolutePath())) { - return failSave(tr("The database directory could not be created: %1").arg(fileDir.absolutePath())); + return false; } if (wizard()->downloadedPlainXml) { - QString error; - if (!OracleDatabase::saveXml(fileName, wizard()->xmlData, &error)) { - return failSave(tr("The file could not be saved to %1: %2").arg(fileName, error)); + QFile file(fileName); + if (!file.open(QIODevice::WriteOnly)) { + qDebug() << "File write (w) failed for" << fileName; + return false; + } + if (file.write(wizard()->xmlData) < 1) { + qDebug() << "File write (w) failed for" << fileName; + return false; } wizard()->xmlData.clear(); } else if (!wizard()->importer->saveToFile(fileName, wizard()->getCardSourceUrl(), wizard()->getCardSourceVersion())) { - // The importer refuses to write a database whose import did not finish, and - // reports the reason there; do not dress an import failure up as a disk failure. - const QString importError = wizard()->importer->getImportError(); - return failSave(importError.isEmpty() ? tr("The file could not be saved to %1").arg(fileName) : importError); + QMessageBox::critical(this, tr("Error"), tr("The file could not be saved to %1").arg(fileName)); + return false; } wizard()->markDatabaseSaved(); diff --git a/oracle/src/pages.h b/oracle/src/pages.h index e29a8e84a..0530010ba 100644 --- a/oracle/src/pages.h +++ b/oracle/src/pages.h @@ -128,7 +128,6 @@ private: QFuture importFuture; int totalSets = 0; bool importActive = false; - bool importReady = false; protected: void initializePage() override; diff --git a/oracle/src/pagetemplates.cpp b/oracle/src/pagetemplates.cpp index 7e5f30bb7..dcc8e070c 100644 --- a/oracle/src/pagetemplates.cpp +++ b/oracle/src/pagetemplates.cpp @@ -1,6 +1,5 @@ #include "pagetemplates.h" -#include "database_writer.h" #include "oraclewizard.h" #include @@ -185,17 +184,9 @@ void SimpleDownloadFilePage::actDownloadFinished() } QNetworkReply::NetworkError errorCode = reply->error(); if (errorCode != QNetworkReply::NoError) { - const QString message = tr("Network error: %1.").arg(reply->errorString()); - reply->deleteLater(); - // A hidden wizard cannot show a modal box: without an explicit stop it would sit - // here forever and the hosting client would wait with no timeout. - if (wizard()->backgroundMode) { - qWarning() << message; - wizard()->reject(); - return; - } - QMessageBox::critical(this, tr("Error"), message); + QMessageBox::critical(this, tr("Error"), tr("Network error: %1.").arg(reply->errorString())); wizard()->enableButtons(); + reply->deleteLater(); return; } @@ -227,15 +218,6 @@ void SimpleDownloadFilePage::actDownloadFinished() bool SimpleDownloadFilePage::saveToFile() { - const auto failSave = [this](const QString &message) { - qWarning() << message; - if (wizard()->backgroundMode) { - wizard()->reject(); - } else { - QMessageBox::critical(this, tr("Error"), message); - } - return false; - }; QString defaultPath = getDefaultSavePath(); QString windowName = getWindowTitle(); QString fileType = getFileType(); @@ -254,11 +236,12 @@ bool SimpleDownloadFilePage::saveToFile() QFileInfo fi(fileName); QDir fileDir(fi.path()); if (!fileDir.exists() && !fileDir.mkpath(fileDir.absolutePath())) { - return failSave(tr("The database directory could not be created: %1").arg(fileDir.absolutePath())); + return false; } if (!internalSaveToFile(fileName)) { - return failSave(tr("The file could not be saved to %1").arg(fileName)); + QMessageBox::critical(this, tr("Error"), tr("The file could not be saved to %1").arg(fileName)); + return false; } // clean saved downloadData @@ -269,10 +252,17 @@ bool SimpleDownloadFilePage::saveToFile() bool SimpleDownloadFilePage::internalSaveToFile(const QString &fileName) { - QString error; - if (!OracleDatabase::saveXml(fileName, downloadData, &error)) { - qWarning() << error; + QFile file(fileName); + if (!file.open(QIODevice::WriteOnly)) { + qDebug() << "File open (w) failed for" << fileName; return false; } + + if (file.write(downloadData) == -1) { + qDebug() << "File write (w) failed for" << fileName; + return false; + } + + file.close(); return true; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e25390ba0..c87bec627 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -220,18 +220,6 @@ cockatrice_add_test( libcockatrice_deck_list libcockatrice_utility ) -cockatrice_add_test( - NAME - card_art_crop_test - SOURCES - ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/widgets/cards/card_art_utils.cpp - ${VERSION_STRING_CPP} - card_art_crop_test.cpp - LIBS - libcockatrice_card - INCLUDE_DIRS - ${CMAKE_SOURCE_DIR}/cockatrice/src -) cockatrice_add_test( NAME server_rate_limiter_test diff --git a/tests/card_art_crop_test.cpp b/tests/card_art_crop_test.cpp deleted file mode 100644 index 1c4a9ff8d..000000000 --- a/tests/card_art_crop_test.cpp +++ /dev/null @@ -1,92 +0,0 @@ -#include "interface/widgets/cards/card_art_utils.h" - -#include -#include -#include -#include -#include -#include -#include - -namespace -{ -ExactCard makeCard(bool landscape) -{ - CardInfo::UiAttributes attributes; - attributes.landscapeOrientation = landscape; - return ExactCard(CardInfo::newInstance("Artwork", "", false, {}, {}, {}, {}, attributes)); -} - -QPixmap landscapeCardWithFrame() -{ - // Normalized landscape frame: the title reaches 14% down the upright - // image, and the Battle type line starts at 56%. Artwork has two colors - // so the test also detects a wrong or missing rotation. - QImage upright(1400, 1000, QImage::Format_RGB32); - upright.fill(Qt::red); - QPainter painter(&upright); - painter.fillRect(QRect(70, 140, 1260, 420), Qt::green); - painter.fillRect(QRect(700, 140, 630, 420), Qt::blue); - painter.end(); - QTransform storedOrientation; - storedOrientation.rotate(-90); - return QPixmap::fromImage(upright.transformed(storedOrientation)); -} - -TEST(CardArtCrop, LandscapeCropExcludesNameAndRulesBoxesAndRotatesUpright) -{ - const QImage crop = CardArtUtils::cropCardArt(landscapeCardWithFrame(), makeCard(true)).toImage(); - ASSERT_GT(crop.width(), crop.height()); - ASSERT_FALSE(crop.isNull()); - for (int y = 0; y < crop.height(); ++y) { - for (int x = 0; x < crop.width(); ++x) { - EXPECT_EQ(x < crop.width() / 2 ? QColor(Qt::green) : QColor(Qt::blue), crop.pixelColor(x, y)); - } - } -} - -TEST(CardArtCrop, PortraitCropRetainsItsExistingArtworkBounds) -{ - QImage source(745, 1040, QImage::Format_RGB32); - for (int y = 0; y < source.height(); ++y) { - for (int x = 0; x < source.width(); ++x) { - source.setPixelColor(x, y, QColor(x % 256, y % 256, (x + y) % 256)); - } - } - const QPixmap input = QPixmap::fromImage(source); - const QImage crop = CardArtUtils::cropCardArt(input, makeCard(false)).toImage(); - EXPECT_EQ(source.copy(QRect(52, 114, 641, 458)), crop); -} - -TEST(CardArtCrop, DevicePixelRatioDoesNotChangeTheArtworkPixels) -{ - QPixmap source = landscapeCardWithFrame(); - const QPixmap normal = CardArtUtils::cropCardArt(source, makeCard(true)); - source.setDevicePixelRatio(2.0); - const QPixmap highDpi = CardArtUtils::cropCardArt(source, makeCard(true)); - EXPECT_EQ(normal.size(), highDpi.size()); - EXPECT_EQ(2.0, highDpi.devicePixelRatio()); - QImage normalPixels = normal.toImage(); - QImage highDpiPixels = highDpi.toImage(); - normalPixels.setDevicePixelRatio(1.0); - highDpiPixels.setDevicePixelRatio(1.0); - EXPECT_EQ(normalPixels, highDpiPixels); -} - -TEST(CardArtCrop, EmptyAndSmallImagesStayWithinTheirBounds) -{ - EXPECT_TRUE(CardArtUtils::cropCardArt({}, makeCard(true)).isNull()); - QPixmap small(1, 1); - small.fill(Qt::green); - EXPECT_EQ(small.toImage(), CardArtUtils::cropCardArt(small, makeCard(true)).toImage()); - EXPECT_EQ(small.toImage(), CardArtUtils::cropCardArt(small, makeCard(false)).toImage()); -} -} // namespace - -int main(int argc, char **argv) -{ - qputenv("QT_QPA_PLATFORM", "offscreen"); - QApplication app(argc, argv); - ::testing::InitGoogleTest(&argc, argv); - return RUN_ALL_TESTS(); -} diff --git a/tests/carddatabase/carddatabase_test.cpp b/tests/carddatabase/carddatabase_test.cpp index b54205f30..22cbe9074 100644 --- a/tests/carddatabase/carddatabase_test.cpp +++ b/tests/carddatabase/carddatabase_test.cpp @@ -36,12 +36,6 @@ #include #include -#ifdef Q_OS_UNIX -#include -#include -#include -#endif - namespace { @@ -210,64 +204,6 @@ TEST(CardDatabaseTest, Xml4LocalizedDataRoundTrip) ASSERT_EQ(printings.first().getProperty("manacost"), "1R"); } -#ifdef Q_OS_UNIX -/** - * @brief A failed write keeps the previously installed card database intact. - * - * This is the writer Oracle uses to install a refreshed database, so the truncation - * bug has to be covered here and not only through OracleDatabase::saveXml. - */ -TEST(CardDatabaseTest, Xml4ShortWriteKeepsInstalledFile) -{ - NoopCardSetPriorityController controller; - CardSetPtr set = - CardSet::newInstance(&controller, "TST", "Test Set", "expansion", QDate(), CardSet::PriorityPrimary); - - QHash props; - props["manacost"] = "1R"; - PrintingInfo printing(set, LazyPropertiesHash(props)); - SetToPrintingsMap setsInfo; - setsInfo["TST"].append(printing); - - CardInfo::UiAttributes attributes = {.tableRow = 1}; - CardInfoPtr card = - CardInfo::newInstance("Lightning Bolt", QString(1024, 'x'), false, {}, {}, {}, setsInfo, attributes); - - SetNameMap sets; - sets.insert("TST", set); - CardNameMap cards; - cards.insert("Lightning Bolt", card); - - QTemporaryDir tempDir; - ASSERT_TRUE(tempDir.isValid()); - const QString fileName = tempDir.filePath("cards.xml"); - - QFile installed(fileName); - ASSERT_TRUE(installed.open(QIODevice::WriteOnly)); - ASSERT_EQ(installed.write("installed database"), 18); - installed.close(); - - // A real filesystem write failure, isolated in the death-test child so the - // process-wide file limit never affects other cases or the parent runner. - ASSERT_EXIT( - { - std::signal(SIGXFSZ, SIG_IGN); - struct rlimit limit; - limit.rlim_cur = 128; - limit.rlim_max = 128; - if (setrlimit(RLIMIT_FSIZE, &limit) != 0) { - _exit(2); - } - CockatriceXml4Parser writer(&controller); - _exit(writer.saveToFile({}, sets, cards, fileName) ? 1 : 0); - }, - ::testing::ExitedWithCode(0), ""); - - ASSERT_TRUE(installed.open(QIODevice::ReadOnly)); - EXPECT_EQ(installed.readAll(), "installed database"); -} -#endif - /** * @brief Path provider serving a sandboxed copy of the test card database. */ 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 d4e44125c..95df74702 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,37 +278,6 @@ 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 a67e689cf..1903b70df 100644 --- a/tests/deck_list_zones/deck_list_zones_test.cpp +++ b/tests/deck_list_zones/deck_list_zones_test.cpp @@ -393,28 +393,6 @@ 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); diff --git a/tests/movecard_tests/CMakeLists.txt b/tests/movecard_tests/CMakeLists.txt index 35d7b44ed..68d8fc84c 100755 --- a/tests/movecard_tests/CMakeLists.txt +++ b/tests/movecard_tests/CMakeLists.txt @@ -9,15 +9,3 @@ cockatrice_add_test( LABELS server ) - -cockatrice_add_test( - NAME - player_order_test - SOURCES - player_order_test.cpp - LIBS - libcockatrice_network_server_remote - libcockatrice_rng - LABELS - server -) diff --git a/tests/movecard_tests/player_order_test.cpp b/tests/movecard_tests/player_order_test.cpp deleted file mode 100644 index 6e398af24..000000000 --- a/tests/movecard_tests/player_order_test.cpp +++ /dev/null @@ -1,268 +0,0 @@ -#include "game/game_config.h" -#include "game/role_info.h" -#include "game/server_abstract_participant.h" -#include "game/server_game.h" -#include "server_abstractuserinterface.h" -#include "server_response_containers.h" -#include "server_room.h" -#include "server_test_helpers.h" - -#include -#include -#include -#include -#include -#include -#include - -RNG_Abstract *rng = nullptr; // this needs to be defined due to other functions in server - -class ScriptedRng : public RNG_Abstract -{ -public: - QList values; - - unsigned int rand(int min, int max) override - { - const unsigned int value = values.takeFirst(); - EXPECT_GE(value, static_cast(min)); - EXPECT_LE(value, static_cast(max)); - return value; - } -}; - -class FakeUserInterface : public Server_AbstractUserInterface -{ -public: - FakeUserInterface(Server *_server, const ServerInfo_User &_userInfo) : Server_AbstractUserInterface(_server) - { - setUserInfo(_userInfo); - } - - int getLastCommandTime() const override - { - return 0; - } - - bool addSaidMessageSize(int) override - { - return true; - } - - void sendProtocolItem(const Response &) override - { - } - - void sendProtocolItem(const SessionEvent &) override - { - } - - void sendProtocolItem(const GameEventContainer &) override - { - } - - void sendProtocolItem(const RoomEvent &) override - { - } -}; - -namespace -{ -QStringList participantNamesInOrder(const Server_Game &game) -{ - QStringList names; - for (int id : game.getSeatOrder()) { - const auto *participant = game.getParticipants().value(id, nullptr); - if (participant) { - names.append(QString::fromStdString(participant->getUserInfo()->name())); - } - } - return names; -} - -QList participantIdsInOrder(const Server_Game &game) -{ - return game.getSeatOrder(); -} -} // namespace - -class PlayerOrderTest : public ::testing::Test -{ -protected: - ServerInfo_User creator; - ServerInfo_User userB; - ServerInfo_User userC; - - PlayerOrderTest() - { - creator.set_name("A"); - creator.set_user_level(ServerInfo_User::IsRegistered); - userB.set_name("B"); - userB.set_user_level(ServerInfo_User::IsRegistered); - userC.set_name("C"); - userC.set_user_level(ServerInfo_User::IsRegistered); - } -}; - -TEST_F(PlayerOrderTest, JoinOrderIsSeatOrder) -{ - FakeServer server; - Server_Room room(0, 0, "", "", "", "", false, "", {}, &server); - FakeUserInterface uiA(&server, creator); - FakeUserInterface uiB(&server, userB); - FakeUserInterface uiC(&server, userC); - GameConfig config{.creatorInfo = creator, - .gameId = 1, - .description = QString(), - .password = QString(), - .maxPlayers = 3, - .gameTypes = QList(), - .onlyBuddies = false, - .onlyRegistered = false, - .spectatorsAllowed = false, - .spectatorsNeedPassword = false, - .spectatorsCanTalk = false, - .spectatorsSeeEverything = false, - .startingLifeTotal = 20, - .shareDecklistsOnLoad = false, - .shufflePlayers = false}; - Server_Game game(config, &room); - ResponseContainer rc(0); - - game.addPlayer(&uiA, rc, RoleInfo{}, false); - game.addPlayer(&uiB, rc, RoleInfo{}, false); - game.addPlayer(&uiC, rc, RoleInfo{}, false); - - EXPECT_EQ(participantNamesInOrder(game), QStringList({"A", "B", "C"})); - EXPECT_EQ(participantIdsInOrder(game), QList({0, 1, 2})); - EXPECT_EQ(game.getHostId(), 0); - EXPECT_EQ(game.getPlayerCount(), 3); -} - -TEST_F(PlayerOrderTest, ReorderPlayerSeats) -{ - FakeServer server; - Server_Room room(0, 0, "", "", "", "", false, "", {}, &server); - FakeUserInterface uiA(&server, creator); - FakeUserInterface uiB(&server, userB); - FakeUserInterface uiC(&server, userC); - GameConfig config{.creatorInfo = creator, - .gameId = 1, - .description = QString(), - .password = QString(), - .maxPlayers = 3, - .gameTypes = QList(), - .onlyBuddies = false, - .onlyRegistered = false, - .spectatorsAllowed = false, - .spectatorsNeedPassword = false, - .spectatorsCanTalk = false, - .spectatorsSeeEverything = false, - .startingLifeTotal = 20, - .shareDecklistsOnLoad = false, - .shufflePlayers = false}; - Server_Game game(config, &room); - ResponseContainer rc(0); - - game.addPlayer(&uiA, rc, RoleInfo{}, false); - game.addPlayer(&uiB, rc, RoleInfo{}, false); - game.addPlayer(&uiC, rc, RoleInfo{}, false); - - game.reorderPlayerSeats({"C", "A", "B"}); - - // player ids are stable; only the seat order changes - EXPECT_EQ(participantNamesInOrder(game), QStringList({"C", "A", "B"})); - EXPECT_EQ(participantIdsInOrder(game), QList({2, 0, 1})); - EXPECT_EQ(game.getParticipants().size(), 3); - - // the host keeps its seat: creator A is still in the game, so host stays 0 - EXPECT_EQ(game.getHostId(), 0); - - // no re-keying means every mapping keyed by player id stays valid - const int gameId = game.getGameId(); - EXPECT_EQ(uiA.getGames().value(gameId).second, 0); - EXPECT_EQ(uiB.getGames().value(gameId).second, 1); - EXPECT_EQ(uiC.getGames().value(gameId).second, 2); -} - -TEST_F(PlayerOrderTest, ReorderPlayerSeatsKeepsPlayerCount) -{ - FakeServer server; - Server_Room room(0, 0, "", "", "", "", false, "", {}, &server); - FakeUserInterface uiA(&server, creator); - FakeUserInterface uiB(&server, userB); - FakeUserInterface uiC(&server, userC); - GameConfig config{.creatorInfo = creator, - .gameId = 1, - .description = QString(), - .password = QString(), - .maxPlayers = 3, - .gameTypes = QList(), - .onlyBuddies = false, - .onlyRegistered = false, - .spectatorsAllowed = false, - .spectatorsNeedPassword = false, - .spectatorsCanTalk = false, - .spectatorsSeeEverything = false, - .startingLifeTotal = 20, - .shareDecklistsOnLoad = false, - .shufflePlayers = false}; - Server_Game game(config, &room); - ResponseContainer rc(0); - - game.addPlayer(&uiA, rc, RoleInfo{}, false); - game.addPlayer(&uiB, rc, RoleInfo{}, false); - game.addPlayer(&uiC, rc, RoleInfo{}, false); - - // listing only a subset appends the remaining players in their current order - game.reorderPlayerSeats({"C"}); - - EXPECT_EQ(participantNamesInOrder(game), QStringList({"C", "A", "B"})); - EXPECT_EQ(participantIdsInOrder(game), QList({2, 0, 1})); - EXPECT_EQ(game.getPlayerCount(), 3); - EXPECT_EQ(game.getHostId(), 0); -} - -TEST_F(PlayerOrderTest, ShufflePlayerSeatsIsAPermutation) -{ - FakeServer server; - Server_Room room(0, 0, "", "", "", "", false, "", {}, &server); - FakeUserInterface uiA(&server, creator); - FakeUserInterface uiB(&server, userB); - FakeUserInterface uiC(&server, userC); - GameConfig config{.creatorInfo = creator, - .gameId = 1, - .description = QString(), - .password = QString(), - .maxPlayers = 3, - .gameTypes = QList(), - .onlyBuddies = false, - .onlyRegistered = false, - .spectatorsAllowed = false, - .spectatorsNeedPassword = false, - .spectatorsCanTalk = false, - .spectatorsSeeEverything = false, - .startingLifeTotal = 20, - .shareDecklistsOnLoad = false, - .shufflePlayers = false}; - Server_Game game(config, &room); - ResponseContainer rc(0); - - game.addPlayer(&uiA, rc, RoleInfo{}, false); - game.addPlayer(&uiB, rc, RoleInfo{}, false); - game.addPlayer(&uiC, rc, RoleInfo{}, false); - - // fisher-yates on [A, B, C]: rand(0, 2) -> 1 (take B), rand(0, 1) -> 0 (take A), rand(0, 0) -> 0 (take C) - auto *scriptedRng = new ScriptedRng; - scriptedRng->values = {1, 0, 0}; - rng = scriptedRng; - - game.shufflePlayerSeats(); - - EXPECT_EQ(participantNamesInOrder(game), QStringList({"B", "A", "C"})); - EXPECT_EQ(participantIdsInOrder(game), QList({1, 0, 2})); - QSet idSet(participantIdsInOrder(game).cbegin(), participantIdsInOrder(game).cend()); - EXPECT_EQ(idSet.size(), 3); // ids are unique - // ids are stable, so the host (creator A, id 0) is unchanged - EXPECT_EQ(game.getHostId(), 0); -} diff --git a/tests/oracle/CMakeLists.txt b/tests/oracle/CMakeLists.txt index 1cc1ad952..6958673f7 100644 --- a/tests/oracle/CMakeLists.txt +++ b/tests/oracle/CMakeLists.txt @@ -9,7 +9,6 @@ cockatrice_add_test( ../../oracle/src/oracleimporter.cpp ../../oracle/src/parsehelpers.cpp ../../oracle/src/raw_json_scanner.cpp - ../../oracle/src/database_writer.cpp oracle_importer_test.cpp LIBS libcockatrice_card diff --git a/tests/oracle/oracle_importer_test.cpp b/tests/oracle/oracle_importer_test.cpp index 259bae087..83d77d465 100644 --- a/tests/oracle/oracle_importer_test.cpp +++ b/tests/oracle/oracle_importer_test.cpp @@ -1,4 +1,3 @@ -#include "../../oracle/src/database_writer.h" #include "../../oracle/src/oracleimporter.h" #include "../../oracle/src/raw_json_scanner.h" #include "libcockatrice/card/card_info.h" @@ -7,7 +6,6 @@ #include #include #include -#include #include #include #include @@ -22,7 +20,6 @@ #include #include #include -#include #include #include #include @@ -33,12 +30,6 @@ #include #include -#ifdef Q_OS_UNIX -#include -#include -#include -#endif - class OracleImporterTest : public ::testing::Test { protected: @@ -1149,212 +1140,6 @@ TEST_F(OracleImporterTest, StartImportAppliesLocalizedData) ASSERT_EQ(result->getLocalizedText("pt"), "Raio causa 3 de dano a qualquer alvo."); } -namespace -{ -QByteArray refreshSource(const QString &name = "New Card") -{ - QJsonObject card{{"name", name}, {"text", "Rules text."}, {"layout", "normal"}, - {"type", "Artifact"}, {"number", "1"}, {"rarity", "common"}}; - QJsonObject set{{"code", "TST"}, {"name", "Test Set"}, {"type", "expansion"}, {"cards", QJsonArray{card}}}; - return QJsonDocument(QJsonObject{{"data", QJsonObject{{"TST", set}}}}).toJson(QJsonDocument::Compact); -} -} // namespace - -TEST_F(OracleImporterTest, FailedLazyParseKeepsInstalledDatabase) -{ - QTemporaryDir dir; - ASSERT_TRUE(dir.isValid()); - const QString path = dir.filePath("cards.xml"); - QFile original(path); - ASSERT_TRUE(original.open(QIODevice::WriteOnly)); - ASSERT_EQ(original.write("existing database"), 17); - original.close(); - - // The metadata scanner skips numeric values without converting them. Qt's - // per-set parser rejects an overflowing number after selection succeeds. - auto badSet = QJsonDocument::fromJson(refreshSource()).object().value("data").toObject().value("TST").toObject(); - auto goodSet = badSet; - goodSet["code"] = "AAA"; - goodSet["name"] = "A successful first set"; - auto badCard = badSet.value("cards").toArray().first().toObject(); - badCard["number"] = "overflow"; - badSet["cards"] = QJsonArray{badCard}; - QByteArray data = QJsonDocument(QJsonObject{{"data", QJsonObject{{"AAA", goodSet}, {"TST", badSet}}}}) - .toJson(QJsonDocument::Compact); - data.replace("\"number\":\"overflow\"", "\"number\":1e9999"); - ASSERT_TRUE(importer->readSetsFromByteArray(data)); - EXPECT_EQ(importer->startImport(), 0); - EXPECT_FALSE(importer->hasSuccessfulImport()); - EXPECT_TRUE(importer->getImportError().contains("TST")); - EXPECT_TRUE(importer->getCardList().isEmpty()); - EXPECT_FALSE(importer->saveToFile(path, {}, {})); - ASSERT_TRUE(original.open(QIODevice::ReadOnly)); - EXPECT_EQ(original.readAll(), "existing database"); -} - -TEST_F(OracleImporterTest, EmptyImportCannotReplaceInstalledCards) -{ - const QByteArray source = R"({"data":{"TST":{"code":"TST","name":"Test","type":"expansion","cards":[]}}})"; - ASSERT_TRUE(importer->readSetsFromByteArray(source)); - EXPECT_EQ(importer->startImport(), 0); - EXPECT_FALSE(importer->hasSuccessfulImport()); - EXPECT_FALSE(importer->getImportError().isEmpty()); -} - -TEST_F(OracleImporterTest, MissingSourceBytesCannotProduceSuccessfulImport) -{ - ASSERT_TRUE(importer->readSetsFromByteArray(refreshSource())); - const auto selected = importer->getSets(); - importer->releaseSetData(); - importer->getSets() = selected; - EXPECT_EQ(importer->startImport(), 0); - EXPECT_FALSE(importer->hasSuccessfulImport()); - EXPECT_TRUE(importer->getImportError().contains("TST")); -} - -TEST_F(OracleImporterTest, CancellationDuringFinalSetCannotSavePartialDatabase) -{ - ASSERT_TRUE(importer->readSetsFromByteArray(refreshSource())); - QObject::connect( - importer, &OracleImporter::setIndexChanged, importer, - [this](int, int, const QString &name) { - if (!name.isEmpty()) { - importer->cancelImport(); - } - }, - Qt::DirectConnection); - EXPECT_EQ(importer->startImport(), 0); - EXPECT_FALSE(importer->hasSuccessfulImport()); - EXPECT_TRUE(importer->getCardList().isEmpty()); -} - -TEST_F(OracleImporterTest, CancellationBeforeWorkerStartsIsNotLost) -{ - ASSERT_TRUE(importer->readSetsFromByteArray(refreshSource())); - importer->prepareImport(); - importer->cancelImport(); - EXPECT_EQ(importer->startImport(), 0); - EXPECT_FALSE(importer->hasSuccessfulImport()); - importer->prepareImport(); - EXPECT_EQ(importer->startImport(), 1); - EXPECT_TRUE(importer->hasSuccessfulImport()); -} - -TEST_F(OracleImporterTest, NewImportReplacesPreviousInMemoryCards) -{ - ASSERT_TRUE(importer->readSetsFromByteArray(refreshSource("Old Card"))); - ASSERT_EQ(importer->startImport(), 1); - ASSERT_TRUE(importer->readSetsFromByteArray(refreshSource("New Card"))); - EXPECT_FALSE(importer->hasSuccessfulImport()); - ASSERT_EQ(importer->startImport(), 1); - EXPECT_EQ(importer->getCardList().size(), 1); - EXPECT_FALSE(importer->getCardList().contains("Old Card")); - EXPECT_TRUE(importer->getCardList().contains("New Card")); -} - -TEST_F(OracleImporterTest, FailedSourceLoadCannotReusePreviousSuccessfulImport) -{ - ASSERT_TRUE(importer->readSetsFromByteArray(refreshSource())); - ASSERT_EQ(importer->startImport(), 1); - ASSERT_FALSE(importer->readSetsFromByteArray("invalid JSON")); - EXPECT_FALSE(importer->hasSuccessfulImport()); - EXPECT_EQ(importer->startImport(), 0); -} - -TEST_F(OracleImporterTest, SuccessfulRefreshCommitsCompleteXml) -{ - QTemporaryDir dir; - ASSERT_TRUE(dir.isValid()); - const QString path = dir.filePath("cards.xml"); - ASSERT_TRUE(importer->readSetsFromByteArray(refreshSource())); - ASSERT_EQ(importer->startImport(), 1); - importer->releaseSetData(); - ASSERT_TRUE(importer->saveToFile(path, "test-source", "v1")); - QFile file(path); - ASSERT_TRUE(file.open(QIODevice::ReadOnly)); - const QByteArray xml = file.readAll(); - EXPECT_TRUE(xml.contains("New Card")); - EXPECT_TRUE(xml.contains("test-source")); - EXPECT_TRUE(xml.contains("")); -} - -TEST(OracleDatabaseWriter, AcceptsCompleteSupportedVersions) -{ - EXPECT_TRUE(OracleDatabase::validateXml( - "") - .isEmpty()); - EXPECT_TRUE(OracleDatabase::validateXml("") - .isEmpty()); - EXPECT_FALSE( - OracleDatabase::validateXml("") - .isEmpty()); -} - -TEST(OracleDatabaseWriter, InvalidDownloadPreservesInstalledBytes) -{ - QTemporaryDir dir; - ASSERT_TRUE(dir.isValid()); - const QString path = dir.filePath("tokens.xml"); - QFile original(path); - ASSERT_TRUE(original.open(QIODevice::WriteOnly)); - ASSERT_EQ(original.write("installed tokens"), 16); - original.close(); - for (const QByteArray &payload : {QByteArray("server error"), - QByteArray(""), QByteArray()}) { - QString error; - EXPECT_FALSE(OracleDatabase::saveXml(path, payload, &error)); - EXPECT_FALSE(error.isEmpty()); - ASSERT_TRUE(original.open(QIODevice::ReadOnly)); - EXPECT_EQ(original.readAll(), "installed tokens"); - original.close(); - } -} - -TEST(OracleDatabaseWriter, CompleteDownloadReplacesInstalledBytes) -{ - QTemporaryDir dir; - ASSERT_TRUE(dir.isValid()); - const QString path = dir.filePath("cards.xml"); - const QByteArray data(""); - QString error = "previous failure"; - ASSERT_TRUE(OracleDatabase::saveXml(path, data, &error)); - EXPECT_TRUE(error.isEmpty()); - QFile file(path); - ASSERT_TRUE(file.open(QIODevice::ReadOnly)); - EXPECT_EQ(file.readAll(), data); -} - -#ifdef Q_OS_UNIX -TEST(OracleDatabaseWriter, ShortWriteKeepsPreviouslyInstalledDatabase) -{ - QTemporaryDir dir; - ASSERT_TRUE(dir.isValid()); - const QString path = dir.filePath("cards.xml"); - QFile original(path); - ASSERT_TRUE(original.open(QIODevice::WriteOnly)); - ASSERT_EQ(original.write("installed database"), 18); - original.close(); - const QByteArray data = ""; - // A real filesystem write failure, isolated in the death-test child so the - // process-wide file limit never affects other cases or the parent runner. - ASSERT_EXIT( - { - std::signal(SIGXFSZ, SIG_IGN); - struct rlimit limit; - limit.rlim_cur = 128; - limit.rlim_max = 128; - if (setrlimit(RLIMIT_FSIZE, &limit) != 0) { - _exit(2); - } - _exit(OracleDatabase::saveXml(path, data) ? 1 : 0); - }, - ::testing::ExitedWithCode(0), ""); - ASSERT_TRUE(original.open(QIODevice::ReadOnly)); - EXPECT_EQ(original.readAll(), "installed database"); -} -#endif - int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv);