diff --git a/.ci/compile.sh b/.ci/compile.sh index 1db0cacd1..9be8ba741 100755 --- a/.ci/compile.sh +++ b/.ci/compile.sh @@ -175,9 +175,9 @@ fi if [[ $MAKE_SANITIZE ]]; then flags+=("-DENABLE_SANITIZERS=ON") else - # Every leg states the flag explicitly rather than relying on the CMake - # default, so a build directory reused from a sanitized configuration can - # never leak libasan/libubsan into a shipped binary. + # The release-package step re-runs cmake in the same build dir, so an + # ENABLE_SANITIZERS=ON left over from the debug step would otherwise leak + # into the shipped binaries (libasan/libubsan Depends in the DEB). flags+=("-DENABLE_SANITIZERS=OFF") fi if [[ $MAKE_COVERAGE ]]; then diff --git a/.github/workflows/desktop-build.yml b/.github/workflows/desktop-build.yml index d72cb881e..df054ea26 100644 --- a/.github/workflows/desktop-build.yml +++ b/.github/workflows/desktop-build.yml @@ -33,18 +33,10 @@ on: - 'CMakeLists.txt' - 'vcpkg.json' - 'vcpkg' # needed to match submodule bumps (gitlink) - # The legs the pull-request matrix leaves out run here instead, so the legacy distros and - # the slow macOS runner stay covered without gating a pull request on them. Tags still - # build every leg, because a tag is what ships. - schedule: - - cron: '23 3 * * *' # Cancel earlier, unfinished runs of this workflow on the same branch (unless on tag --> release) concurrency: - # The event name is part of the group because a nightly run and a push run on master share - # refs/heads/master but must not cancel each other: the nightly would drop the push's vcpkg - # import, and a merge landing during the nightly would drop that night's coverage. - group: "${{ github.workflow }} @ ${{ github.ref_name }} (${{ github.event_name }})" + group: "${{ github.workflow }} @ ${{ github.ref_name }}" cancel-in-progress: ${{ github.ref_type != 'tag' }} jobs: @@ -149,7 +141,6 @@ jobs: allow-failure: yes package: skip # We are packaged in Arch already - debug_test: yes # see the note on Fedora 44 below - distro: Servatrice_Debian version: 12 @@ -158,47 +149,59 @@ jobs: server_only: yes test: skip + - distro: Debian + version: 12 + + package: DEB + test: skip # Running tests on all distros is superfluous + - distro: Debian version: 13 package: DEB + - distro: Fedora + version: 43 + + package: RPM + test: skip # Running tests on all distros is superfluous + - distro: Fedora version: 44 package: RPM - # Arch and Fedora 44 are the two newest toolchains, and the -Wno-error= exceptions - # in CMakeLists.txt exist for exactly their warnings. They keep compiling Debug: - # 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 + + - distro: Ubuntu + version: 24.04 + + package: DEB + test: skip # Running tests on all distros is superfluous - distro: Ubuntu version: 26.04 package: DEB - test: skip # The sanitizer leg below runs this distro's suite - # The one leg that runs the suite under AddressSanitizer and - # UndefinedBehaviorSanitizer. Debian 13 and Fedora 44 keep covering the - # uninstrumented suite, so nothing is lost by instrumenting this one. - # - # It packages nothing. A sanitized Debug build is not something to ship, and - # carrying the package build here meant 18 minutes of Release compile queued - # behind the instrumented one in a single job. The leg above publishes this - # distro's DEB. + # The one leg that runs the suite under AddressSanitizer and + # UndefinedBehaviorSanitizer. Debian 13 and Fedora 44 keep covering the + # uninstrumented suite, so nothing is lost by instrumenting this one. + sanitize: yes + - distro: Ubuntu version: 26.04 - variant: sanitize # separate image and ccache, same distro + variant: cover # separate image and ccache, same distro + # The one leg that runs instrumented for line coverage and publishes the report as + # a workflow artifact. It is a separate leg rather than a second flag on the + # sanitizer leg because the two instruments answer different questions: the + # sanitizer leg reports what the run did wrong, the coverage leg reports what the + # run never touched at all. Combining them would also slow both down. Packaging is + # skipped because the leg above already publishes the DEB for this distro. + coverage: yes package: skip - sanitize: yes name: ${{ matrix.distro }} ${{ matrix.version }}${{ matrix.variant && format(' {0}', matrix.variant) || '' }} needs: configure - # The nightly legs live in build-linux-nightly below. A leg is in exactly one of the two - # matrices, so nothing that reviews a change waits for a distro nobody is changing for. - if: github.event_name != 'schedule' runs-on: ubuntu-latest # https://github.com/actions/runner-images continue-on-error: ${{ matrix.allow-failure == 'yes' }} timeout-minutes: 70 @@ -209,18 +212,8 @@ jobs: CMAKE_GENERATOR: 'Ninja' NAME: ${{ matrix.distro }}${{ matrix.version }} IMAGE_SUFFIX: ${{ matrix.variant }} - # Which build the suite runs in, decided once per leg here so the steps' if: conditions - # and their shells cannot drift apart. A step's own env is not visible to its if:, which - # is why these live at job level rather than on the release step. - # DEBUG_TEST the instrumented legs (symbols and instrumentation matter) and the two - # newest distro toolchains, which only fail on a warning in Debug. - # RUN_TESTS every other leg that tests runs the suite against the release build it may - # 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') }} - # Shared with build-linux-nightly so the two leg sets run the same steps. - steps: &linux-steps + steps: - name: "Checkout" uses: actions/checkout@v7 @@ -235,11 +228,8 @@ jobs: shell: bash run: source .ci/docker.sh --build - # The instrumented legs and the two newest distro toolchains build and test Debug; every - # other leg runs its suite against the release build below, so testing and packaging cost - # one compile instead of two. - name: "Build debug and test" - if: env.DEBUG_TEST == 'true' + if: matrix.test != 'skip' shell: bash env: sanitize: '${{ matrix.sanitize }}' @@ -279,23 +269,15 @@ jobs: path: coverage/ if-no-files-found: warn - # One build serves both purposes: the suite runs against the same configuration that - # gets packaged, so a packaging leg no longer pays for a full Debug compile it then - # throws away. macOS and Windows have always tested their release build. - # - # Instrumented and debug-test legs reach this step only when they also package; their - # suite already ran in the debug step above, so the tests are not repeated here. - - name: "Build release, test and package" + - name: "Build release package" id: build - if: matrix.package != 'skip' || env.RUN_TESTS == 'true' + if: matrix.package != 'skip' shell: bash env: SUFFIX: '-${{ matrix.distro }}${{ matrix.version }}' package: '${{ matrix.package }}' server_only: '${{ matrix.server_only }}' - # Only a leg that packages builds the shipping configuration; a release build that - # only runs tests stays free of the crash reporting SDK. - WITH_CRASH_REPORTING: ${{ matrix.package != 'skip' && 'ON' || '' }} + WITH_CRASH_REPORTING: 'ON' # Tag builds only: artifacts from master pushes and PR branches must not report to # Sentry. Forks get no secrets on pull_request, so they keep the mock fallback either way. # configure.outputs.tag is set for ANY tag, so beta (Development) tags get the DSN @@ -305,7 +287,6 @@ jobs: source .ci/docker.sh args=() [[ $server_only == yes ]] && args+=(--no-client) - [[ $RUN_TESTS == true ]] && args+=(--test) [[ $GITHUB_REF == "refs/heads/master" ]] && args+=(--evict-ccache "$CCACHE_EVICTION_AGE") args+=(--ccache "$CCACHE_SIZE") args+=(--cmake-generator "$CMAKE_GENERATOR") @@ -315,14 +296,7 @@ jobs: # configures, and empty keeps the mock fallback. export RUN_ARGS="${RUN_ARGS:-} -e COCKATRICE_SENTRY_DSN" - # --package takes an optional type, so the flag and its argument have to travel - # together or the type would become the next flag. - package_args=() - [[ -n $package && $package != skip ]] && package_args+=(--package "$package") - - # compile.sh runs the tests before the package target, so a failing suite stops the - # leg before anything is built into an installer. - RUN --server --release "${package_args[@]}" "${args[@]}" + RUN --server --release --package "$package" "${args[@]}" - name: "Install sentry-cli" if: matrix.package != 'skip' && needs.configure.outputs.tag != null @@ -424,72 +398,30 @@ jobs: GH_TOKEN: ${{ github.token }} run: gh attestation verify "$BUILD_PATH" --repo Cockatrice/Cockatrice - build-linux-nightly: - strategy: - fail-fast: false - matrix: - include: - - distro: Debian - version: 12 # previous stable, still covered by the nightly - - package: DEB - test: skip # Running tests on all distros is superfluous - - - distro: Fedora - version: 43 # previous release, still covered by the nightly - - package: RPM - test: skip # Running tests on all distros is superfluous - - - distro: Ubuntu - version: 24.04 # previous LTS, still covered by the nightly - - package: DEB - test: skip # Running tests on all distros is superfluous - - # The one leg that runs instrumented for line coverage and publishes the report as - # a workflow artifact. It stays a separate leg rather than a second flag on the - # sanitizer leg because the two instruments answer different questions: the - # sanitizer leg reports what the run did wrong, the coverage leg reports what the - # run never touched at all. Combining them would also slow both down. - # - # It moved here from build-linux: gcovr.cfg sets no fail_under, so the leg cannot - # fail a build, and nothing consumes the report per change, which made it an - # artifact nobody read on every pull request. On the default branch it becomes a - # trend instead. Image and ccache keys come from distro, version and variant, so - # they are unchanged and the warm cache that leg built still matches. - - distro: Ubuntu - version: 26.04 - variant: cover # separate image and ccache, same distro - - coverage: yes - package: skip - # A report must not redden the nightly run: vcpkg-cache-import only imports the - # archives of a run whose conclusion is success. - allow-failure: yes - - name: ${{ matrix.distro }} ${{ matrix.version }}${{ matrix.variant && format(' {0}', matrix.variant) || '' }} - needs: configure - # A nightly and a tag build cover the legs a pull request or a merge does not wait for: - # the previous release of each distro, and the one instrument that only reports. - if: github.event_name == 'schedule' || github.ref_type == 'tag' - runs-on: ubuntu-latest # https://github.com/actions/runner-images - continue-on-error: ${{ matrix.allow-failure == 'yes' }} - timeout-minutes: 70 - env: - CACHE: ${{ github.workspace }}/.cache/${{ matrix.distro }}${{ matrix.version }}${{ matrix.variant }} # directory for caching docker image and ccache - CCACHE_EVICTION_AGE: 7d - 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 }} - steps: *linux-steps - build-vcpkg: strategy: fail-fast: false matrix: include: + - os: macOS + target: 13 # EOL 2025-09-15 + runner: macos-15-intel # https://github.com/actions/runner-images/blob/main/images/macos/macos-15-Readme.md + # macos-26-intel is very slow and fails in CPack during DMG config if not increasing Finder timeout + + ccache_eviction_age: 7d + cmake_generator: Ninja + make_package: 1 + override_target: 13 + package_suffix: "-macOS13_Intel" + qt_version: 6.11.* + qt_modules: qtimageformats qtmultimedia qtshadertools qtwebsockets + soc: Intel + type: Release + 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" + - os: macOS target: 14 # EOL 2026-?? runner: macos-26 # https://github.com/actions/runner-images/blob/main/images/macos/macos-26-arm64-Readme.md @@ -565,16 +497,13 @@ jobs: name: ${{ matrix.os }} ${{ matrix.target }}${{ matrix.soc == 'Intel' && ' Intel' || '' }}${{ matrix.type == 'Debug' && ' Debug' || '' }} needs: [configure, vcpkg-baseline] - # The nightly leg lives in build-vcpkg-nightly below, next to the Linux nightly legs. - if: github.event_name != 'schedule' runs-on: ${{ matrix.runner }} timeout-minutes: 100 env: CCACHE_DIR: ${{ github.workspace }}/.cache/ 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 + steps: - name: "Checkout" uses: actions/checkout@v7 with: @@ -1049,42 +978,6 @@ jobs: GH_TOKEN: ${{ github.token }} run: gh attestation verify "$BUILD_PATH" --repo Cockatrice/Cockatrice - build-vcpkg-nightly: - strategy: - fail-fast: false - matrix: - include: - - os: macOS - target: 13 # EOL 2025-09-15 - runner: macos-15-intel # https://github.com/actions/runner-images/blob/main/images/macos/macos-15-Readme.md - # macos-26-intel is very slow and fails in CPack during DMG config if not increasing Finder timeout - - ccache_eviction_age: 7d - cmake_generator: Ninja - make_package: 1 - override_target: 13 - package_suffix: "-macOS13_Intel" - qt_version: 6.11.* - qt_modules: qtimageformats qtmultimedia qtshadertools qtwebsockets - soc: Intel - type: Release - 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 - # The EOL target on the slowest runner is too slow to gate a change, so it runs in the - # nightly and on tags only, where the vcpkg cache import also has to accept schedule runs. - if: github.event_name == 'schedule' || github.ref_type == 'tag' - runs-on: ${{ matrix.runner }} - timeout-minutes: 100 - env: - CCACHE_DIR: ${{ github.workspace }}/.cache/ - 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: name: Prune caches # Only master writes caches at all, so only master may clean them up too @@ -1103,9 +996,8 @@ jobs: shell: bash env: GH_TOKEN: ${{ github.token }} - # Must list every vcpkg_scope of the build-vcpkg and build-vcpkg-nightly - # matrices above, or the entries of a scope that is still in use get deleted - # on the next run. + # Must list every vcpkg_scope of the build-vcpkg matrix 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/' run: | gh api --paginate "repos/$GITHUB_REPOSITORY/actions/caches?per_page=100" \ diff --git a/.github/workflows/vcpkg-cache-import.yml b/.github/workflows/vcpkg-cache-import.yml index 0a377a8fd..3cc7b2203 100644 --- a/.github/workflows/vcpkg-cache-import.yml +++ b/.github/workflows/vcpkg-cache-import.yml @@ -4,14 +4,12 @@ name: Import vcpkg cache # rebuilds protobuf, openssl and abseil from source and saves them into the shared # cache, where every later run can restore them. # -# Only master pushes and the nightly schedule run on master are imported here: the -# collect step, keys.txt and the zip contents all come from the workflow run being -# imported, so importing from an unmerged pull request would let anyone who can push -# a branch — or a bot actor such as Dependabot, whose PRs are same-repo — promote -# arbitrary binaries into the scope that master and tag builds restore and link into -# signed releases. Forks build untrusted code and must never reach that scope either. -# A scheduled run is the default branch at the commit a master push already built, so -# it carries the same trust as a push. +# Only master pushes are imported here: the collect step, keys.txt and the zip +# contents all come from the workflow run being imported, so importing from an +# unmerged pull request would let anyone who can push a branch — or a bot actor +# such as Dependabot, whose PRs are same-repo — promote arbitrary binaries into +# the scope that master and tag builds restore and link into signed releases. +# Forks build untrusted code and must never reach that scope either. # # repository_dispatch is not used on purpose: it would also resolve to the default branch, # but it counts as a low-trust trigger and may only write there when it declares a @@ -40,7 +38,7 @@ jobs: # the shared scope even if they somehow target the default branch. if: >- github.event.workflow_run.conclusion == 'success' && - (github.event.workflow_run.event == 'push' || github.event.workflow_run.event == 'schedule') && + github.event.workflow_run.event == 'push' && github.event.workflow_run.head_branch == 'master' && github.event.workflow_run.head_repository.full_name == github.repository && !endsWith(github.event.workflow_run.actor.login, '[bot]') && diff --git a/.gitmodules b/.gitmodules index e69c8e686..732fec7ca 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,6 +4,3 @@ [submodule "doxygen-awesome-css"] path = doc/doxygen/theme url = https://github.com/jothepro/doxygen-awesome-css.git -[submodule "external/qttesting"] - path = external/qttesting - url = https://github.com/Kitware/QtTesting.git diff --git a/CMakeLists.txt b/CMakeLists.txt index 563f174e8..677ef0c12 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -20,8 +20,6 @@ option(WITH_CLIENT "Build Cockatrice client" ON) option(WITH_ORACLE "Build Cockatrice card database tool (Oracle)" ON) # Compile Servatrice option(WITH_SERVER "Build Cockatrice server (Servatrice)" OFF) -# Compile GUI tests (QtTesting widget-level playback) -option(WITH_GUI_TEST "Build the QtTesting-based GUI test harness" OFF) # Compile tests option(TEST "Build tests" OFF) # Build with AddressSanitizer and UndefinedBehaviorSanitizer @@ -31,14 +29,6 @@ option(ENABLE_COVERAGE "Build instrumented for GCC line coverage" OFF) # Use vcpkg regardless of OS option(USE_VCPKG "Use vcpkg regardless of OS" OFF) -# The GUI test harness lives in a submodule; a checkout without it must still configure. -if(WITH_GUI_TEST AND NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/external/qttesting/CMakeLists.txt") - message(WARNING "WITH_GUI_TEST is ON but external/qttesting is missing, so GUI tests are disabled.\n" - "Run 'git submodule update --init external/qttesting' to enable them." - ) - set(WITH_GUI_TEST OFF) -endif() - # Default to "Release" build type # User-provided value for CMAKE_BUILD_TYPE must be checked before the PROJECT() call if(DEFINED CMAKE_BUILD_TYPE) @@ -473,34 +463,6 @@ if(WITH_ORACLE) set(CPACK_INSTALL_CMAKE_PROJECTS "Oracle;Oracle;ALL;/" ${CPACK_INSTALL_CMAKE_PROJECTS}) endif() -if(WITH_GUI_TEST) - set(QtTesting_QT_VERSION "6") - # QtTesting registers its own unit tests when BUILD_TESTING is on. They target the - # Qt version the harness was written for and fail on Qt 6.11 (toolbutton - # long-press recording), and they are not Cockatrice's tests, so suppress them - # for the duration of the subdirectory instead of painting ctest red. - if(DEFINED BUILD_TESTING) - set(_cockatrice_saved_build_testing ${BUILD_TESTING}) - endif() - set(BUILD_TESTING - OFF - CACHE BOOL "Build the testing tree." FORCE - ) - add_subdirectory(external/qttesting) - if(DEFINED _cockatrice_saved_build_testing) - set(BUILD_TESTING - ${_cockatrice_saved_build_testing} - CACHE BOOL "Build the testing tree." FORCE - ) - else() - unset(BUILD_TESTING CACHE) - endif() - # cl.exe has no warnings-as-errors here, and it rejects -Wno-error with D8021. - if(NOT MSVC) - target_compile_options(qttesting PRIVATE -Wno-error) - endif() -endif() - if(TEST) include(CTest) add_subdirectory(tests) diff --git a/Testing/README.md b/Testing/README.md deleted file mode 100644 index cb990ec29..000000000 --- a/Testing/README.md +++ /dev/null @@ -1,66 +0,0 @@ -# GUI tests - -Widget-level playback tests for the Cockatrice client, driven by -[QtTesting](https://github.com/Kitware/QtTesting) (`external/qttesting`, a submodule). -Each `*_test.xml` file is a list of events that QtTesting replays against the running -application; a test passes when every event is handled without error. - -## Running - -```bash -git submodule update --init external/qttesting # once -cmake -S . -B build/debug -DCMAKE_BUILD_TYPE=Debug -DWITH_SERVER=0 -DWITH_GUI_TEST=ON -cmake --build build/debug -j 24 - -./Testing/run_test.sh --all # headless, offscreen -./Testing/run_test.sh Testing/smoke_test.xml # one file -./Testing/run_test.sh --visual Testing/smoke_test.xml # shows the window -``` - -`WITH_GUI_TEST` is on by default; a checkout without the submodule configures with a -warning and no harness. `COCKATRICE_BIN` points the runner at another build, and -`TEST_TIMEOUT` (seconds, default 180) bounds a single test — a failing event inside a -modal dialog can otherwise leave the process alive. The client also hard-exits a few -seconds after playback returns, whatever the result. - -The tests are **not** part of `ctest`: they need a card database and a developer's -settings, so they stay a local loop for now. - -## Caveats - -- **Not hermetic.** The client reads the local settings and card database, so a test can - pass on one machine and fail on another. Avoid asserting things that restored - workspace state or user preferences decide (for example the number of open tabs). -- **English labels only.** Menu entries are addressed by their (translated) text, so the - tests expect the source language. Widgets are addressed by objectName paths, which are - stable across translations. -- Actions that open a dialog are followed by a `pause` event: playback continues in the - dialog's own event loop, and the pause gives the UI time to build itself. -- Widget paths include Qt-internal names where a QTabWidget is involved - (`qt_tabwidget_stackedwidget`), and the deck views in a game are indexed - (`tabbedDeckView_0`, `tabbedDeckView_1`). - -## Writing a test - -A test is a `` list. Widgets are named by their path from the main window, e.g. -`MainWindow/tabSupervisor/stackedWidget/TabGame/phasesMenu`. Useful commands: - -| Command | Arguments | Effect | -|---------|-----------|--------| -| `activate` | action or menu name | triggers a QAction (menus are opened first) | -| `key` | Qt key code | presses that key on the focused widget | -| `pause` | milliseconds | waits, letting the UI settle | -| `clickCard` | `card,zone` | double click the card found in that zone (plays it from hand) | -| `flipCard` | `card,zone` | right-click → *Turn Over* | -| `cardAction` | `card,zone,label` | right-click → first menu entry whose label contains `label` (submenus included) | -| `moveCard` | `card,srcZone,dstZone` | drags the card onto the destination zone | -| `drawArrow` | `srcCard,srcZone,dstCard,dstZone` | right-drag an arrow between two cards | -| `createToken` | `name,color,pt` | creates a token as the active local player | -| `selectItem` | `item label` | selects the tree/list entry with that label | -| `typeText` | `text` | types text one key event at a time | -| `expectProperty` | `property,value` | fails playback unless the widget property matches | - -The card, item and property commands are `pqCockatriceEventPlayer` -(`cockatrice/src/game_graphics/pq_cockatrice_event_player.h`). They resolve cards by name -and zone through the `QGraphicsScene` and send real mouse events at the card's current -viewport position, so a test does not depend on the window size or screen resolution. diff --git a/Testing/card_actions_test.xml b/Testing/card_actions_test.xml deleted file mode 100644 index 8d6de4c2b..000000000 --- a/Testing/card_actions_test.xml +++ /dev/null @@ -1,69 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/Testing/deck_editor_open_test.xml b/Testing/deck_editor_open_test.xml deleted file mode 100644 index 437c4bd23..000000000 --- a/Testing/deck_editor_open_test.xml +++ /dev/null @@ -1,4 +0,0 @@ - - - - diff --git a/Testing/deck_editor_search_test.xml b/Testing/deck_editor_search_test.xml deleted file mode 100644 index 0b9f88fa1..000000000 --- a/Testing/deck_editor_search_test.xml +++ /dev/null @@ -1,14 +0,0 @@ - - - - - - - - - - - - - - diff --git a/Testing/dialog_lifecycle_test.xml b/Testing/dialog_lifecycle_test.xml deleted file mode 100644 index 00510b802..000000000 --- a/Testing/dialog_lifecycle_test.xml +++ /dev/null @@ -1,11 +0,0 @@ - - - - - - - - - - - diff --git a/Testing/game_mechanics_test.xml b/Testing/game_mechanics_test.xml deleted file mode 100644 index efef78707..000000000 --- a/Testing/game_mechanics_test.xml +++ /dev/null @@ -1,101 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/Testing/integration_test.xml b/Testing/integration_test.xml deleted file mode 100644 index 5fc7a971a..000000000 --- a/Testing/integration_test.xml +++ /dev/null @@ -1,45 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/Testing/local_game_lifecycle_test.xml b/Testing/local_game_lifecycle_test.xml deleted file mode 100644 index bdc964921..000000000 --- a/Testing/local_game_lifecycle_test.xml +++ /dev/null @@ -1,178 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/Testing/menu_open_close_test.xml b/Testing/menu_open_close_test.xml deleted file mode 100644 index d894b47b5..000000000 --- a/Testing/menu_open_close_test.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - - - - - - - - - - - - - - - - - diff --git a/Testing/nonmodal_manage_sets_test.xml b/Testing/nonmodal_manage_sets_test.xml deleted file mode 100644 index 5fe2bfc0c..000000000 --- a/Testing/nonmodal_manage_sets_test.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - - - - diff --git a/Testing/nonmodal_tip_test.xml b/Testing/nonmodal_tip_test.xml deleted file mode 100644 index 0f4ffc825..000000000 --- a/Testing/nonmodal_tip_test.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - - - - diff --git a/Testing/nonmodal_view_log_test.xml b/Testing/nonmodal_view_log_test.xml deleted file mode 100644 index 3369f1fd5..000000000 --- a/Testing/nonmodal_view_log_test.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - - - - diff --git a/Testing/run_test.sh b/Testing/run_test.sh deleted file mode 100755 index 8ff3fe028..000000000 --- a/Testing/run_test.sh +++ /dev/null @@ -1,118 +0,0 @@ -#!/bin/bash -# Run QtTesting XML test files against the Cockatrice build. -# -# Usage: -# ./Testing/run_test.sh [--visual] [--all | ...] -# -# Options: -# --visual Run with native display (shows the GUI window). -# Default is headless (QT_QPA_PLATFORM=offscreen). -# --all Run every *.xml file in Testing/. -# -# Environment: -# COCKATRICE_BIN Path to the client binary (default: build/debug/cockatrice/cockatrice, -# or the .app bundle binary on macOS). -# TEST_TIMEOUT Seconds a single test may run before it is killed (default: 180). -# Uses timeout, or gtimeout from coreutils on macOS. -# -# Requires the debug build to exist at build/debug/cockatrice/cockatrice. - -set -euo pipefail -shopt -s nullglob - -REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" -if [ -n "${COCKATRICE_BIN:-}" ]; then - COCKATRICE="$COCKATRICE_BIN" -elif [ "$(uname -s)" = "Darwin" ]; then - # A macOS build produces an app bundle, not a bare executable. - COCKATRICE="$REPO_ROOT/build/debug/cockatrice/cockatrice.app/Contents/MacOS/cockatrice" -else - COCKATRICE="$REPO_ROOT/build/debug/cockatrice/cockatrice" -fi -TEST_DIR="$REPO_ROOT/Testing" -TEST_TIMEOUT="${TEST_TIMEOUT:-180}" - -# Stock macOS has no `timeout`; coreutils provides `gtimeout`. Without either the -# runner still works, but a hung test is not killed. -TIMEOUT=() -if command -v timeout >/dev/null 2>&1; then - TIMEOUT=(timeout --foreground "$TEST_TIMEOUT") -elif command -v gtimeout >/dev/null 2>&1; then - TIMEOUT=(gtimeout --foreground "$TEST_TIMEOUT") -else - echo "Warning: neither timeout nor gtimeout found; tests are not time-limited." >&2 -fi - -if [ ! -f "$COCKATRICE" ]; then - echo "Error: $COCKATRICE not found. Build first:" >&2 - echo " cmake --build build/debug" >&2 - exit 1 -fi - -VISUAL=0 -if [ "${1:-}" = "--visual" ]; then - VISUAL=1 - shift -fi - -if [ "${1:-}" = "--all" ]; then - shift - TEST_FILES=("$TEST_DIR"/*.xml) - if [ ${#TEST_FILES[@]} -eq 0 ]; then - echo "No *.xml files found in $TEST_DIR" >&2 - exit 1 - fi -elif [ $# -ge 1 ]; then - TEST_FILES=("$@") -else - echo "Usage: $0 [--visual] [--all | ...]" >&2 - echo " Run a single test: $0 Testing/smoke_test.xml" >&2 - echo " Run all tests: $0 --all" >&2 - echo " Run visually: $0 --visual Testing/smoke_test.xml" >&2 - echo " Run all visually: $0 --visual --all" >&2 - exit 1 -fi - -PASS=0 -FAIL=0 -FAILED_TESTS=() - -run_one() { - local file="$1" - - local -a cmd=() - if [ "${#TIMEOUT[@]}" -gt 0 ]; then - cmd=("${TIMEOUT[@]}") - fi - cmd+=("$COCKATRICE" --no-connect --test "$file") - - if [ "$VISUAL" -eq 1 ]; then - "${cmd[@]}" - else - env QT_QPA_PLATFORM=offscreen "${cmd[@]}" - fi -} - -for f in "${TEST_FILES[@]}"; do - name="$(basename "$f")" - printf "%-40s " "$name" - if run_one "$f"; then - echo "PASS" - PASS=$((PASS + 1)) - else - echo "FAIL" - FAIL=$((FAIL + 1)) - FAILED_TESTS+=("$name") - fi -done - -TOTAL=$((PASS + FAIL)) -echo "" -echo "Results: $PASS/$TOTAL passed, $FAIL failed" -if [ "$FAIL" -gt 0 ]; then - echo "Failed:" - for t in "${FAILED_TESTS[@]}"; do - echo " $t" - done - exit 1 -fi diff --git a/Testing/settings_dialog_test.xml b/Testing/settings_dialog_test.xml deleted file mode 100644 index 2dd9dda0a..000000000 --- a/Testing/settings_dialog_test.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - - - - diff --git a/Testing/settings_navigation_test.xml b/Testing/settings_navigation_test.xml deleted file mode 100644 index c112966e0..000000000 --- a/Testing/settings_navigation_test.xml +++ /dev/null @@ -1,33 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/Testing/settings_search_test.xml b/Testing/settings_search_test.xml deleted file mode 100644 index ba60251f0..000000000 --- a/Testing/settings_search_test.xml +++ /dev/null @@ -1,13 +0,0 @@ - - - - - - - - - - - - - diff --git a/Testing/smoke_test.xml b/Testing/smoke_test.xml deleted file mode 100644 index a31edbdd4..000000000 --- a/Testing/smoke_test.xml +++ /dev/null @@ -1,8 +0,0 @@ - - - - - - - - diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index 9a3172aa8..d5e383d95 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -450,8 +450,6 @@ set(cockatrice_SOURCES src/interface/intents/intent_open_shared_deck.h src/interface/intents/url_parser.cpp src/interface/intents/url_parser.h - src/interface/widgets/server/user/user_games_model.cpp - src/interface/widgets/server/user/user_games_model.h src/interface/widgets/server/user/user_info_popup.cpp src/interface/widgets/server/user/user_info_popup.h ) @@ -567,14 +565,6 @@ target_link_libraries( cockatrice_target_enable_crash_reporting(cockatrice) -if(WITH_GUI_TEST) - # These two translate between XML test files and the QtTesting event pipeline; - # they need the QtTesting headers, so they only exist with the harness. - target_sources(cockatrice PRIVATE src/game_graphics/pq_cockatrice_event_player.cpp src/interface/recording.cpp) - target_link_libraries(cockatrice PRIVATE QtTesting) - target_compile_definitions(cockatrice PRIVATE WITH_GUI_TEST) -endif() - if(UNIX) if(APPLE) set(MACOSX_BUNDLE_INFO_STRING "${PROJECT_NAME}") diff --git a/cockatrice/resources/config/qtlogging.ini b/cockatrice/resources/config/qtlogging.ini index e4b845af9..7ac0d9ca4 100644 --- a/cockatrice/resources/config/qtlogging.ini +++ b/cockatrice/resources/config/qtlogging.ini @@ -10,7 +10,6 @@ # or set = false to disable logging #main = true -#recording = true #qt_translator = true #window_main.* = true #release_channel = true diff --git a/cockatrice/src/client/network/interfaces/deck_analysis_url.h b/cockatrice/src/client/network/interfaces/deck_analysis_url.h deleted file mode 100644 index 425056270..000000000 --- a/cockatrice/src/client/network/interfaces/deck_analysis_url.h +++ /dev/null @@ -1,42 +0,0 @@ -/** - * @file deck_analysis_url.h - * @ingroup ApiInterfaces - */ - -#ifndef DECK_ANALYSIS_URL_H -#define DECK_ANALYSIS_URL_H - -#include -#include - -/** - * Validates a deck address returned by a deck analysis service. - * - * Only the expected host (optionally "www."-prefixed), HTTPS, default ports and - * URLs without user information are accepted. Legacy HTTP addresses for the same - * service are upgraded to HTTPS in place instead of being rejected. - * - * Relative addresses must be resolved against the response URL by the caller. - * - * @return true when @p url names a deck on @p serviceHost; false otherwise. - */ -inline bool normalizeAnalysisUrl(QUrl &url, const QString &serviceHost) -{ - const QString host = url.host().toLower(); - const bool legacyHttp = url.scheme() == QStringLiteral("http"); - const int defaultPort = legacyHttp ? 80 : 443; - if (!url.isValid() || (!legacyHttp && url.scheme() != QStringLiteral("https")) || !url.userInfo().isEmpty() || - !url.path().startsWith('/') || (url.port() != -1 && url.port() != defaultPort) || - (host != serviceHost && host != QStringLiteral("www.") + serviceHost)) { - return false; - } - // Both services accept HTTPS. Older public metadata may still identify the - // same deck with HTTP; retain its host/path/query while using HTTPS to open it. - if (legacyHttp) { - url.setScheme(QStringLiteral("https")); - url.setPort(-1); - } - return true; -} - -#endif diff --git a/cockatrice/src/client/network/interfaces/deck_stats_interface.cpp b/cockatrice/src/client/network/interfaces/deck_stats_interface.cpp index 22be120c4..508f0bdc9 100644 --- a/cockatrice/src/client/network/interfaces/deck_stats_interface.cpp +++ b/cockatrice/src/client/network/interfaces/deck_stats_interface.cpp @@ -1,6 +1,5 @@ #include "deck_stats_interface.h" -#include "deck_analysis_url.h" #include "libcockatrice/card/card_info.h" #include "libcockatrice/card/database/card_database_querier.h" #include "libcockatrice/deck_list/tree/inner_deck_list_node.h" @@ -11,14 +10,12 @@ #include #include #include -#include #include #include #include #include -#include #include -#include +#include #include #include #include @@ -32,56 +29,30 @@ DeckStatsInterface::DeckStatsInterface(QObject *parent) : QObject(parent) connect(manager, &QNetworkAccessManager::finished, this, &DeckStatsInterface::queryFinished); } -void DeckStatsInterface::showError(const QString &message) -{ - // The notice belongs to the originating editor, not this one-shot request. - // Retire the request before opening UI: opening a window may reenter Qt. - QWidget *owner = qobject_cast(parent()); - auto *notice = new QMessageBox(QMessageBox::Critical, tr("Deck analysis"), message, QMessageBox::Ok, owner); - notice->setTextFormat(Qt::PlainText); - notice->setTextInteractionFlags(Qt::TextSelectableByMouse | Qt::TextSelectableByKeyboard); - notice->setAttribute(Qt::WA_DeleteOnClose); - deleteLater(); - notice->open(); -} - void DeckStatsInterface::queryFinished(QNetworkReply *reply) { - if (replyHandled) { - return; - } - replyHandled = true; - // All terminal paths release the reply before showing UI or invoking an - // external URL handler, either of which may destroy the originating editor. - reply->deleteLater(); if (reply->error() != QNetworkReply::NoError) { - showError(reply->errorString()); + QMessageBox::critical(nullptr, tr("Error"), reply->errorString()); + reply->deleteLater(); + deleteLater(); return; } - const QString data = QString::fromUtf8(reply->readAll()); - static const QRegularExpression rx( - R"(]*\bproperty\s*=\s*(['"])og:url\1)[^>]*\bcontent\s*=\s*(['"])(.*?)\2)", - QRegularExpression::CaseInsensitiveOption | QRegularExpression::DotMatchesEverythingOption); - const auto match = rx.match(data); + QString data(reply->readAll()); + reply->deleteLater(); + + static const QRegularExpression rx("url().resolved(QUrl(address, QUrl::StrictMode)); - if (address.trimmed().isEmpty() || !normalizeAnalysisUrl(deckUrl, QStringLiteral("deckstats.net"))) { - showError(tr("The reply from Deckstats contained an invalid deck address.")); - return; - } + QString deckUrl = match.captured(1); + QDesktopServices::openUrl(deckUrl); - QPointer guard(this); deleteLater(); - if (!QDesktopServices::openUrl(deckUrl) && guard) { - showError(tr("Could not open your browser. Open this address to see the analyzed deck:\n%1") - .arg(deckUrl.toDisplayString())); - } } void DeckStatsInterface::getAnalyzeRequestData(const DeckList &deck, QByteArray &data) @@ -89,8 +60,12 @@ void DeckStatsInterface::getAnalyzeRequestData(const DeckList &deck, QByteArray DeckList deckWithoutTokens; copyDeckWithoutTokens(deck, deckWithoutTokens); - data.append("deck=" + QUrl::toPercentEncoding(deckWithoutTokens.writeToString_Plain())); - data.append("&decktitle=" + QUrl::toPercentEncoding(deck.getName())); + QUrl params; + QUrlQuery urlQuery; + urlQuery.addQueryItem("deck", deckWithoutTokens.writeToString_Plain()); + urlQuery.addQueryItem("decktitle", deck.getName()); + params.setQuery(urlQuery); + data.append(params.query(QUrl::EncodeReserved).toUtf8()); } void DeckStatsInterface::analyzeDeck(const DeckList &deck) @@ -109,9 +84,7 @@ void DeckStatsInterface::copyDeckWithoutTokens(const DeckList &source, DeckList { auto copyIfNotAToken = [&destination](const auto node, const auto card) { CardInfoPtr dbCard = CardDatabaseManager::query()->getCardInfo(card->getName()); - // Missing local card data is not evidence that a card is a token. Keep - // its original name and quantity so the service analyzes the whole deck. - if ((!dbCard || !dbCard->getIsToken()) && node->getName() != DECK_ZONE_MAYBEBOARD) { + if (dbCard && !dbCard->getIsToken() && node->getName() != DECK_ZONE_MAYBEBOARD) { DecklistCardNode *addedCard = destination.addCard(card->getName(), node->getName(), -1); addedCard->setNumber(card->getNumber()); } diff --git a/cockatrice/src/client/network/interfaces/deck_stats_interface.h b/cockatrice/src/client/network/interfaces/deck_stats_interface.h index b70646403..e7791075c 100644 --- a/cockatrice/src/client/network/interfaces/deck_stats_interface.h +++ b/cockatrice/src/client/network/interfaces/deck_stats_interface.h @@ -11,7 +11,6 @@ #include class QByteArray; -class QString; class QNetworkAccessManager; class QNetworkReply; class DeckList; @@ -21,8 +20,6 @@ class DeckStatsInterface : public QObject Q_OBJECT private: QNetworkAccessManager *manager; - bool replyHandled = false; - void showError(const QString &message); /** * Deckstats doesn't recognize token cards, and instead tries to find the diff --git a/cockatrice/src/client/network/interfaces/tapped_out_interface.cpp b/cockatrice/src/client/network/interfaces/tapped_out_interface.cpp index a57877b6a..34dd03765 100644 --- a/cockatrice/src/client/network/interfaces/tapped_out_interface.cpp +++ b/cockatrice/src/client/network/interfaces/tapped_out_interface.cpp @@ -1,6 +1,5 @@ #include "tapped_out_interface.h" -#include "deck_analysis_url.h" #include "libcockatrice/card/card_info.h" #include "libcockatrice/card/database/card_database_querier.h" #include "libcockatrice/deck_list/tree/inner_deck_list_node.h" @@ -14,17 +13,15 @@ #include #include #include -#include #include #include #include #include #include #include -#include #include +#include #include -#include #include #include #include @@ -38,67 +35,54 @@ TappedOutInterface::TappedOutInterface(QObject *parent) : QObject(parent) connect(manager, &QNetworkAccessManager::finished, this, &TappedOutInterface::queryFinished); } -void TappedOutInterface::showError(const QString &message) -{ - // The notice belongs to the originating editor, not this one-shot request. - // Retire the request before opening UI: opening a window may reenter Qt. - QWidget *owner = qobject_cast(parent()); - auto *notice = new QMessageBox(QMessageBox::Critical, tr("Deck analysis"), message, QMessageBox::Ok, owner); - notice->setTextFormat(Qt::PlainText); - notice->setTextInteractionFlags(Qt::TextSelectableByMouse | Qt::TextSelectableByKeyboard); - notice->setAttribute(Qt::WA_DeleteOnClose); - deleteLater(); - notice->open(); -} - void TappedOutInterface::queryFinished(QNetworkReply *reply) { - if (replyHandled) { - return; - } - replyHandled = true; - // All terminal paths release the reply before showing UI or invoking an - // external URL handler, either of which may destroy the originating editor. - reply->deleteLater(); if (reply->error() != QNetworkReply::NoError) { - showError(reply->errorString()); - return; - } - - const int httpStatus = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); - if (httpStatus >= 200 && httpStatus < 400 && reply->hasRawHeader("Location")) { - const QByteArray location = reply->rawHeader("Location"); - QUrl deckUrl = reply->url().resolved(QUrl::fromEncoded(location, QUrl::StrictMode)); - if (location.trimmed().isEmpty() || !normalizeAnalysisUrl(deckUrl, QStringLiteral("tappedout.net"))) { - showError(tr("The reply from TappedOut contained an invalid deck address.")); - return; - } - QPointer guard(this); + QMessageBox::critical(nullptr, tr("Error"), reply->errorString()); + reply->deleteLater(); deleteLater(); - if (!QDesktopServices::openUrl(deckUrl) && guard) { - showError(tr("Could not open your browser. Open this address to see the analyzed deck:\n%1") - .arg(deckUrl.toDisplayString())); - } return; } - const QString data = QString::fromUtf8(reply->readAll()); - QStringList errorMessageList = {tr("Unable to analyze the deck on TappedOut.")}; - static const QRegularExpression errorsRx("
(.*?)", - QRegularExpression::DotMatchesEverythingOption); - const auto match = errorsRx.match(data); - if (match.hasMatch()) { - static const QRegularExpression itemRx("
  • (.*?)
    • ", QRegularExpression::DotMatchesEverythingOption); - auto items = itemRx.globalMatch(match.captured(1)); - while (items.hasNext()) { - const QString error = QTextDocumentFragment::fromHtml(items.next().captured(1)).toPlainText().simplified(); - if (!error.isEmpty()) { - errorMessageList.append(error); + int httpStatus = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); + if (reply->hasRawHeader("Location")) { + /* + * If the reply contains a "Location" header, a relative URL to the deck on TappedOut + * can be extracted from the header. The http status is a 302 "redirect". + */ + QString deckUrl = reply->rawHeader("Location"); + qCInfo(TappedOutInterfaceLog) << "Tappedout: good reply, http status" << httpStatus << "location" << deckUrl; + QDesktopServices::openUrl("https://tappedout.net" + deckUrl); + } else { + /* + * Otherwise, the deck has not been parsed correctly. Error messages can be extracted + * from the html. Css pseudo selector for errors: $("div.alert-danger > ul > li") + */ + QString data(reply->readAll()); + QStringList errorMessageList = {tr("Unable to analyze the deck.")}; + + static const QRegularExpression rx("
    (.*?)"); + auto match = rx.match(data); + if (match.hasMatch()) { + QString errors = match.captured(1); + static const QRegularExpression rx2("
  • (.*?)
    • "); + static const QRegularExpression rxremove("<[^>]*>"); + auto matchIterator = rx2.globalMatch(errors); + while (matchIterator.hasNext()) { + auto match2 = matchIterator.next(); + errorMessageList.append(match2.captured(1).remove(rxremove).simplified()); } } + + QString errorMessage = errorMessageList.join("\n"); + qCWarning(TappedOutInterfaceLog) << "Tappedout: bad reply, http status" << httpStatus << "size" << data.size() + << "message" << errorMessage; + + QMessageBox::critical(nullptr, tr("Error"), errorMessage); } - qCWarning(TappedOutInterfaceLog) << "TappedOut analysis failed, HTTP status" << httpStatus; - showError(errorMessageList.join('\n')); + + reply->deleteLater(); + deleteLater(); } void TappedOutInterface::getAnalyzeRequestData(const DeckList &deck, QByteArray &data) @@ -106,9 +90,13 @@ void TappedOutInterface::getAnalyzeRequestData(const DeckList &deck, QByteArray DeckList mainboard, sideboard; copyDeckSplitMainAndSide(deck, mainboard, sideboard); - data.append("name=" + QUrl::toPercentEncoding(deck.getName())); - data.append("&mainboard=" + QUrl::toPercentEncoding(mainboard.writeToString_Plain(false, true))); - data.append("&sideboard=" + QUrl::toPercentEncoding(sideboard.writeToString_Plain(false, true))); + QUrl params; + QUrlQuery urlQuery; + urlQuery.addQueryItem("name", deck.getName()); + urlQuery.addQueryItem("mainboard", mainboard.writeToString_Plain(false, true)); + urlQuery.addQueryItem("sideboard", sideboard.writeToString_Plain(false, true)); + params.setQuery(urlQuery); + data.append(params.query(QUrl::EncodeReserved).toUtf8()); } void TappedOutInterface::analyzeDeck(const DeckList &deck) @@ -129,9 +117,7 @@ void TappedOutInterface::copyDeckSplitMainAndSide(const DeckList &source, DeckLi { auto copyMainOrSide = [&mainboard, &sideboard](const auto node, const auto card) { CardInfoPtr dbCard = CardDatabaseManager::query()->getCardInfo(card->getName()); - // Exclude confirmed tokens, but preserve cards missing from the local - // database, including newly released cards in an imported deck. - if ((dbCard && dbCard->getIsToken()) || node->getName() == DECK_ZONE_MAYBEBOARD) { + if (!dbCard || dbCard->getIsToken() || node->getName() == DECK_ZONE_MAYBEBOARD) { return; } diff --git a/cockatrice/src/client/network/interfaces/tapped_out_interface.h b/cockatrice/src/client/network/interfaces/tapped_out_interface.h index 27e8c2273..615991b07 100644 --- a/cockatrice/src/client/network/interfaces/tapped_out_interface.h +++ b/cockatrice/src/client/network/interfaces/tapped_out_interface.h @@ -14,7 +14,6 @@ inline Q_LOGGING_CATEGORY(TappedOutInterfaceLog, "tapped_out_interface"); class QByteArray; -class QString; class QNetworkAccessManager; class QNetworkReply; class DeckList; @@ -30,8 +29,6 @@ class TappedOutInterface : public QObject Q_OBJECT private: QNetworkAccessManager *manager; - bool replyHandled = false; - void showError(const QString &message); void copyDeckSplitMainAndSide(const DeckList &source, DeckList &mainboard, DeckList &sideboard); private slots: diff --git a/cockatrice/src/game_graphics/deckview/deck_view_container.cpp b/cockatrice/src/game_graphics/deckview/deck_view_container.cpp index fe63d7a6f..3bef9e59e 100644 --- a/cockatrice/src/game_graphics/deckview/deck_view_container.cpp +++ b/cockatrice/src/game_graphics/deckview/deck_view_container.cpp @@ -71,8 +71,6 @@ void ToggleButton::setState(bool _state) DeckViewContainer::DeckViewContainer(int _playerId, TabGame *parent) : QWidget(nullptr), visualDeckStorageWidget(nullptr), parentGame(parent), playerId(_playerId) { - setObjectName("deckViewContainer"); - loadLocalButton = new QPushButton; loadRemoteButton = new QPushButton; loadFromClipboardButton = new QPushButton; @@ -82,14 +80,6 @@ DeckViewContainer::DeckViewContainer(int _playerId, TabGame *parent) forceStartGameButton = new QPushButton; sideboardLockButton = new ToggleButton; - loadLocalButton->setObjectName("loadLocalButton"); - loadRemoteButton->setObjectName("loadRemoteButton"); - loadFromClipboardButton->setObjectName("loadFromClipboardButton"); - readyStartButton->setObjectName("readyStartButton"); - forceStartGameButton->setObjectName("forceStartGameButton"); - unloadDeckButton->setObjectName("unloadDeckButton"); - sideboardLockButton->setObjectName("sideboardLockButton"); - connect(loadLocalButton, &QPushButton::clicked, this, &DeckViewContainer::loadLocalDeck); connect(loadRemoteButton, &QPushButton::clicked, this, &DeckViewContainer::loadRemoteDeck); connect(loadFromClipboardButton, &QPushButton::clicked, this, &DeckViewContainer::loadFromClipboard); diff --git a/cockatrice/src/game_graphics/deckview/tabbed_deck_view_container.cpp b/cockatrice/src/game_graphics/deckview/tabbed_deck_view_container.cpp index 75ede0ca8..90cbed9a1 100644 --- a/cockatrice/src/game_graphics/deckview/tabbed_deck_view_container.cpp +++ b/cockatrice/src/game_graphics/deckview/tabbed_deck_view_container.cpp @@ -14,7 +14,6 @@ class TabGame; TabbedDeckViewContainer::TabbedDeckViewContainer(int _playerId, TabGame *parent) : QTabWidget(nullptr), playerId(_playerId), parentGame(parent) { - setObjectName(QString("tabbedDeckView_%1").arg(_playerId)); setTabsClosable(true); connect(this, &QTabWidget::tabCloseRequested, this, &TabbedDeckViewContainer::closeTab); diff --git a/cockatrice/src/game_graphics/game_view.cpp b/cockatrice/src/game_graphics/game_view.cpp index 7a5aa073a..7a51fc97f 100644 --- a/cockatrice/src/game_graphics/game_view.cpp +++ b/cockatrice/src/game_graphics/game_view.cpp @@ -65,7 +65,6 @@ protected: GameView::GameView(GameScene *scene, QWidget *parent) : QGraphicsView(scene, parent), rubberBand(0) { - setObjectName("gameView"); setBackgroundBrush(QBrush(QColor(0, 0, 0))); setRenderHints(QPainter::TextAntialiasing | QPainter::Antialiasing); setViewportUpdateMode(BoundingRectViewportUpdate); diff --git a/cockatrice/src/game_graphics/pq_cockatrice_event_player.cpp b/cockatrice/src/game_graphics/pq_cockatrice_event_player.cpp deleted file mode 100644 index c6376ef75..000000000 --- a/cockatrice/src/game_graphics/pq_cockatrice_event_player.cpp +++ /dev/null @@ -1,628 +0,0 @@ -#include "pq_cockatrice_event_player.h" - -#include "../../interface/widgets/tabs/tab_game.h" -#include "board/card_item.h" -#include "game_scene.h" -#include "game_view.h" -#include "zones/card_zone.h" - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -Q_LOGGING_CATEGORY(PQ_COCKATRICE_EVENT_PLAYER, "cockatrice.guiTest.eventPlayer") - -namespace -{ -/** - * Strips Qt's mnemonic markers so a test can name a menu entry by the label the - * user sees ("Turn Over" rather than "T&urn Over"). - */ -QString normalizeActionText(const QString &text) -{ - QString normalized = text; - normalized.remove('&'); - return normalized.simplified(); -} - -/** - * Maps a printable character onto the key a US keyboard would report for it, so - * typing produces the same QKeyEvents a recording would. - */ -Qt::Key keyForCharacter(QChar character) -{ - if (character.isLetter()) { - return static_cast(Qt::Key_A + (character.toUpper().unicode() - 'A')); - } - if (character.isDigit()) { - return static_cast(Qt::Key_0 + (character.unicode() - '0')); - } - switch (character.unicode()) { - case ' ': - return Qt::Key_Space; - case '\n': - return Qt::Key_Return; - case '\t': - return Qt::Key_Tab; - case '-': - return Qt::Key_Minus; - case '_': - return Qt::Key_Underscore; - case '/': - return Qt::Key_Slash; - case ',': - return Qt::Key_Comma; - case '.': - return Qt::Key_Period; - case '\'': - return Qt::Key_Apostrophe; - case ';': - return Qt::Key_Semicolon; - case ':': - return Qt::Key_Colon; - case '"': - return Qt::Key_QuoteDbl; - case '&': - return Qt::Key_Ampersand; - case '(': - return Qt::Key_ParenLeft; - case ')': - return Qt::Key_ParenRight; - case '[': - return Qt::Key_BracketLeft; - case ']': - return Qt::Key_BracketRight; - default: - return Qt::Key_unknown; - } -} - -/** Depth-first search for the first model index whose label matches @p text. */ -QModelIndex findModelIndexByText(const QAbstractItemModel *model, const QString &text, const QModelIndex &parent = {}) -{ - for (int row = 0; row < model->rowCount(parent); ++row) { - const QModelIndex index = model->index(row, 0, parent); - if (index.data(Qt::DisplayRole).toString() == text) { - return index; - } - const QModelIndex child = findModelIndexByText(model, text, index); - if (child.isValid()) { - return child; - } - } - return {}; -} - -bool needsShift(QChar character) -{ - if (character.isUpper()) { - return true; - } - return QStringLiteral("_:\"&()").contains(character); -} -} // namespace - -pqCockatriceEventPlayer::pqCockatriceEventPlayer(QObject *parent) : pqWidgetEventPlayer(parent) -{ -} - -bool pqCockatriceEventPlayer::playEvent(QObject *object, const QString &command, const QString &arguments, bool &error) -{ - // A known command that fails is a handled-but-failed event: QtTesting logs - // "Unhandled event" when playEvent returns false, so report through `error` - // (the contract playExpectProperty already follows) and still return true. - const auto run = [&error](bool ok) { - if (!ok) { - error = true; - } - return true; - }; - - if (command == "clickCard") { - return run(playClickCard(object, arguments)); - } else if (command == "flipCard") { - return run(playFlipCard(object, arguments)); - } else if (command == "cardAction") { - return run(playCardAction(object, arguments)); - } else if (command == "moveCard") { - return run(playMoveCard(object, arguments)); - } else if (command == "drawArrow") { - return run(playDrawArrow(object, arguments)); - } else if (command == "createToken") { - return run(playCreateToken(object, arguments)); - } else if (command == "selectItem") { - return run(playSelectItem(object, arguments)); - } else if (command == "typeText") { - return run(playTypeText(object, arguments)); - } else if (command == "expectProperty") { - return playExpectProperty(object, arguments, error); - } - return false; -} - -QGraphicsItem * -pqCockatriceEventPlayer::findCardInScene(QGraphicsScene *scene, const QString &cardName, const QString &zoneName) -{ - for (QGraphicsItem *gi : scene->items()) { - if (gi->type() != typeCard) { - continue; - } - auto *card = static_cast(gi); - if (!card->getZone() || card->getZone()->getName() != zoneName) { - continue; - } - QString actualName = card->getName(); - if (actualName == cardName || actualName.endsWith(" " + cardName)) { - return card; - } - } - return nullptr; -} - -QGraphicsItem *pqCockatriceEventPlayer::findZoneInScene(QGraphicsScene *scene, const QString &zoneName) -{ - for (QGraphicsItem *gi : scene->items()) { - if (gi->type() != typeZone) { - continue; - } - auto *zone = static_cast(gi); - if (zone->getLogic() && zone->getLogic()->getName() == zoneName) { - return zone; - } - } - return nullptr; -} - -QAction *pqCockatriceEventPlayer::findActionInMenu(QMenu *menu, const QString &actionText) -{ - if (!menu || actionText.isEmpty()) { - return nullptr; - } - for (QAction *action : menu->actions()) { - if (action->isSeparator()) { - continue; - } - if (normalizeActionText(action->text()).contains(actionText, Qt::CaseInsensitive)) { - return action; - } - if (QMenu *subMenu = action->menu()) { - if (QAction *found = findActionInMenu(subMenu, actionText)) { - return found; - } - } - } - return nullptr; -} - -QMenu *pqCockatriceEventPlayer::visiblePopupMenu() -{ - for (QWidget *w : QApplication::topLevelWidgets()) { - auto *menu = qobject_cast(w); - if (menu && menu->isVisible()) { - return menu; - } - } - return nullptr; -} - -QPoint pqCockatriceEventPlayer::viewportPoint(QGraphicsView *view, const QGraphicsItem *item) -{ - return view->mapFromScene(item->sceneBoundingRect().center()); -} -void pqCockatriceEventPlayer::sendMouseEvent(QGraphicsView *view, - QEvent::Type type, - const QPoint &pos, - Qt::MouseButton button, - Qt::MouseButtons buttons) -{ - QMouseEvent event(type, pos, view->viewport()->mapToGlobal(pos), button, buttons, Qt::NoModifier); - qApp->notify(view->viewport(), &event); -} - -bool pqCockatriceEventPlayer::playClickCard(QObject *object, const QString &arguments) -{ - auto *gv = qobject_cast(object); - if (!gv) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "clickCard: target widget is not a QGraphicsView"; - return false; - } - - QStringList parts = arguments.split(','); - if (parts.size() < 2) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "clickCard: arguments must be 'cardName,zoneName', got:" << arguments; - return false; - } - - QGraphicsScene *scene = gv->scene(); - if (!scene) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "clickCard: no scene on the QGraphicsView"; - return false; - } - - auto *target = findCardInScene(scene, parts[0].trimmed(), parts[1].trimmed()); - if (!target) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) - << "clickCard: card" << parts[0].trimmed() << "not found in zone" << parts[1].trimmed(); - return false; - } - - const QPoint pos = viewportPoint(gv, target); - // A double click is the play gesture from hand: press/release, then the - // double-click event and its release, exactly as a recording produces it. - sendMouseEvent(gv, QEvent::MouseButtonPress, pos, Qt::LeftButton, Qt::LeftButton); - sendMouseEvent(gv, QEvent::MouseButtonRelease, pos, Qt::LeftButton, Qt::LeftButton); - sendMouseEvent(gv, QEvent::MouseButtonDblClick, pos, Qt::LeftButton, Qt::LeftButton); - sendMouseEvent(gv, QEvent::MouseButtonRelease, pos, Qt::LeftButton, Qt::LeftButton); - - return true; -} - -bool pqCockatriceEventPlayer::playFlipCard(QObject *object, const QString &arguments) -{ - auto *gv = qobject_cast(object); - if (!gv) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "flipCard: target widget is not a QGraphicsView"; - return false; - } - - QStringList parts = arguments.split(','); - if (parts.size() < 2) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "flipCard: arguments must be 'cardName,zoneName', got:" << arguments; - return false; - } - - QGraphicsScene *scene = gv->scene(); - if (!scene) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "flipCard: no scene"; - return false; - } - - auto *card = qgraphicsitem_cast(findCardInScene(scene, parts[0].trimmed(), parts[1].trimmed())); - if (!card) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) - << "flipCard: card" << parts[0].trimmed() << "not found in zone" << parts[1].trimmed(); - return false; - } - - return invokeCardMenuAction("flipCard", gv, card, QStringLiteral("Turn Over")); -} - -bool pqCockatriceEventPlayer::playCardAction(QObject *object, const QString &arguments) -{ - auto *gv = qobject_cast(object); - if (!gv) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "cardAction: target widget is not a QGraphicsView"; - return false; - } - - QStringList parts = arguments.split(','); - if (parts.size() < 3) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) - << "cardAction: arguments must be 'cardName,zoneName,actionText', got:" << arguments; - return false; - } - - QGraphicsScene *scene = gv->scene(); - if (!scene) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "cardAction: no scene"; - return false; - } - - auto *card = qgraphicsitem_cast(findCardInScene(scene, parts[0].trimmed(), parts[1].trimmed())); - if (!card) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) - << "cardAction: card" << parts[0].trimmed() << "not found in zone" << parts[1].trimmed(); - return false; - } - - return invokeCardMenuAction("cardAction", gv, card, parts.mid(2).join(',').trimmed()); -} - -bool pqCockatriceEventPlayer::invokeCardMenuAction(const char *command, - QGraphicsView *view, - CardItem *card, - const QString &actionText) -{ - const QPoint pos = viewportPoint(view, card); - sendMouseEvent(view, QEvent::MouseButtonPress, pos, Qt::RightButton, Qt::RightButton); - sendMouseEvent(view, QEvent::MouseButtonRelease, pos, Qt::RightButton, Qt::NoButton); - qApp->processEvents(); - - QMenu *menu = visiblePopupMenu(); - if (!menu) { - qCWarning(PQ_COCKATRICE_EVENT_PLAYER) << command << ": no popup menu appeared after right-click"; - return false; - } - - QAction *action = findActionInMenu(menu, actionText); - if (!action) { - qCWarning(PQ_COCKATRICE_EVENT_PLAYER) << command << ": no context menu action matching" << actionText; - menu->close(); - return false; - } - - action->trigger(); - menu->close(); - qApp->processEvents(); - return true; -} - -bool pqCockatriceEventPlayer::playMoveCard(QObject *object, const QString &arguments) -{ - auto *gv = qobject_cast(object); - if (!gv) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "moveCard: target widget is not a QGraphicsView"; - return false; - } - - QStringList parts = arguments.split(','); - if (parts.size() < 3) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) - << "moveCard: arguments must be 'cardName,srcZoneName,dstZoneName', got:" << arguments; - return false; - } - - const QString cardName = parts[0].trimmed(); - const QString srcZoneName = parts[1].trimmed(); - const QString dstZoneName = parts[2].trimmed(); - - QGraphicsScene *scene = gv->scene(); - if (!scene) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "moveCard: no scene"; - return false; - } - - auto *card = findCardInScene(scene, cardName, srcZoneName); - if (!card) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "moveCard: card" << cardName << "not found in zone" << srcZoneName; - return false; - } - - auto *dstZone = findZoneInScene(scene, dstZoneName); - if (!dstZone) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "moveCard: destination zone" << dstZoneName << "not found"; - return false; - } - - const QPoint srcPos = viewportPoint(gv, card); - const QPoint dstPos = viewportPoint(gv, dstZone); - - // A drag has to travel past Qt's start distance before the scene turns it - // into a card drag item, otherwise the move is interpreted as a click. - const int dragDistance = 2 * QApplication::startDragDistance() + 10; - const QPoint pastDragStart = srcPos + QPoint(dragDistance, 0); - - sendMouseEvent(gv, QEvent::MouseButtonPress, srcPos, Qt::LeftButton, Qt::LeftButton); - sendMouseEvent(gv, QEvent::MouseMove, pastDragStart, Qt::LeftButton, Qt::LeftButton); - qApp->processEvents(); - sendMouseEvent(gv, QEvent::MouseMove, dstPos, Qt::LeftButton, Qt::LeftButton); - qApp->processEvents(); - sendMouseEvent(gv, QEvent::MouseButtonRelease, dstPos, Qt::LeftButton, Qt::NoButton); - qApp->processEvents(); - - return true; -} - -bool pqCockatriceEventPlayer::playDrawArrow(QObject *object, const QString &arguments) -{ - auto *gv = qobject_cast(object); - if (!gv) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "drawArrow: target widget is not a QGraphicsView"; - return false; - } - - QStringList parts = arguments.split(','); - if (parts.size() < 4) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) - << "drawArrow: arguments must be 'srcCard,srcZone,dstCard,dstZone', got:" << arguments; - return false; - } - - const QString srcCardName = parts[0].trimmed(); - const QString srcZoneName = parts[1].trimmed(); - const QString dstCardName = parts[2].trimmed(); - const QString dstZoneName = parts[3].trimmed(); - - QGraphicsScene *scene = gv->scene(); - if (!scene) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "drawArrow: no scene"; - return false; - } - - auto *srcCard = findCardInScene(scene, srcCardName, srcZoneName); - if (!srcCard) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) - << "drawArrow: source card" << srcCardName << "not found in zone" << srcZoneName; - return false; - } - - auto *dstCard = findCardInScene(scene, dstCardName, dstZoneName); - if (!dstCard) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) - << "drawArrow: dest card" << dstCardName << "not found in zone" << dstZoneName; - return false; - } - - const QPoint srcPos = viewportPoint(gv, srcCard); - const QPoint dstPos = viewportPoint(gv, dstCard); - - const int dragDistance = 2 * QApplication::startDragDistance() + 10; - const QPoint pastDragStart = srcPos + QPoint(dragDistance, 0); - - sendMouseEvent(gv, QEvent::MouseButtonPress, srcPos, Qt::RightButton, Qt::RightButton); - sendMouseEvent(gv, QEvent::MouseMove, pastDragStart, Qt::RightButton, Qt::RightButton); - qApp->processEvents(); - sendMouseEvent(gv, QEvent::MouseMove, dstPos, Qt::RightButton, Qt::RightButton); - sendMouseEvent(gv, QEvent::MouseButtonRelease, dstPos, Qt::RightButton, Qt::NoButton); - qApp->processEvents(); - - return true; -} - -bool pqCockatriceEventPlayer::playCreateToken(QObject *object, const QString &arguments) -{ - auto *gv = qobject_cast(object); - if (!gv) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "createToken: target widget is not a QGraphicsView"; - return false; - } - - QStringList parts = arguments.split(','); - if (parts.size() < 3) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "createToken: arguments must be 'name,color,pt', got:" << arguments; - return false; - } - - QString tokenName = parts[0].trimmed(); - QString tokenColor = parts[1].trimmed(); - QString tokenPt = parts[2].trimmed(); - - TabGame *tabGame = nullptr; - QWidget *w = gv; - while (w) { - tabGame = qobject_cast(w); - if (tabGame) { - break; - } - w = w->parentWidget(); - } - if (!tabGame) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "createToken: TabGame not found"; - return false; - } - - AbstractGame *game = tabGame->getGame(); - if (!game) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "createToken: no game"; - return false; - } - - int activePlayerId = game->getGameState()->getActivePlayer(); - PlayerLogic *player = game->getPlayerManager()->getActiveLocalPlayer(activePlayerId); - if (!player) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "createToken: no active local player"; - return false; - } - - TokenInfo info; - info.name = tokenName; - info.color = tokenColor; - info.pt = tokenPt; - info.destroy = true; - info.faceDown = false; - - player->getPlayerActions()->actCreateToken(info); - - for (int i = 0; i < 10; ++i) { - qApp->processEvents(); - } - - return true; -} - -bool pqCockatriceEventPlayer::playSelectItem(QObject *object, const QString &arguments) -{ - const QString text = arguments.trimmed(); - if (text.isEmpty()) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "selectItem: needs the label of the item to select"; - return false; - } - - if (auto *tree = qobject_cast(object)) { - QTreeWidgetItemIterator item(tree); - while (*item) { - if ((*item)->text(0) == text) { - tree->setCurrentItem(*item); - qApp->processEvents(); - return true; - } - ++item; - } - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "selectItem: no tree item labelled" << text; - return false; - } - - if (auto *view = qobject_cast(object)) { - const QModelIndex index = findModelIndexByText(view->model(), text); - if (!index.isValid()) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "selectItem: no item labelled" << text; - return false; - } - view->setCurrentIndex(index); - qApp->processEvents(); - return true; - } - - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "selectItem: target is neither a tree widget nor an item view"; - return false; -} - -bool pqCockatriceEventPlayer::playTypeText(QObject *object, const QString &arguments) -{ - auto *widget = qobject_cast(object); - if (!widget) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "typeText: target is not a QWidget"; - return false; - } - - for (const QChar character : arguments) { - const Qt::Key key = keyForCharacter(character); - const Qt::KeyboardModifiers modifiers = needsShift(character) ? Qt::ShiftModifier : Qt::NoModifier; - QKeyEvent press(QEvent::KeyPress, key, modifiers, QString(character)); - QKeyEvent release(QEvent::KeyRelease, key, modifiers, QString(character)); - qApp->notify(widget, &press); - qApp->notify(widget, &release); - } - qApp->processEvents(); - - return true; -} - -bool pqCockatriceEventPlayer::playExpectProperty(QObject *object, const QString &arguments, bool &error) -{ - if (!object) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "expectProperty: no target object"; - error = true; - return true; - } - - QStringList parts = arguments.split(','); - if (parts.size() < 2) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) - << "expectProperty: arguments must be 'propertyName,expectedValue', got:" << arguments; - error = true; - return true; - } - - const QString propertyName = parts[0].trimmed(); - const QString expected = parts.mid(1).join(','); - - const QVariant actual = object->property(propertyName.toUtf8().constData()); - if (!actual.isValid()) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) - << "expectProperty:" << object->objectName() << "has no property" << propertyName; - error = true; - return true; - } - - if (actual.toString() != expected) { - qCCritical(PQ_COCKATRICE_EVENT_PLAYER) << "expectProperty:" << object->objectName() << propertyName << "is" - << actual.toString() << "but the test expected" << expected; - error = true; - return true; - } - - return true; -} diff --git a/cockatrice/src/game_graphics/pq_cockatrice_event_player.h b/cockatrice/src/game_graphics/pq_cockatrice_event_player.h deleted file mode 100644 index 89d88af77..000000000 --- a/cockatrice/src/game_graphics/pq_cockatrice_event_player.h +++ /dev/null @@ -1,91 +0,0 @@ -#ifndef PQ_COCKATRICE_EVENT_PLAYER_H -#define PQ_COCKATRICE_EVENT_PLAYER_H - -#include "pqWidgetEventPlayer.h" - -#include - -class CardItem; -class QAction; -class QGraphicsItem; -class QGraphicsScene; -class QGraphicsView; -class QMenu; - -/** - * Custom event player for Cockatrice game interactions. - * - * Card commands are resolution independent: the card is looked up in the - * QGraphicsScene by name and zone, then mouse events are sent at the card's - * current viewport position. That keeps recordings usable across screen sizes - * and window geometries. - * - * Commands: - * clickCard "cardName,zoneName" - * left press/release + double click — plays a card from hand, taps a - * permanent, opens the deck view entry, depending on the zone - * flipCard "cardName,zoneName" - * right-click → "Turn Over" (shorthand for cardAction) - * cardAction "cardName,zoneName,actionText" - * right-click the card and trigger the first context menu action whose - * label contains actionText; submenus are searched as well, so - * "Add counter (A)" works - * moveCard "cardName,srcZoneName,dstZoneName" - * left-drag the card onto the destination zone — plays a card, moves it - * to the graveyard/exile, puts it back into hand, ... - * drawArrow "srcCard,srcZone,dstCard,dstZone" - * right-drag an arrow from one card to another - * createToken "name,color,pt" - * create a token through the active local player, as the table context - * menu's "Create token" dialog would - * selectItem "item text" - * selects the first item of a tree/list widget whose label matches, for - * navigating the settings navigation tree or similar label-driven views - * typeText "some text" - * types the text into the widget one key event at a time — use it for - * deck lists and search boxes instead of one event per character - * expectProperty "propertyName,expectedValue" - * assertion: playback fails unless object->property(propertyName) - * equals expectedValue — for example "count,2" on the tab widget - */ -class pqCockatriceEventPlayer : public pqWidgetEventPlayer -{ - Q_OBJECT - typedef pqWidgetEventPlayer Superclass; - -public: - explicit pqCockatriceEventPlayer(QObject *parent = nullptr); - - using Superclass::playEvent; - - bool playEvent(QObject *object, const QString &command, const QString &arguments, bool &error) override; - -private: - bool playClickCard(QObject *object, const QString &arguments); - bool playFlipCard(QObject *object, const QString &arguments); - bool playCardAction(QObject *object, const QString &arguments); - bool playMoveCard(QObject *object, const QString &arguments); - bool playDrawArrow(QObject *object, const QString &arguments); - bool playCreateToken(QObject *object, const QString &arguments); - bool playSelectItem(QObject *object, const QString &arguments); - bool playTypeText(QObject *object, const QString &arguments); - bool playExpectProperty(QObject *object, const QString &arguments, bool &error); - - /** @brief Triggers the context menu entry @p actionText after right-clicking @p card. */ - bool invokeCardMenuAction(const char *command, QGraphicsView *view, CardItem *card, const QString &actionText); - - static QGraphicsItem *findCardInScene(QGraphicsScene *scene, const QString &cardName, const QString &zoneName); - static QGraphicsItem *findZoneInScene(QGraphicsScene *scene, const QString &zoneName); - static QAction *findActionInMenu(QMenu *menu, const QString &actionText); - static QMenu *visiblePopupMenu(); - - /** @brief Viewport position of the item's centre, as the mouse events have to be sent. */ - static QPoint viewportPoint(QGraphicsView *view, const QGraphicsItem *item); - static void sendMouseEvent(QGraphicsView *view, - QEvent::Type type, - const QPoint &pos, - Qt::MouseButton button, - Qt::MouseButtons buttons); -}; - -#endif // PQ_COCKATRICE_EVENT_PLAYER_H diff --git a/cockatrice/src/interface/recording.cpp b/cockatrice/src/interface/recording.cpp deleted file mode 100644 index 2b11255f9..000000000 --- a/cockatrice/src/interface/recording.cpp +++ /dev/null @@ -1,102 +0,0 @@ -#include "recording.h" - -#include "pqEventTypes.h" - -#include -#include -#include -#include - -XMLEventObserver::XMLEventObserver(QObject *parent) : pqEventObserver(parent) -{ -} - -XMLEventObserver::~XMLEventObserver() = default; - -void XMLEventObserver::setStream(QTextStream *stream) -{ - if (xmlStream) { - xmlStream->writeEndElement(); - xmlStream->writeEndDocument(); - xmlStream.reset(); - } - if (Stream) { - *Stream << xmlString; - } - xmlString.clear(); - pqEventObserver::setStream(stream); - if (Stream) { - xmlStream.reset(new QXmlStreamWriter(&xmlString)); - xmlStream->setAutoFormatting(true); - xmlStream->writeStartDocument(); - xmlStream->writeStartElement(QStringLiteral("events")); - } -} - -void XMLEventObserver::onRecordEvent(const QString &widget, - const QString &command, - const QString &arguments, - const int &eventType) -{ - if (!xmlStream) { - return; - } - - xmlStream->writeStartElement(QStringLiteral("event")); - xmlStream->writeAttribute(QStringLiteral("widget"), widget); - if (eventType == pqEventTypes::ACTION_EVENT) { - xmlStream->writeAttribute(QStringLiteral("command"), command); - } else { // pqEventTypes::CHECK_EVENT - xmlStream->writeAttribute(QStringLiteral("property"), command); - } - xmlStream->writeAttribute(QStringLiteral("arguments"), arguments); - xmlStream->writeEndElement(); -} - -XMLEventSource::XMLEventSource(QObject *parent) : pqEventSource(parent) -{ -} - -XMLEventSource::~XMLEventSource() = default; - -void XMLEventSource::setContent(const QString &xmlFileName) -{ - xmlStream.reset(); - - QFile xml(xmlFileName); - if (!xml.open(QIODevice::ReadOnly)) { - qCWarning(RecordingLog) << "Could not read test file" << xmlFileName; - return; - } - - xmlStream.reset(new QXmlStreamReader(xml.readAll())); -} - -int XMLEventSource::getNextEvent(QString &widget, QString &command, QString &arguments, int &eventType) -{ - if (!xmlStream) { - return DONE; - } - - while (!xmlStream->atEnd()) { - if (xmlStream->readNext() != QXmlStreamReader::StartElement || xmlStream->name() != QStringLiteral("event")) { - continue; - } - - const QXmlStreamAttributes attributes = xmlStream->attributes(); - eventType = pqEventTypes::ACTION_EVENT; - widget = attributes.value(QStringLiteral("widget")).toString(); - command = attributes.value(QStringLiteral("command")).toString(); - if (command.isEmpty()) { - // A recorded check event carries a property instead of a command. - command = attributes.value(QStringLiteral("property")).toString(); - } - arguments = attributes.value(QStringLiteral("arguments")).toString(); - return SUCCESS; - } - - if (xmlStream->hasError()) { - qCWarning(RecordingLog) << "Malformed test file:" << xmlStream->errorString(); - } - return DONE; -} diff --git a/cockatrice/src/interface/recording.h b/cockatrice/src/interface/recording.h deleted file mode 100644 index a5d813ea9..000000000 --- a/cockatrice/src/interface/recording.h +++ /dev/null @@ -1,54 +0,0 @@ -#ifndef COCKATRICE_RECORDING_H -#define COCKATRICE_RECORDING_H - -#include "pqEventObserver.h" -#include "pqEventSource.h" - -#include -#include - -inline Q_LOGGING_CATEGORY(RecordingLog, "recording"); - -class QXmlStreamReader; -class QXmlStreamWriter; - -/** - * @brief Writes a session recording as an XML test file. - * - * The events are buffered in memory and flushed to the stream when recording - * ends, because the stream is handed over once the file name is known. - */ -class XMLEventObserver : public pqEventObserver -{ - QScopedPointer xmlStream; - QString xmlString; - -public: - explicit XMLEventObserver(QObject *parent); - ~XMLEventObserver() override; - -protected: - void setStream(QTextStream *stream) override; - void onRecordEvent(const QString &widget, - const QString &command, - const QString &arguments, - const int &eventType) override; -}; - -/** - * @brief Plays the events of an XML test file back, one at a time. - */ -class XMLEventSource : public pqEventSource -{ - QScopedPointer xmlStream; - -public: - explicit XMLEventSource(QObject *parent); - ~XMLEventSource() override; - -protected: - void setContent(const QString &xmlFileName) override; - int getNextEvent(QString &widget, QString &command, QString &arguments, int &eventType) override; -}; - -#endif // COCKATRICE_RECORDING_H diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_connect.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_connect.cpp index 853a00828..1f567989e 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_connect.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_connect.cpp @@ -33,7 +33,6 @@ class QWidget; DlgConnect::DlgConnect(QWidget *parent) : QDialog(parent) { - setObjectName("connectDialog"); previousHostButton = new QRadioButton(tr("Known Hosts"), this); previousHosts = new QComboBox(this); @@ -86,11 +85,6 @@ DlgConnect::DlgConnect(QWidget *parent) : QDialog(parent) passwordLabel->setBuddy(passwordEdit); passwordEdit->setEchoMode(QLineEdit::Password); - hostEdit->setObjectName("connectHostEdit"); - portEdit->setObjectName("connectPortEdit"); - playernameEdit->setObjectName("connectPlayernameEdit"); - passwordEdit->setObjectName("connectPasswordEdit"); - savePasswordCheckBox = new QCheckBox(tr("&Save password")); autoConnectCheckBox = new QCheckBox(tr("A&uto connect")); @@ -296,13 +290,11 @@ void DlgConnect::updateDisplayInfo(const QString &saveName) playernameEdit->setFocus(); savePasswordCheckBox->setChecked(savePasswordStatus); storedVerifier.clear(); - savedVerifier.clear(); if (savePasswordStatus) { const QString stored = _data.at(4); if (stored.startsWith("$")) { storedVerifier = stored; - savedVerifier = stored; passwordEdit->clear(); passwordEdit->setPlaceholderText(tr("Password saved")); } else { @@ -339,7 +331,6 @@ void DlgConnect::newHostSelected(bool state) passwordEdit->clear(); passwordEdit->setPlaceholderText(QString()); storedVerifier.clear(); - savedVerifier.clear(); saveEdit->clear(); saveEdit->setPlaceholderText(tr("Unique Server Name")); saveEdit->setDisabled(false); @@ -355,19 +346,9 @@ void DlgConnect::passwordSaved(QT_STATE_CHANGED_T state) Q_UNUSED(state); if (savePasswordCheckBox->isChecked()) { autoConnectCheckBox->setEnabled(true); - // Re-ticking keeps a password the user has started typing; otherwise restore the saved - // login this dialog was opened with, so unticking and changing your mind is harmless. - if (passwordEdit->text().isEmpty()) { - storedVerifier = savedVerifier; - passwordEdit->setPlaceholderText(savedVerifier.isEmpty() ? QString() : tr("Password saved")); - } } else { autoConnectCheckBox->setChecked(false); autoConnectCheckBox->setEnabled(false); - // Unticking means the password typed below is sent for this connection: stop sending the - // saved verifier instead, and stop claiming a password is saved. - storedVerifier.clear(); - passwordEdit->setPlaceholderText(QString()); } } @@ -375,11 +356,10 @@ void DlgConnect::actOk() { ServersSettings &servers = SettingsCache::instance().servers(); - // Never write a newly typed plaintext password to disk when a verifier is available: the - // typed value is used for this connection and a fresh verifier is persisted after a - // successful login. Without any verifier we keep the previous (plaintext legacy) behavior. - const QString verifierToSave = storedVerifier.isEmpty() ? savedVerifier : storedVerifier; - const QString passwordToSave = verifierToSave.isEmpty() ? passwordEdit->text() : verifierToSave; + // Never write a newly typed plaintext password to disk when a verifier is already stored: + // the typed value is used for this connection and a fresh verifier is persisted after a + // successful login. Without a stored verifier we keep the previous (plaintext legacy) behavior. + const QString passwordToSave = storedVerifier.isEmpty() ? passwordEdit->text() : storedVerifier; if (newHostButton->isChecked()) { if (saveEdit->text().isEmpty()) { diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_connect.h b/cockatrice/src/interface/widgets/dialogs/dlg_connect.h index dad3623f5..bef5f136e 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_connect.h +++ b/cockatrice/src/interface/widgets/dialogs/dlg_connect.h @@ -98,10 +98,6 @@ private: QPushButton *btnConnect, *btnForgotPassword, *btnRefreshServers, *btnDeleteServer; QMap> savedHostList; HandlePublicServers *hps; - //! \brief Saved "$scrypt$..." verifier as loaded for the selected profile, kept so that - //! re-ticking "Save password" after unticking restores it. - QString savedVerifier; - //! \brief Verifier in effect for this connection: empty while "Save password" is unticked. QString storedVerifier; const QString placeHolderText = tr("Downloading..."); }; diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_edit_tokens.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_edit_tokens.cpp index 3845029f7..cbf61a394 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_edit_tokens.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_edit_tokens.cpp @@ -40,7 +40,6 @@ class QWidget; DlgEditTokens::DlgEditTokens(QWidget *parent) : QDialog(parent), currentCard(nullptr) { - setObjectName("editTokensDialog"); nameLabel = new QLabel(tr("&Name:")); nameEdit = new QLineEdit; nameEdit->setMaxLength(MAX_NAME_LENGTH); diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_clipboard.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_clipboard.cpp index c1d0676ca..2c0fd3f6f 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_clipboard.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_clipboard.cpp @@ -28,16 +28,12 @@ class QWidget; */ AbstractDlgDeckTextEdit::AbstractDlgDeckTextEdit(QWidget *parent) : QDialog(parent) { - setObjectName("dlgDeckTextEdit"); - contentsEdit = new QPlainTextEdit; - contentsEdit->setObjectName("contentsEdit"); refreshButton = new QPushButton(tr("&Refresh")); connect(refreshButton, &QPushButton::clicked, this, &AbstractDlgDeckTextEdit::actRefresh); auto *buttonBox = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel); - buttonBox->setObjectName("buttonBox"); buttonBox->addButton(refreshButton, QDialogButtonBox::ActionRole); connect(buttonBox, &QDialogButtonBox::accepted, this, &AbstractDlgDeckTextEdit::actOK); connect(buttonBox, &QDialogButtonBox::rejected, this, &AbstractDlgDeckTextEdit::reject); diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_local_game_options.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_local_game_options.cpp index 3ebb5c093..858deb751 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_local_game_options.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_local_game_options.cpp @@ -16,11 +16,8 @@ class QWidget; DlgLocalGameOptions::DlgLocalGameOptions(QWidget *parent) : QDialog(parent) { - setObjectName("localGameDialog"); - numberPlayersLabel = new QLabel(tr("Players:"), this); numberPlayersEdit = new QSpinBox(this); - numberPlayersEdit->setObjectName("numberPlayersEdit"); numberPlayersEdit->setMinimum(1); numberPlayersEdit->setMaximum(8); numberPlayersEdit->setValue(1); @@ -31,12 +28,10 @@ DlgLocalGameOptions::DlgLocalGameOptions(QWidget *parent) : QDialog(parent) generalGrid->addWidget(numberPlayersLabel, 0, 0); generalGrid->addWidget(numberPlayersEdit, 0, 1); generalGroupBox = new QGroupBox(tr("General"), this); - generalGroupBox->setObjectName("generalGroupBox"); generalGroupBox->setLayout(generalGrid); startingLifeTotalLabel = new QLabel(tr("Starting life total:"), this); startingLifeTotalEdit = new QSpinBox(this); - startingLifeTotalEdit->setObjectName("startingLifeTotalEdit"); startingLifeTotalEdit->setMinimum(-999999999); startingLifeTotalEdit->setMaximum(999999999); startingLifeTotalEdit->setValue(20); @@ -47,14 +42,11 @@ DlgLocalGameOptions::DlgLocalGameOptions(QWidget *parent) : QDialog(parent) gameSetupGrid->addWidget(startingLifeTotalLabel, 0, 0); gameSetupGrid->addWidget(startingLifeTotalEdit, 0, 1); gameSetupOptionsGroupBox = new QGroupBox(tr("Game setup options"), this); - gameSetupOptionsGroupBox->setObjectName("gameSetupOptionsGroupBox"); gameSetupOptionsGroupBox->setLayout(gameSetupGrid); rememberSettingsCheckBox = new QCheckBox(tr("Remember settings"), this); - rememberSettingsCheckBox->setObjectName("rememberSettingsCheckBox"); buttonBox = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel, this); - buttonBox->setObjectName("buttonBox"); connect(buttonBox, &QDialogButtonBox::accepted, this, &DlgLocalGameOptions::actOK); connect(buttonBox, &QDialogButtonBox::rejected, this, &DlgLocalGameOptions::reject); diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_manage_sets.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_manage_sets.cpp index caca5da92..dbfe38e26 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_manage_sets.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_manage_sets.cpp @@ -48,7 +48,6 @@ WndSets::WndSets(QWidget *parent) : QMainWindow(parent) { - setObjectName("manageSetsWindow"); setOrderIsSorted = false; // left toolbar @@ -222,7 +221,6 @@ WndSets::WndSets(QWidget *parent) : QMainWindow(parent) WndSets::~WndSets() { - disconnect(view->header(), &QHeaderView::geometriesChanged, this, &WndSets::saveHeaderState); } void WndSets::closeEvent(QCloseEvent * /*ev*/) diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_settings.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_settings.cpp index 3602e67c6..ab870a2c8 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_settings.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_settings.cpp @@ -73,8 +73,6 @@ DlgSettings::DlgSettings(QWidget *parent) : QDialog(parent) resize(calculateDialogSize(hostSize, available.size(), minimum)); setSizeGripEnabled(true); - setObjectName("settingsDialog"); - connect(&SettingsCache::instance().personal(), &PersonalSettings::langChanged, this, &DlgSettings::updateLanguage); settingsView = new SettingsView(this); @@ -84,7 +82,6 @@ DlgSettings::DlgSettings(QWidget *parent) : QDialog(parent) buttonBox->addStretch(); okButton = new QPushButton; okButton->setDefault(true); - okButton->setObjectName("settingsOkButton"); connect(okButton, &QPushButton::clicked, this, &DlgSettings::close); buttonBox->addWidget(okButton); diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_tip_of_the_day.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_tip_of_the_day.cpp index c1df0c963..511ff1c7d 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_tip_of_the_day.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_tip_of_the_day.cpp @@ -28,7 +28,6 @@ DlgTipOfTheDay::DlgTipOfTheDay(QWidget *parent) : QDialog(parent) { - setObjectName("tipOfTheDayDialog"); successfulInit = false; QString xmlPath = "theme:tips/tips_of_the_day.xml"; tipDatabase = new TipsOfTheDay(xmlPath, this); diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_update.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_update.cpp index 11dcd7e27..ca55c7b41 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_update.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_update.cpp @@ -42,7 +42,6 @@ static const QString UPDATE_INSTALLER_OVERRIDE = "Cockatrice-Update-Override.exe DlgUpdate::DlgUpdate(QWidget *parent) : QDialog(parent) { - setObjectName("updateDialog"); // Handle layout statusLabel = new QLabel(this); diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_view_log.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_view_log.cpp index e75cf352a..d55e800a1 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_view_log.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_view_log.cpp @@ -18,7 +18,6 @@ class QWidget; DlgViewLog::DlgViewLog(QWidget *parent) : QDialog(parent) { - setObjectName("viewLogDialog"); logArea = new QPlainTextEdit; logArea->setReadOnly(true); diff --git a/cockatrice/src/interface/widgets/dialogs/settings_view.cpp b/cockatrice/src/interface/widgets/dialogs/settings_view.cpp index 005824995..14d16b92c 100644 --- a/cockatrice/src/interface/widgets/dialogs/settings_view.cpp +++ b/cockatrice/src/interface/widgets/dialogs/settings_view.cpp @@ -120,8 +120,6 @@ QWidget *fromWidgetId(WidgetId id) SettingsView::SettingsView(QWidget *parent) : QWidget(parent), currentTabIndex(0), searchActive(false) { - setObjectName("settingsView"); - setupUi(); connect(&SettingsCache::instance().personal(), &PersonalSettings::langChanged, this, &SettingsView::retranslateUi); @@ -138,14 +136,12 @@ void SettingsView::setupUi() searchEdit->addAction(themePixmap(QStringLiteral("icons/search")), QLineEdit::LeadingPosition); searchEdit->installEventFilter(this); connect(searchEdit, &QLineEdit::textChanged, this, &SettingsView::onSearchTextChanged); - searchEdit->setObjectName("settingsSearchEdit"); auto *searchLayout = new QHBoxLayout; searchLayout->addWidget(searchEdit); // Pages stacked widget pagesWidget = new QStackedWidget; - pagesWidget->setObjectName("pagesWidget"); auto *generalPage = new GeneralSettingsPage; auto *appearancePage = new AppearanceSettingsPage; @@ -214,7 +210,6 @@ void SettingsView::setupUi() // Pages container (stacked widget + search results overlay) pagesContainer = new QWidget; - pagesContainer->setObjectName("pagesContainer"); auto *containerLayout = new QStackedLayout; containerLayout->setStackingMode(QStackedLayout::StackAll); containerLayout->addWidget(pagesWidget); @@ -254,7 +249,6 @@ void SettingsView::setupUi() void SettingsView::setupNavTree() { navTree = new QTreeWidget; - navTree->setObjectName("navTree"); navTree->setHeaderHidden(true); navTree->setRootIsDecorated(true); navTree->setIconSize(QSize(24, 24)); diff --git a/cockatrice/src/interface/widgets/general/home_widget.cpp b/cockatrice/src/interface/widgets/general/home_widget.cpp index e6b5bf255..5d7670c1f 100644 --- a/cockatrice/src/interface/widgets/general/home_widget.cpp +++ b/cockatrice/src/interface/widgets/general/home_widget.cpp @@ -68,7 +68,6 @@ 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); @@ -288,7 +287,6 @@ void HomeWidget::updateButtonsToBackgroundColor() QGroupBox *HomeWidget::createButtons() { QGroupBox *box = new QGroupBox(this); - box->setObjectName("homeButtonsBox"); box->setStyleSheet(R"( QGroupBox { color: white; /* Title text color */ @@ -320,34 +318,27 @@ QGroupBox *HomeWidget::createButtons() boxLayout->addSpacing(25); connectButton = new HomeStyledButton(QString(), gradientColors); - connectButton->setObjectName("homeConnectButton"); boxLayout->addWidget(connectButton); visualDeckEditorButton = new HomeStyledButton(QString(), gradientColors); - visualDeckEditorButton->setObjectName("homeCreateNewDeckButton"); connect(visualDeckEditorButton, &QPushButton::clicked, tabSupervisor, [this] { tabSupervisor->openDeckInNewTab(LoadedDeck()); }); boxLayout->addWidget(visualDeckEditorButton); visualDeckStorageButton = new HomeStyledButton(QString(), gradientColors); - visualDeckStorageButton->setObjectName("homeBrowseDecksButton"); connect(visualDeckStorageButton, &QPushButton::clicked, tabSupervisor, [this] { tabSupervisor->actTabVisualDeckStorage(true); }); boxLayout->addWidget(visualDeckStorageButton); visualDatabaseDisplayButton = new HomeStyledButton(QString(), gradientColors); - visualDatabaseDisplayButton->setObjectName("homeBrowseCardDatabaseButton"); connect(visualDatabaseDisplayButton, &QPushButton::clicked, tabSupervisor, &TabSupervisor::addVisualDatabaseDisplayTab); boxLayout->addWidget(visualDatabaseDisplayButton); edhrecButton = new HomeStyledButton(QString(), gradientColors); - edhrecButton->setObjectName("homeBrowseEdhrecButton"); connect(edhrecButton, &QPushButton::clicked, tabSupervisor, &TabSupervisor::addEdhrecMainTab); boxLayout->addWidget(edhrecButton); archidektButton = new HomeStyledButton(QString(), gradientColors); - archidektButton->setObjectName("homeBrowseArchidektButton"); connect(archidektButton, &QPushButton::clicked, tabSupervisor, &TabSupervisor::addArchidektTab); boxLayout->addWidget(archidektButton); replayButton = new HomeStyledButton(QString(), gradientColors); - replayButton->setObjectName("homeViewReplaysButton"); connect(replayButton, &QPushButton::clicked, tabSupervisor, [this] { tabSupervisor->actTabReplays(true); }); boxLayout->addWidget(replayButton); if (qobject_cast(tabSupervisor->parentWidget())) { diff --git a/cockatrice/src/interface/widgets/replay/replay_manager.cpp b/cockatrice/src/interface/widgets/replay/replay_manager.cpp index b01d7f9d6..8ba0cb653 100644 --- a/cockatrice/src/interface/widgets/replay/replay_manager.cpp +++ b/cockatrice/src/interface/widgets/replay/replay_manager.cpp @@ -57,7 +57,7 @@ ReplayManager::ReplayManager(QObject *parent, GameReplay *replay) rewindBufferingTimer = new QTimer(this); rewindBufferingTimer->setSingleShot(true); - connect(rewindBufferingTimer, &QTimer::timeout, this, [this] { processRewind(); }); + connect(rewindBufferingTimer, &QTimer::timeout, this, &ReplayManager::processRewind); } ReplayManager::~ReplayManager() @@ -65,7 +65,7 @@ ReplayManager::~ReplayManager() delete replay; } -void ReplayManager::skipToTime(int newTime, bool doRewindBuffering, bool includeEventsAtTime) +void ReplayManager::skipToTime(int newTime, bool doRewindBuffering) { // check boundary conditions if (newTime < 0) { @@ -75,17 +75,15 @@ void ReplayManager::skipToTime(int newTime, bool doRewindBuffering, bool include newTime = maxTime; } - if (!includeEventsAtTime) { - newTime -= newTime % TIMER_INTERVAL_MS; // Time should always be a multiple of the interval - } + newTime -= newTime % TIMER_INTERVAL_MS; // Time should always be a multiple of the interval const bool isBackwardsSkip = newTime < currentProcessedTime; currentVisualTime = newTime; if (isBackwardsSkip) { - handleBackwardsSkip(doRewindBuffering, includeEventsAtTime); + handleBackwardsSkip(doRewindBuffering); } else { - processNewEvents(FORWARD_SKIP, includeEventsAtTime); + processNewEvents(FORWARD_SKIP); } timeChanged(currentVisualTime); @@ -97,7 +95,7 @@ void ReplayManager::skipToTime(int newTime, bool doRewindBuffering, bool include * @param doRewindBuffering When true, if multiple backward skips are made in quick succession, only a single rewind * is processed at the end. When false, the backwards skip will always cause an immediate rewind. */ -void ReplayManager::handleBackwardsSkip(bool doRewindBuffering, bool includeEventsAtTime) +void ReplayManager::handleBackwardsSkip(bool doRewindBuffering) { if (doRewindBuffering) { // We use a one-shot timer to implement the rewind buffering. @@ -107,11 +105,11 @@ void ReplayManager::handleBackwardsSkip(bool doRewindBuffering, bool includeEven rewindBufferingTimer->start(SettingsCache::instance().userInterface().getRewindBufferingMs()); } else { // otherwise, process the rewind immediately - processRewind(includeEventsAtTime); + processRewind(); } } -void ReplayManager::processRewind(bool includeEventsAtTime) +void ReplayManager::processRewind() { // stop any queued-up rewinds rewindBufferingTimer->stop(); @@ -119,7 +117,7 @@ void ReplayManager::processRewind(bool includeEventsAtTime) // process the rewind currentEvent = 0; emit rewound(); - processNewEvents(BACKWARD_SKIP, includeEventsAtTime); + processNewEvents(BACKWARD_SKIP); } void ReplayManager::replayTimerTimeout() @@ -136,13 +134,11 @@ void ReplayManager::replayTimerTimeout() } /** @brief Processes all unprocessed events up to the current time. */ -void ReplayManager::processNewEvents(PlaybackMode playbackMode, bool includeEventsAtTime) +void ReplayManager::processNewEvents(PlaybackMode playbackMode) { currentProcessedTime = currentVisualTime; - while (currentEvent < replayTimeline.size() && - (replayTimeline[currentEvent] < currentProcessedTime || - (includeEventsAtTime && replayTimeline[currentEvent] == currentProcessedTime))) { + while (currentEvent < replayTimeline.size() && replayTimeline[currentEvent] < currentProcessedTime) { EventProcessingOptions options; // backwards skip => always skip reveal windows @@ -240,24 +236,6 @@ void ReplayManager::setTime(int time) skipToTime(time, false); } -void ReplayManager::setTimeToEvent(int eventIndex) -{ - if (eventIndex < 0 || eventIndex >= replayTimeline.size()) { - return; - } - - // Flush a pending buffered rewind before seeking, otherwise its timer fires after this seek and rebuilds the - // replay state from an outdated currentVisualTime, dropping the event we seek to. Stopping the timer instead - // of processing it is not enough: playback ticks advance currentProcessedTime without rewinding currentEvent, - // so the board could still be ahead of the target. - if (rewindBufferingTimer->isActive()) { - processRewind(); - } - - // Unlike a timeline seek, jumping to an event should apply that event immediately. - skipToTime(replayTimeline.at(eventIndex), false, true); -} - void ReplayManager::skipByAmount(int amount) { skipToTime(currentVisualTime + amount, amount < 0); diff --git a/cockatrice/src/interface/widgets/replay/replay_manager.h b/cockatrice/src/interface/widgets/replay/replay_manager.h index 34bae5f9a..3849b86ae 100644 --- a/cockatrice/src/interface/widgets/replay/replay_manager.h +++ b/cockatrice/src/interface/widgets/replay/replay_manager.h @@ -40,10 +40,10 @@ class ReplayManager : public QObject ///< to rewind buffering int currentEvent = 0; ///< current event's index - void skipToTime(int newTime, bool doRewindBuffering, bool includeEventsAtTime = false); - void handleBackwardsSkip(bool doRewindBuffering, bool includeEventsAtTime = false); - void processRewind(bool includeEventsAtTime = false); - void processNewEvents(PlaybackMode playbackMode, bool includeEventsAtTime = false); + void skipToTime(int newTime, bool doRewindBuffering); + void handleBackwardsSkip(bool doRewindBuffering); + void processRewind(); + void processNewEvents(PlaybackMode playbackMode); void handleSkipEmptySection(); private slots: @@ -73,7 +73,6 @@ public slots: void startReplay(); void stopReplay(); void setTime(int time); - void setTimeToEvent(int eventIndex); void skipByAmount(int amount); // use a negative amount to skip backwards signals: diff --git a/cockatrice/src/interface/widgets/replay/replay_widget.cpp b/cockatrice/src/interface/widgets/replay/replay_widget.cpp index 3f0bf35d9..07b4e72ce 100644 --- a/cockatrice/src/interface/widgets/replay/replay_widget.cpp +++ b/cockatrice/src/interface/widgets/replay/replay_widget.cpp @@ -28,7 +28,6 @@ ReplayWidget::ReplayWidget(QWidget *parent, GameReplay *replay) connect(replayManager, &ReplayManager::eventReplayed, this, &ReplayWidget::eventReplayed); connect(replayManager, &ReplayManager::replayFinished, this, &ReplayWidget::replayFinished); connect(replayManager, &ReplayManager::rewound, this, &ReplayWidget::rewound); - connect(replayManager, &ReplayManager::timeChanged, this, &ReplayWidget::timeChanged); // timeline widget timelineWidget = new ReplayTimelineWidget; @@ -97,16 +96,6 @@ ReplayWidget::ReplayWidget(QWidget *parent, GameReplay *replay) refreshShortcuts(); } -const QList &ReplayWidget::getReplayTimeline() const -{ - return replayManager->getReplayTimeline(); -} - -void ReplayWidget::seekToEvent(int eventIndex) -{ - replayManager->setTimeToEvent(eventIndex); -} - void ReplayWidget::replayFinished() { replayPlayButton->setChecked(false); diff --git a/cockatrice/src/interface/widgets/replay/replay_widget.h b/cockatrice/src/interface/widgets/replay/replay_widget.h index 5d23b358d..0e78442ce 100644 --- a/cockatrice/src/interface/widgets/replay/replay_widget.h +++ b/cockatrice/src/interface/widgets/replay/replay_widget.h @@ -8,7 +8,6 @@ #ifndef REPLAY_WIDGET_H #define REPLAY_WIDGET_H -#include #include #include #include @@ -36,14 +35,8 @@ public: */ ReplayWidget(QWidget *parent, GameReplay *replay); - [[nodiscard]] const QList &getReplayTimeline() const; - -public slots: - void seekToEvent(int eventIndex); - signals: void rewound(); - void timeChanged(int time); void eventReplayed(const GameEventContainer &cont, EventProcessingOptions options); private: diff --git a/cockatrice/src/interface/widgets/server/user/user_games_model.cpp b/cockatrice/src/interface/widgets/server/user/user_games_model.cpp deleted file mode 100644 index b7c728c45..000000000 --- a/cockatrice/src/interface/widgets/server/user/user_games_model.cpp +++ /dev/null @@ -1,149 +0,0 @@ -#include "user_games_model.h" - -#include -#include -#include -#include -#include -#include -#include - -UserGamesModel::UserGamesModel(AbstractClient *_client, int requestTimeoutMs, QObject *parent) - : QAbstractListModel(parent), client(_client) -{ - timeout.setSingleShot(true); - timeout.setInterval(requestTimeoutMs); - connect(&timeout, &QTimer::timeout, this, [this] { - invalidateRequest(); - setState(State::Error); - }); - connected = client->getStatus() == StatusLoggedIn; - connect(client, &AbstractClient::statusChanged, this, [this](ClientStatus status) { - // AbstractClient::setStatus emits this signal while holding the (non-recursive) - // status mutex, which getStatus() would try to lock again. Track the emitted - // value here, and read this cache from canRefresh() for the same reason. - connected = status == StatusLoggedIn; - if (!connected) { - invalidateRequest(); - replaceGames({}); - setState(online ? State::Disconnected : State::Offline); - } else if (online) { - refresh(); - } - }); - connect(client, &QObject::destroyed, this, [this] { - client = nullptr; - connected = false; - invalidateRequest(); - replaceGames({}); - setState(online ? State::Disconnected : State::Offline); - }); -} - -int UserGamesModel::rowCount(const QModelIndex &parent) const -{ - return parent.isValid() ? 0 : games.size(); -} - -QVariant UserGamesModel::data(const QModelIndex &index, int role) const -{ - if (!index.isValid() || index.model() != this || index.column() != 0 || index.row() < 0 || - index.row() >= games.size() || role != GameRole) { - return {}; - } - return QVariant::fromValue(games.at(index.row())); -} - -void UserGamesModel::setUser(const QString &name, bool isOnline) -{ - userName = name; - online = isOnline; - refresh(); -} - -UserGamesModel::State UserGamesModel::getState() const -{ - return state; -} - -bool UserGamesModel::canRefresh() const -{ - return online && !userName.isEmpty() && client && connected && state != State::Loading; -} - -void UserGamesModel::invalidateRequest() -{ - ++requestGeneration; - // The client owns pending commands. Detach our interest without deleting - // an object that may already have moved to the client's thread. - disconnect(responseConnection); - responseConnection = {}; - timeout.stop(); -} - -void UserGamesModel::replaceGames(QList replacement) -{ - beginResetModel(); - games = std::move(replacement); - endResetModel(); -} - -void UserGamesModel::setState(State newState) -{ - state = newState; - emit stateChanged(); -} - -void UserGamesModel::refresh() -{ - invalidateRequest(); - const quint64 request = requestGeneration; - replaceGames({}); - if (!online || userName.isEmpty()) { - setState(State::Offline); - return; - } - if (!client || !connected) { - setState(State::Disconnected); - return; - } - - setState(State::Loading); - // State listeners may select another user or disconnect the client. - if (request != requestGeneration) { - return; - } - - Command_GetGamesOfUser command; - command.set_user_name(userName.toStdString()); - PendingCommand *pending = client->prepareSessionCommand(command); - responseConnection = connect(pending, &PendingCommand::finished, this, - [this, request](const Response &response) { finishRequest(response, request); }); - timeout.start(); - client->sendCommand(pending); -} - -void UserGamesModel::finishRequest(const Response &response, quint64 request) -{ - if (request != requestGeneration) { - return; - } - invalidateRequest(); - const quint64 completion = requestGeneration; - if (response.response_code() != Response::RespOk || !response.HasExtension(Response_GetGamesOfUser::ext)) { - setState(State::Error); - return; - } - - const auto &result = response.GetExtension(Response_GetGamesOfUser::ext); - QList replacement; - replacement.reserve(result.game_list_size()); - for (const auto &game : result.game_list()) { - replacement.append(game); - } - replaceGames(std::move(replacement)); - if (completion != requestGeneration) { - return; - } - setState(games.isEmpty() ? State::Empty : State::Ready); -} diff --git a/cockatrice/src/interface/widgets/server/user/user_games_model.h b/cockatrice/src/interface/widgets/server/user/user_games_model.h deleted file mode 100644 index b8ad71a67..000000000 --- a/cockatrice/src/interface/widgets/server/user/user_games_model.h +++ /dev/null @@ -1,63 +0,0 @@ -#ifndef COCKATRICE_USER_GAMES_MODEL_H -#define COCKATRICE_USER_GAMES_MODEL_H - -#include -#include -#include -#include -#include -#include - -class AbstractClient; -class Response; - -/** Current user's games and the lifetime of the request that supplies them. */ -class UserGamesModel : public QAbstractListModel -{ - Q_OBJECT - -public: - enum class State - { - Offline, - Disconnected, - Loading, - Ready, - Empty, - Error - }; - static constexpr int DefaultRequestTimeoutMs = 15000; - static constexpr int GameRole = Qt::UserRole + 10; - - explicit UserGamesModel(AbstractClient *client, - int requestTimeoutMs = DefaultRequestTimeoutMs, - QObject *parent = nullptr); - int rowCount(const QModelIndex &parent = {}) const override; - QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override; - - void setUser(const QString &name, bool isOnline); - void refresh(); - State getState() const; - bool canRefresh() const; - -signals: - void stateChanged(); - -private: - void invalidateRequest(); - void replaceGames(QList replacement); - void finishRequest(const Response &response, quint64 request); - void setState(State newState); - - QPointer client; - QString userName; - bool online = false; - bool connected = false; - State state = State::Offline; - QList games; - quint64 requestGeneration = 0; - QMetaObject::Connection responseConnection; - QTimer timeout; -}; - -#endif // COCKATRICE_USER_GAMES_MODEL_H diff --git a/cockatrice/src/interface/widgets/server/user/user_info_popup.cpp b/cockatrice/src/interface/widgets/server/user/user_info_popup.cpp index 4b4887bc4..f6e6508e2 100644 --- a/cockatrice/src/interface/widgets/server/user/user_info_popup.cpp +++ b/cockatrice/src/interface/widgets/server/user/user_info_popup.cpp @@ -6,9 +6,10 @@ #include "../../interface/widgets/tabs/tab_supervisor.h" #include "libcockatrice/card/database/card_database_querier.h" #include "libcockatrice/card/printing/exact_card.h" +#include "libcockatrice/protocol/pb/response.pb.h" #include "libcockatrice/protocol/pb/serverinfo_game.pb.h" #include "libcockatrice/protocol/pb/serverinfo_user.pb.h" -#include "user_games_model.h" +#include "libcockatrice/protocol/pb/session_commands.pb.h" #include "user_level.h" #include "user_list_painter.h" @@ -33,7 +34,6 @@ #include #include #include -#include #include #include #include @@ -41,6 +41,8 @@ #include #include #include +#include +#include #include #include #include @@ -50,6 +52,8 @@ #include #include #include +#include +#include #include #include #include @@ -427,7 +431,6 @@ void UserInfoPopup::buildUi() refreshBtn = new QPushButton(QStringLiteral("↻"), this); refreshBtn->setFixedSize(20, 20); refreshBtn->setFlat(true); - refreshBtn->setToolTip(tr("Refresh games")); connect(refreshBtn, &QPushButton::clicked, this, &UserInfoPopup::refreshGames); gh->addWidget(refreshBtn); root->addLayout(gh); @@ -438,8 +441,7 @@ void UserInfoPopup::buildUi() root->addWidget(gamesStatus); // Games list - gamesModel = new UserGamesModel(client, UserGamesModel::DefaultRequestTimeoutMs, this); - connect(gamesModel, &UserGamesModel::stateChanged, this, &UserInfoPopup::updateGamesDisplay); + gamesModel = new QStandardItemModel(this); gamesView = new QListView(this); gamesView->setModel(gamesModel); gamesView->setItemDelegate(new PopupGameDelegate(gamesView)); @@ -663,21 +665,13 @@ void UserInfoPopup::rebuildActionButtons(const ServerInfo_User &userInfo, bool o void UserInfoPopup::updateActionButtons(const ServerInfo_User &userInfo, bool online, bool isBuddy, bool isIgnored) { - currentUserInfo = userInfo; - currentBuddy = isBuddy; - currentIgnored = isIgnored; - if (currentOnline != online) { - setOnline(online); - } else { - refreshHeader(); - rebuildActionButtons(userInfo, online, isBuddy, isIgnored); - adjustSize(); - } + rebuildActionButtons(userInfo, online, isBuddy, isIgnored); + adjustSize(); } void UserInfoPopup::onGamesContextMenu(const QPoint &pos) { - const QPersistentModelIndex idx = gamesView->indexAt(pos); + const QModelIndex idx = gamesView->indexAt(pos); if (!idx.isValid()) { return; } @@ -705,7 +699,7 @@ void UserInfoPopup::onGamesContextMenu(const QPoint &pos) } const QAction *chosen = menu.exec(gamesView->viewport()->mapToGlobal(pos)); - if (!chosen || !idx.isValid()) { + if (!chosen) { return; } @@ -742,8 +736,6 @@ void UserInfoPopup::showForUser(const QString &userName, currentUser = userName; currentUserInfo = userInfo; currentOnline = online; - currentBuddy = isBuddy; - currentIgnored = isIgnored; // Header refreshHeader(); @@ -751,71 +743,88 @@ void UserInfoPopup::showForUser(const QString &userName, // Actions rebuildActionButtons(userInfo, online, isBuddy, isIgnored); + // Games list reset + gamesModel->clear(); + gamesView->hide(); + gamesStatus->setText(tr("Loading games…")); + gamesStatus->show(); + // Close button, top right corner, above everything closeBtn->move(PopupWidth - closeBtn->width() - 6, 6); closeBtn->raise(); - gamesModel->setUser(currentUser, currentOnline); adjustSize(); + fetchGames(); } // ── Games fetch ─────────────────────────────────────────────────────────────── void UserInfoPopup::fetchGames() { - gamesModel->refresh(); -} - -void UserInfoPopup::setOnline(bool online) -{ - if (currentOnline == online) { + if (!client || currentUser.isEmpty()) { return; } - currentOnline = online; - refreshHeader(); - rebuildActionButtons(currentUserInfo, online, currentBuddy, currentIgnored); - gamesModel->setUser(currentUser, online); + + Command_GetGamesOfUser cmd; + cmd.set_user_name(currentUser.toStdString()); + + const QString snapshot = currentUser; + PendingCommand *pend = client->prepareSessionCommand(cmd); + connect(pend, &PendingCommand::finished, this, + [this, snapshot](const Response &r) { onGamesReceived(r, snapshot); }); + client->sendCommand(pend); } -void UserInfoPopup::updateGamesDisplay() +void UserInfoPopup::onGamesReceived(const Response &r, const QString &forUser) { - refreshBtn->setEnabled(gamesModel->canRefresh()); - const auto state = gamesModel->getState(); - const bool ready = state == UserGamesModel::State::Ready; - gamesView->setVisible(ready); - gamesStatus->setVisible(!ready); - - switch (state) { - case UserGamesModel::State::Offline: - gamesStatus->setText(tr("User is offline.")); - break; - case UserGamesModel::State::Disconnected: - gamesStatus->setText(tr("Not connected to the server.")); - break; - case UserGamesModel::State::Loading: - gamesStatus->setText(tr("Loading games…")); - break; - case UserGamesModel::State::Empty: - gamesStatus->setText(tr("No active games.")); - break; - case UserGamesModel::State::Error: - gamesStatus->setText(tr("Could not load games.")); - break; - case UserGamesModel::State::Ready: { - // Fit exactly to the number of visible rows, scroll when more than 5. - constexpr int rowH = 38; // must match PopupGameDelegate::sizeHint - constexpr int maxRows = 5; - const int count = gamesModel->rowCount(); - gamesView->setFixedHeight(qMin(count, maxRows) * rowH + 2); - gamesView->setVerticalScrollBarPolicy(count > maxRows ? Qt::ScrollBarAlwaysOn : Qt::ScrollBarAlwaysOff); - break; - } + if (forUser != currentUser) { + return; // stale response, different user showing now } + + gamesModel->clear(); + + if (r.response_code() != Response::RespOk) { + gamesStatus->setText(tr("Could not load games.")); + gamesStatus->show(); + gamesView->hide(); + return; + } + + const auto &resp = r.GetExtension(Response_GetGamesOfUser::ext); + if (resp.game_list_size() == 0) { + gamesStatus->setText(tr("No active games.")); + gamesStatus->show(); + gamesView->hide(); + return; + } + + for (int i = 0; i < resp.game_list_size(); ++i) { + auto *item = new QStandardItem; + item->setData(QVariant::fromValue(resp.game_list(i)), PopupRoles::GameData); + item->setEditable(false); + gamesModel->appendRow(item); + } + + gamesStatus->hide(); + gamesView->show(); + + // Fit exactly to the number of visible rows, scroll when more than 5 + constexpr int rowH = 38; // must match PopupGameDelegate::sizeHint + constexpr int maxRows = 5; + const int count = gamesModel->rowCount(); + const int visible = qMin(count, maxRows); + gamesView->setFixedHeight(visible * rowH + 2); + gamesView->setVerticalScrollBarPolicy(count > maxRows ? Qt::ScrollBarAlwaysOn : Qt::ScrollBarAlwaysOff); + adjustSize(); } void UserInfoPopup::refreshGames() { + gamesModel->clear(); + gamesView->hide(); + gamesStatus->setText(tr("Loading games…")); + gamesStatus->show(); fetchGames(); } diff --git a/cockatrice/src/interface/widgets/server/user/user_info_popup.h b/cockatrice/src/interface/widgets/server/user/user_info_popup.h index 1af4ad0e1..f2a91d025 100644 --- a/cockatrice/src/interface/widgets/server/user/user_info_popup.h +++ b/cockatrice/src/interface/widgets/server/user/user_info_popup.h @@ -1,7 +1,6 @@ #ifndef COCKATRICE_USER_INFO_POPUP_H #define COCKATRICE_USER_INFO_POPUP_H -#include "user_games_model.h" #include "user_list_painter.h" #include @@ -22,13 +21,15 @@ class QPushButton; class TabSupervisor; class QListView; class QPoint; +class QStandardItemModel; +class Response; template class QMap; // ── Roles ───────────────────────────────────────────────────────────────────── namespace PopupRoles { -constexpr int GameData = UserGamesModel::GameRole; +constexpr int GameData = Qt::UserRole + 10; } // Popup theme @@ -142,7 +143,6 @@ public: void showForUser(const QString &userName, const ServerInfo_User &userInfo, bool online, bool isBuddy, bool isIgnored); void fetchGames(); - void setOnline(bool online); [[nodiscard]] QString getCurrentUser() const { @@ -199,7 +199,7 @@ protected: private slots: void refreshGames(); - void updateGamesDisplay(); + void onGamesReceived(const Response &r, const QString &forUser); void onGamesContextMenu(const QPoint &pos); private: @@ -218,8 +218,6 @@ private: QString currentUser; ServerInfo_User currentUserInfo; bool currentOnline = false; - bool currentBuddy = false; - bool currentIgnored = false; std::function gameInviteAvailable; UserInfoHeaderWidget *header; @@ -227,7 +225,7 @@ private: QLabel *gamesLabel; QFrame *separator; QListView *gamesView; - UserGamesModel *gamesModel; + QStandardItemModel *gamesModel; QLabel *gamesStatus; QPushButton *closeBtn; QPushButton *refreshBtn; diff --git a/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp b/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp index 943b4d5de..34e542e02 100644 --- a/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp +++ b/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp @@ -630,14 +630,7 @@ void UserListWidget::bind(UserListManager *mgr) connect(manager, &UserListManager::addedToIgnoreList, this, refreshCurrentPopup); connect(manager, &UserListManager::removedFromIgnoreList, this, refreshIfPopupOpen); connect(manager, &UserListManager::userJoinedOnline, this, refreshCurrentPopup); - connect(manager, &UserListManager::userLeftOnline, this, [this, refreshIfPopupOpen](const QString &name) { - // The online row may already have been removed; the pinned popup must - // still invalidate its games request and display the offline state. - if (userInfoPopup && userInfoPopup->getCurrentUser() == name) { - userInfoPopup->setOnline(false); - } - refreshIfPopupOpen(name); - }); + connect(manager, &UserListManager::userLeftOnline, this, refreshIfPopupOpen); rebuild(); } diff --git a/cockatrice/src/interface/widgets/tabs/api/archidekt/tab_archidekt.cpp b/cockatrice/src/interface/widgets/tabs/api/archidekt/tab_archidekt.cpp index 9f7369435..7f7a2d3fd 100644 --- a/cockatrice/src/interface/widgets/tabs/api/archidekt/tab_archidekt.cpp +++ b/cockatrice/src/interface/widgets/tabs/api/archidekt/tab_archidekt.cpp @@ -25,7 +25,6 @@ #include #include #include -#include #include #include #include @@ -61,6 +60,8 @@ TabArchidekt::TabArchidekt(TabSupervisor *_tabSupervisor) { // Initialize network networkManager = NetworkManagerFactory::create(this); + networkManager->setRedirectPolicy(QNetworkRequest::ManualRedirectPolicy); + connect(networkManager, &QNetworkAccessManager::finished, this, &TabArchidekt::processApiJson); // Initialize debounce timer searchDebounceTimer = new QTimer(this); @@ -76,48 +77,6 @@ TabArchidekt::TabArchidekt(TabSupervisor *_tabSupervisor) getTopDecks(); } -TabArchidekt::~TabArchidekt() -{ - cancelRequest(); -} - -void TabArchidekt::cancelRequest() -{ - const QPointer previousReply = activeReply; - activeReply = nullptr; - isLoadingMore = false; - if (previousReply) { - disconnect(previousReply, nullptr, this, nullptr); - previousReply->abort(); - previousReply->deleteLater(); - } -} - -void TabArchidekt::startRequest(QUrl url, RequestKind kind, int page) -{ - cancelRequest(); - requestUrl = url; - requestKind = kind; - requestedPage = page; - isLoadingMore = true; - retryButton->hide(); - requestStatusLabel->setText(kind == RequestKind::Deck ? tr("Loading deck…") : tr("Loading decks…")); - requestStatusLabel->show(); - - QNetworkRequest request(url); - request.setHeader(QNetworkRequest::UserAgentHeader, QString("Cockatrice %1").arg(VERSION_STRING)); - auto *reply = networkManager->get(request); - activeReply = reply; - connect(reply, &QNetworkReply::finished, this, [this, reply]() { processApiJson(reply); }); -} - -void TabArchidekt::showRequestError(const QString &error) -{ - requestStatusLabel->setText(tr("Unable to load decks: %1").arg(error)); - requestStatusLabel->show(); - retryButton->show(); -} - void TabArchidekt::initializeUi() { // Main container @@ -235,18 +194,6 @@ void TabArchidekt::initializeUi() mainLayout->addWidget(primaryToolbar); mainLayout->addWidget(secondaryToolbar); - - auto *requestStatusRow = new QHBoxLayout; - requestStatusLabel = new QLabel(container); - requestStatusLabel->setTextFormat(Qt::PlainText); - requestStatusLabel->setWordWrap(true); - retryButton = new QPushButton(tr("Retry"), container); - retryButton->hide(); - connect(retryButton, &QPushButton::clicked, this, - [this]() { startRequest(requestUrl, requestKind, requestedPage); }); - requestStatusRow->addWidget(requestStatusLabel, 1); - requestStatusRow->addWidget(retryButton); - mainLayout->addLayout(requestStatusRow); mainLayout->addWidget(scrollArea); setCentralWidget(container); @@ -457,7 +404,6 @@ void TabArchidekt::retranslateUi() minDeckSizeLogicCombo->setItemText(0, tr("Exact")); searchButton->setText(tr("Search")); - retryButton->setText(tr("Retry")); settingsButton->setToolTip(tr("Display Settings")); } @@ -571,95 +517,82 @@ QString TabArchidekt::buildSearchUrl() void TabArchidekt::doSearch() { + // Reset to first page on new search + currentPage = 1; + // We're searching, so we'll be in list mode + isListMode = true; // Don't debounce - only called by explicit user actions now doSearchImmediate(); } void TabArchidekt::doSearchImmediate() { - searchDebounceTimer->stop(); - currentPage = 1; - startRequest(QUrl(buildSearchUrl()), RequestKind::Search); + QString url = buildSearchUrl(); + QNetworkRequest req{QUrl(url)}; + req.setHeader(QNetworkRequest::UserAgentHeader, QString("Cockatrice %1").arg(VERSION_STRING)); + networkManager->get(req); } void TabArchidekt::loadNextPage() { - if (activeReply || !isListMode || !listingsWidget || nextPageUrl.isEmpty()) { + if (isLoadingMore) { return; } - // Continue the accepted search, even if the user has edited filters without submitting them. - // Advance currentPage only after this response has been accepted, so failures can retry it. - startRequest(nextPageUrl, RequestKind::NextPage, currentPage + 1); + isLoadingMore = true; + currentPage++; + + QString url = buildSearchUrl(); + QNetworkRequest req{QUrl(url)}; + req.setHeader(QNetworkRequest::UserAgentHeader, QString("Cockatrice %1").arg(VERSION_STRING)); + networkManager->get(req); } void TabArchidekt::actNavigatePage(QString url) { - searchDebounceTimer->stop(); - startRequest(QUrl(url), RequestKind::Deck); + QNetworkRequest request{QUrl(url)}; + request.setHeader(QNetworkRequest::UserAgentHeader, QString("Cockatrice %1").arg(VERSION_STRING)); + networkManager->get(request); } void TabArchidekt::getTopDecks() { - doSearchImmediate(); + currentPage = 1; + QNetworkRequest request{QUrl(buildSearchUrl())}; + request.setHeader(QNetworkRequest::UserAgentHeader, QString("Cockatrice %1").arg(VERSION_STRING)); + networkManager->get(request); } void TabArchidekt::processApiJson(QNetworkReply *reply) { - if (reply != activeReply) { - reply->deleteLater(); - return; - } - activeReply = nullptr; - isLoadingMore = false; - reply->deleteLater(); - if (reply->error() != QNetworkReply::NoError) { - showRequestError(reply->errorString()); + isLoadingMore = false; + reply->deleteLater(); return; } QByteArray responseData = reply->readAll(); - QJsonParseError parseError; - QJsonDocument jsonDoc = QJsonDocument::fromJson(responseData, &parseError); + QJsonDocument jsonDoc = QJsonDocument::fromJson(responseData); - if (parseError.error != QJsonParseError::NoError || !jsonDoc.isObject()) { - showRequestError(tr("Invalid response from Archidekt.")); + if (!jsonDoc.isObject()) { + isLoadingMore = false; + reply->deleteLater(); return; } QJsonObject jsonObj = jsonDoc.object(); - if (requestKind != RequestKind::Deck) { - if (!jsonObj.value("results").isArray() || - (!jsonObj.value("next").isNull() && !jsonObj.value("next").isString())) { - showRequestError(tr("Invalid deck listing from Archidekt.")); - return; - } - for (const auto &value : jsonObj.value("results").toArray()) { - if (!value.isObject() || value.toObject().value("id").toInt() <= 0) { - showRequestError(tr("Invalid deck listing from Archidekt.")); - return; - } - } - currentPage = requestedPage; - const QString next = jsonObj.value("next").toString(); - nextPageUrl = next.isEmpty() ? QUrl() : reply->url().resolved(QUrl(next)); - // An empty or self-repeating page cannot make forward progress. - if (jsonObj.value("results").toArray().isEmpty() || nextPageUrl == reply->url()) { - nextPageUrl.clear(); - } - requestStatusLabel->setText( - currentPage == 1 && jsonObj.value("results").toArray().isEmpty() ? tr("No decks found.") : QString()); - requestStatusLabel->setVisible(!requestStatusLabel->text().isEmpty()); + QString responseUrl = reply->url().toString(); + + if (responseUrl.startsWith("https://archidekt.com/api/decks/v3/")) { processTopDecksResponse(jsonObj); - } else { - if (jsonObj.value("id").toInt() <= 0 || !jsonObj.value("cards").isArray()) { - showRequestError(tr("Invalid deck from Archidekt.")); - return; - } - requestStatusLabel->hide(); + } else if (responseUrl.startsWith("https://archidekt.com/api/decks/")) { processDeckResponse(jsonObj); + } else { + prettyPrintJson(jsonObj, 4); } + + isLoadingMore = false; + reply->deleteLater(); } void TabArchidekt::processTopDecksResponse(QJsonObject reply) @@ -667,12 +600,8 @@ void TabArchidekt::processTopDecksResponse(QJsonObject reply) ArchidektDeckListingApiResponse deckData; deckData.fromJson(reply); - // An accepted listing restores list mode; a failed deck navigation leaves the listing usable. - isListMode = true; - // New search → clear everything - if (requestKind == RequestKind::Search) { - listingsWidget = nullptr; + if (currentPage == 1) { QtUtils::clearLayoutRec(resultsLayout, QtUtils::DeletePolicy::Immediate); listingsWidget = new ArchidektApiResponseDeckListingsDisplayWidget(resultsContainer, deckData, cardSizeSlider); @@ -697,10 +626,8 @@ void TabArchidekt::processDeckResponse(QJsonObject reply) // We're in single deck mode - disable infinite scroll isListMode = false; - nextPageUrl.clear(); // Clear existing results for single deck view - listingsWidget = nullptr; QtUtils::clearLayoutRec(resultsLayout, QtUtils::DeletePolicy::Immediate); auto display = new ArchidektApiResponseDeckDisplayWidget(resultsContainer, deckData, cardSizeSlider); diff --git a/cockatrice/src/interface/widgets/tabs/api/archidekt/tab_archidekt.h b/cockatrice/src/interface/widgets/tabs/api/archidekt/tab_archidekt.h index 81c9502df..c74380b63 100644 --- a/cockatrice/src/interface/widgets/tabs/api/archidekt/tab_archidekt.h +++ b/cockatrice/src/interface/widgets/tabs/api/archidekt/tab_archidekt.h @@ -8,10 +8,8 @@ #include #include #include -#include #include #include -#include #include #include @@ -57,7 +55,6 @@ public: * connects signals and slots, and triggers an initial fetch of top decks. */ explicit TabArchidekt(TabSupervisor *_tabSupervisor); - ~TabArchidekt() override; /** * @brief Update all UI text to reflect the current language or translation @@ -193,17 +190,6 @@ protected: bool eventFilter(QObject *obj, QEvent *event) override; private: - enum class RequestKind - { - Search, - NextPage, - Deck - }; - - void startRequest(QUrl url, RequestKind kind, int page = 1); - void cancelRequest(); - void showRequestError(const QString &error); - /** * @brief Initialize the main UI layout and toolbars * @@ -250,14 +236,7 @@ private: int currentPage; ///< Current page number for infinite scroll bool isLoadingMore; ///< Flag to prevent multiple simultaneous page loads bool isListMode; - QPointer activeReply; - RequestKind requestKind = RequestKind::Search; - QUrl requestUrl; - QUrl nextPageUrl; - int requestedPage = 1; - QPointer listingsWidget; - QLabel *requestStatusLabel; - QPushButton *retryButton; + ArchidektApiResponseDeckListingsDisplayWidget *listingsWidget = nullptr; // --------------------------------------------------------------------- // Layout Containers diff --git a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_service.cpp b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_service.cpp index 7b60ed23c..8e0448faa 100644 --- a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_service.cpp +++ b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_service.cpp @@ -26,11 +26,6 @@ quint64 CommanderBracketService::estimateBracket(const DeckList &deck, QObject * return CommanderSpellbookApiAccessor::instance().estimateBracket(deck, requester); } -void CommanderBracketService::cancelEstimate(quint64 requestId) -{ - CommanderSpellbookApiAccessor::instance().cancelEstimate(requestId); -} - void CommanderBracketService::onEstimateBracketFinished(CommanderSpellbookApiAccessor::RequestId id, QObject *requester, const EstimateBracketResult &result) diff --git a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_service.h b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_service.h index 00debce86..22962cc95 100644 --- a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_service.h +++ b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_service.h @@ -30,7 +30,6 @@ public: static CommanderBracketService &instance(); quint64 estimateBracket(const DeckList &deck, QObject *requester); - void cancelEstimate(quint64 requestId); signals: void estimateFinished(quint64 requestId, QObject *requester, const CommanderBracketEstimate &estimate); diff --git a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_widget.cpp b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_widget.cpp index 9adda008e..5279165af 100644 --- a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_widget.cpp +++ b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_widget.cpp @@ -28,18 +28,9 @@ template class QSharedPointer; CommanderBracketWidget::CommanderBracketWidget(QWidget *parent) : QWidget(parent) { - autoEstimateTimer = new QTimer(this); - autoEstimateTimer->setSingleShot(true); - connect(autoEstimateTimer, &QTimer::timeout, this, [this]() { - if (SettingsCache::instance().deckEditor().getCommanderSpellbookIntegrationEnabled() == - commanderSpellbookIntegrationEnabledIndexAutomatic) { - requestBracketEstimate(); - } - }); bracketLabel = new QLabel(tr("Bracket:"), this); bracketValueLabel = new QLabel(this); - bracketValueLabel->setTextFormat(Qt::PlainText); bracketValueLabel->setText("-"); bracketValueLabel->setObjectName("bracketValueLabel"); @@ -74,38 +65,18 @@ CommanderBracketWidget::CommanderBracketWidget(QWidget *parent) : QWidget(parent this, &CommanderBracketWidget::maybeAutoEstimateBracket); connect(&SettingsCache::instance().deckEditor(), &DeckEditorSettings::commanderSpellbookIntegrationUseOfficialBracketNamesChanged, this, - &CommanderBracketWidget::displayEstimate); + &CommanderBracketWidget::maybeAutoEstimateBracket); setVisible(false); } -CommanderBracketWidget::~CommanderBracketWidget() -{ - cancelEstimate(); -} - -void CommanderBracketWidget::cancelEstimate() -{ - autoEstimateTimer->stop(); - const quint64 canceledRequest = requestId; - requestId = 0; - CommanderBracketService::instance().cancelEstimate(canceledRequest); - bracketRefreshButton->setEnabled(true); - if (canceledRequest) { - bracketValueLabel->setText("-"); - bracketValueLabel->setToolTip({}); - } -} - void CommanderBracketWidget::setDeck(const QSharedPointer &_deck) { - cancelEstimate(); deck = _deck; - hasEstimate = false; + requestId = 0; // invalidate any in-flight estimate for the previous deck // Reset the displayed bracket bracketValueLabel->setText("-"); - bracketValueLabel->setToolTip({}); bracketInfoButton->setToolTip({}); bracketInfoButton->setEnabled(false); bracketRefreshButton->setEnabled(true); @@ -173,7 +144,7 @@ bool CommanderBracketWidget::promptCommanderSpellbookIntegration() dialog.exec(); // Persist naming choice (if not disabled) - if (clickedButton == enableBtn || clickedButton == automaticBtn) { + if (clickedButton != disableBtn) { bool useOfficial = namingCombo->currentIndex() == 1; SettingsCache::instance().deckEditor().setCommanderSpellbookIntegrationUseOfficialBracketNames(useOfficial); } @@ -205,20 +176,9 @@ void CommanderBracketWidget::updateBracketVisibility(bool visible) void CommanderBracketWidget::requestBracketEstimate() { - const int mode = SettingsCache::instance().deckEditor().getCommanderSpellbookIntegrationEnabled(); - if (!deck || deck->getGameFormat().compare("commander", Qt::CaseInsensitive) != 0 || requestId || - (mode != commanderSpellbookIntegrationEnabledIndexEnabled && - mode != commanderSpellbookIntegrationEnabledIndexAutomatic)) { - return; - } - - autoEstimateTimer->stop(); - hasEstimate = false; bracketRefreshButton->setEnabled(false); bracketInfoButton->setEnabled(false); - bracketInfoButton->setToolTip({}); bracketValueLabel->setText(tr("Calculating…")); - bracketValueLabel->setToolTip({}); requestId = CommanderBracketService::instance().estimateBracket(*deck, this); } @@ -231,35 +191,22 @@ void CommanderBracketWidget::onEstimateBracketFinished(quint64 id, return; } - requestId = 0; - hasEstimate = true; - lastEstimate = result; - displayEstimate(); - bracketRefreshButton->setEnabled(true); -} - -void CommanderBracketWidget::displayEstimate() -{ - if (!hasEstimate) { - bracketInfoButton->setToolTip(tr("Why this bracket?")); - return; - } - BracketExplainer explainer; - lastBracketExplanation = explainer.explain(lastEstimate.rawResult); + lastBracketExplanation = explainer.explain(result.rawResult); // Display bracket bracketValueLabel->setText( SettingsCache::instance().deckEditor().getCommanderSpellbookIntegrationUseOfficialBracketNames() - ? lastEstimate.officialName - : lastEstimate.displayName); + ? result.officialName + : result.displayName); + bracketRefreshButton->setEnabled(true); // Build tooltip QString tooltip; for (const auto §ion : lastBracketExplanation.sections) { - tooltip += "" + section.title.toHtmlEscaped() + "
    "; + tooltip += "" + section.title + "
    "; for (const auto &line : section.bulletPoints) { - tooltip += "• " + line.toHtmlEscaped() + "
    "; + tooltip += "• " + line + "
    "; } tooltip += "
    "; } @@ -268,17 +215,13 @@ void CommanderBracketWidget::displayEstimate() bracketInfoButton->setEnabled(!tooltip.isEmpty()); } -void CommanderBracketWidget::onEstimateBracketError(quint64 id, QObject *requester, const QString &error) +void CommanderBracketWidget::onEstimateBracketError(quint64 id, QObject *requester, const QString & /*error*/) { if (requester != this || id != requestId) { return; } - requestId = 0; - hasEstimate = false; bracketValueLabel->setText("-"); - bracketValueLabel->setToolTip( - QStringLiteral("%1").arg(tr("Unable to estimate bracket: %1").arg(error).toHtmlEscaped())); bracketRefreshButton->setEnabled(true); bracketInfoButton->setToolTip({}); bracketInfoButton->setEnabled(false); @@ -286,15 +229,6 @@ void CommanderBracketWidget::onEstimateBracketError(quint64 id, QObject *request void CommanderBracketWidget::maybeAutoEstimateBracket() { - if (prompting) { - return; - } - autoEstimateTimer->stop(); - if (!deck) { - cancelEstimate(); - updateBracketVisibility(false); - return; - } const QString formatKey = deck->getGameFormat(); const bool isCommander = (formatKey.compare("commander", Qt::CaseInsensitive) == 0); @@ -302,20 +236,17 @@ void CommanderBracketWidget::maybeAutoEstimateBracket() int mode = SettingsCache::instance().deckEditor().getCommanderSpellbookIntegrationEnabled(); if (!isCommander || mode == commanderSpellbookIntegrationEnabledIndexDisabled) { - cancelEstimate(); updateBracketVisibility(false); return; } if (mode == commanderSpellbookIntegrationEnabledIndexUnprompted) { - const auto promptedDeck = deck; + if (prompting) { + return; + } prompting = true; const bool accepted = promptCommanderSpellbookIntegration(); prompting = false; - if (deck != promptedDeck || !deck || deck->getGameFormat().compare("commander", Qt::CaseInsensitive) != 0) { - maybeAutoEstimateBracket(); - return; - } if (!accepted) { updateBracketVisibility(false); return; @@ -330,17 +261,17 @@ void CommanderBracketWidget::maybeAutoEstimateBracket() } // Avoid firing if we already have a result or a request in flight - if (requestId || hasEstimate) { + if (!bracketRefreshButton->isEnabled()) { return; } // Defer to avoid races during init / model rebuild - autoEstimateTimer->start(0); + QTimer::singleShot(0, this, &CommanderBracketWidget::requestBracketEstimate); } void CommanderBracketWidget::retranslateUi() { bracketLabel->setText(tr("Bracket:")); - displayEstimate(); + bracketInfoButton->setToolTip(tr("Why this bracket?")); bracketRefreshButton->setToolTip(tr("Recalculate bracket")); } diff --git a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_widget.h b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_widget.h index 0ef1eedee..f5b2b7958 100644 --- a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_widget.h +++ b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_widget.h @@ -1,7 +1,6 @@ #ifndef COCKATRICE_COMMANDER_BRACKET_WIDGET_H #define COCKATRICE_COMMANDER_BRACKET_WIDGET_H -#include "commander_bracket_service.h" #include "commander_spellbook_bracket_explainer.h" #include @@ -12,7 +11,6 @@ class QLabel; class QToolButton; -class QTimer; struct CommanderBracketEstimate; class DeckList; class QObject; @@ -24,7 +22,6 @@ class CommanderBracketWidget : public QWidget public: explicit CommanderBracketWidget(QWidget *parent = nullptr); - ~CommanderBracketWidget() override; void setDeck(const QSharedPointer &_deck); void retranslateUi(); @@ -38,15 +35,10 @@ private slots: private: bool promptCommanderSpellbookIntegration(); void updateBracketVisibility(bool visible); - void cancelEstimate(); - void displayEstimate(); QSharedPointer deck; bool prompting = false; quint64 requestId = 0; - QTimer *autoEstimateTimer; - bool hasEstimate = false; - CommanderBracketEstimate lastEstimate; QLabel *bracketLabel; QLabel *bracketValueLabel; diff --git a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_spellbook_api_accessor.cpp b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_spellbook_api_accessor.cpp index 8456e2e21..bd8d9379d 100644 --- a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_spellbook_api_accessor.cpp +++ b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_spellbook_api_accessor.cpp @@ -9,10 +9,9 @@ #include #include #include -#include #include #include -#include +#include #include class DeckList; @@ -25,18 +24,13 @@ CommanderSpellbookApiAccessor &CommanderSpellbookApiAccessor::instance() return instance; } -CommanderSpellbookApiAccessor::CommanderSpellbookApiAccessor(QObject *parent) - : QObject(parent), network(NetworkManagerFactory::create(this)) +CommanderSpellbookApiAccessor::CommanderSpellbookApiAccessor(QObject *parent) : QObject(parent) { } CommanderSpellbookApiAccessor::RequestId CommanderSpellbookApiAccessor::estimateBracket(const DeckList &deck, QObject *requester) { - if (!requester) { - return 0; - } - CommanderSpellbookDeckRequest deckRequest = CommanderSpellbookDeckRequest::fromDeckList(deck); QJsonDocument doc(deckRequest.toJson()); @@ -46,39 +40,30 @@ CommanderSpellbookApiAccessor::RequestId CommanderSpellbookApiAccessor::estimate req.setHeader(QNetworkRequest::ContentTypeHeader, "application/json"); req.setHeader(QNetworkRequest::UserAgentHeader, QString("Cockatrice %1").arg(VERSION_STRING)); - QNetworkReply *reply = network->post(req, body); + QNetworkReply *reply = network.post(req, body); const RequestId id = nextRequestId++; - pendingRequests.insert(id, reply); - const QPointer guardedRequester(requester); - connect(requester, &QObject::destroyed, reply, [this, id]() { cancelEstimate(id); }); - connect(reply, &QNetworkReply::finished, this, [this, reply, id, guardedRequester]() { - if (pendingRequests.take(id) != reply) { - return; - } - reply->deleteLater(); - if (guardedRequester) { - onEstimateReplyFinished(reply, id, guardedRequester); - } - }); + reply->setProperty("requestId", QVariant::fromValue(id)); + reply->setProperty("requester", QVariant::fromValue(requester)); + + connect(reply, &QNetworkReply::finished, this, [this, reply]() { onEstimateReplyFinished(reply); }); return id; } -void CommanderSpellbookApiAccessor::cancelEstimate(RequestId id) +void CommanderSpellbookApiAccessor::onEstimateReplyFinished(QNetworkReply *reply) { - const QPointer reply = pendingRequests.take(id); - if (!reply) { + reply->deleteLater(); + + const RequestId id = reply->property("requestId").toULongLong(); + QObject *requester = reply->property("requester").value(); + + if (!requester) { + // Requester died — silently drop return; } - disconnect(reply, nullptr, this, nullptr); - reply->abort(); - reply->deleteLater(); -} -void CommanderSpellbookApiAccessor::onEstimateReplyFinished(QNetworkReply *reply, RequestId id, QObject *requester) -{ if (reply->error() != QNetworkReply::NoError) { emit estimateBracketError(id, requester, reply->errorString()); return; @@ -93,10 +78,6 @@ void CommanderSpellbookApiAccessor::onEstimateReplyFinished(QNetworkReply *reply } EstimateBracketResult result = EstimateBracketResult::fromJson(doc.object()); - if (result.bracketTag.trimmed().isEmpty()) { - emit estimateBracketError(id, requester, QStringLiteral("Missing bracket in response")); - return; - } emit estimateBracketFinished(id, requester, result); } diff --git a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_spellbook_api_accessor.h b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_spellbook_api_accessor.h index a1b2936bb..98e7e3710 100644 --- a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_spellbook_api_accessor.h +++ b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_spellbook_api_accessor.h @@ -1,16 +1,14 @@ #ifndef COCKATRICE_COMMANDER_SPELLBOOK_API_ACCESSOR_H #define COCKATRICE_COMMANDER_SPELLBOOK_API_ACCESSOR_H -#include +#include #include -#include #include #include #include class DeckList; class QNetworkReply; -class QNetworkAccessManager; struct EstimateBracketResult; class CommanderSpellbookApiAccessor final : public QObject @@ -23,7 +21,6 @@ public: using RequestId = quint64; RequestId estimateBracket(const DeckList &deck, QObject *requester); - void cancelEstimate(RequestId id); signals: void estimateBracketFinished(RequestId id, QObject *requester, const EstimateBracketResult &result); @@ -34,10 +31,9 @@ private: explicit CommanderSpellbookApiAccessor(QObject *parent = nullptr); Q_DISABLE_COPY_MOVE(CommanderSpellbookApiAccessor) - void onEstimateReplyFinished(QNetworkReply *reply, RequestId id, QObject *requester); + void onEstimateReplyFinished(QNetworkReply *reply); - QNetworkAccessManager *network; - QHash> pendingRequests; + QNetworkAccessManager network; RequestId nextRequestId = 1; }; diff --git a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/handle_commander_brackets.cpp b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/handle_commander_brackets.cpp index be490ce37..407f53e99 100644 --- a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/handle_commander_brackets.cpp +++ b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/handle_commander_brackets.cpp @@ -3,14 +3,11 @@ #include "../../../../../client/settings/cache_settings.h" #include -#include #include -#include #include #include #include #include -#include #include #include #include @@ -25,14 +22,6 @@ HandleCommanderBrackets::HandleCommanderBrackets(QObject *parent) { } -HandleCommanderBrackets::~HandleCommanderBrackets() -{ - if (reply) { - disconnect(reply, nullptr, this, nullptr); - reply->abort(); - } -} - void HandleCommanderBrackets::downloadBracketDefinitions() { if (reply) { @@ -46,49 +35,31 @@ void HandleCommanderBrackets::downloadBracketDefinitions() void HandleCommanderBrackets::actFinishParsingDownloadedData() { - auto *finishedReply = qobject_cast(sender()); - if (!finishedReply || finishedReply != reply) { - return; - } + reply = qobject_cast(sender()); - // Clear the active request before notifying listeners, which may immediately retry. - reply = nullptr; - finishedReply->deleteLater(); + if (reply->error() != QNetworkReply::NoError) { + emit sigBracketDefinitionsDownloadFailed(reply->error()); - if (finishedReply->error() != QNetworkReply::NoError) { - emit sigBracketDefinitionsDownloadFailed(finishedReply->error()); + reply->deleteLater(); return; } QJsonParseError parseError; - auto document = QJsonDocument::fromJson(finishedReply->readAll(), &parseError); + auto document = QJsonDocument::fromJson(reply->readAll(), &parseError); - if (parseError.error != QJsonParseError::NoError || !document.isObject() || - !document.object().value("brackets").isArray() || document.object().value("brackets").toArray().isEmpty()) { + if (parseError.error != QJsonParseError::NoError) { emit sigBracketDefinitionsDownloadFailed(QNetworkReply::UnknownContentError); - return; - } - // Validate the complete replacement before touching the last usable definitions. - // Tags are QSettings group names, so path separators must not create nested groups. - QSet tags; - for (const auto &value : document.object().value("brackets").toArray()) { - const auto definition = value.toObject(); - const auto tag = definition.value("tag").toString(); - if (!value.isObject() || tag.trimmed().isEmpty() || tag.contains('/') || tag.contains('\\') || - tags.contains(tag) || definition.value("officialName").toString().trimmed().isEmpty() || - definition.value("displayName").toString().trimmed().isEmpty() || - !definition.value("explanation").isString()) { - emit sigBracketDefinitionsDownloadFailed(QNetworkReply::UnknownContentError); - return; - } - tags.insert(tag); + reply->deleteLater(); + return; } updateBracketDefinitions(document.toVariant().toMap()); emit sigBracketDefinitionsDownloaded(); + + reply->deleteLater(); } void HandleCommanderBrackets::updateBracketDefinitions(const QVariantMap &jsonMap) diff --git a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/handle_commander_brackets.h b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/handle_commander_brackets.h index 99b15832c..39bddba05 100644 --- a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/handle_commander_brackets.h +++ b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/handle_commander_brackets.h @@ -3,7 +3,6 @@ #include #include -#include #include #include @@ -15,7 +14,6 @@ class HandleCommanderBrackets : public QObject public: explicit HandleCommanderBrackets(QObject *parent = nullptr); - ~HandleCommanderBrackets() override; void downloadBracketDefinitions(); @@ -30,7 +28,7 @@ private: void updateBracketDefinitions(const QVariantMap &jsonMap); QNetworkAccessManager *nam; - QPointer reply; + QNetworkReply *reply; }; #endif // COCKATRICE_HANDLE_COMMANDER_BRACKETS_H diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_editor.cpp b/cockatrice/src/interface/widgets/tabs/tab_deck_editor.cpp index e88b2aad6..7ca179cea 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_editor.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_editor.cpp @@ -131,16 +131,10 @@ void TabDeckEditor::loadLayout() LayoutsSettings &layouts = SettingsCache::instance().layouts(); auto layoutState = layouts.getDeckEditorLayoutState(); - // Start with every current dock placed. Saved layouts from older versions - // cannot describe docks introduced later; restoreState leaves those docks - // in their existing positions while restoring the saved docks, including - // their visibility and floating state. - restartLayout(); - if (!layoutState.isEmpty()) { - if (!restoreState(layoutState)) { - // A corrupt or incompatible state must not leave a partial layout. - restartLayout(); - } + if (layoutState.isNull()) { + restartLayout(); + } else { + restoreState(layoutState); restoreGeometry(layouts.getDeckEditorGeometry()); } } diff --git a/cockatrice/src/interface/widgets/tabs/tab_game.cpp b/cockatrice/src/interface/widgets/tabs/tab_game.cpp index cbff850a4..9e724727f 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_game.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_game.cpp @@ -99,7 +99,6 @@ class Message; TabGame::TabGame(TabSupervisor *_tabSupervisor, GameReplay *_replay) : Tab(_tabSupervisor), sayLabel(nullptr), sayEdit(nullptr) { - setObjectName("TabGame"); // THIS CTOR IS USED ON REPLAY game = new Replay(this, _replay, tabSupervisor->getIsLocalGame(), &SettingsCache::instance().userInterface(), new SettingsCardPreferenceProvider(this)); @@ -117,7 +116,6 @@ TabGame::TabGame(TabSupervisor *_tabSupervisor, GameReplay *_replay) addDockWidget(Qt::BottomDockWidgetArea, replayDock); mainWidget = new QStackedWidget(this); - mainWidget->setObjectName("mainWidget"); mainWidget->addWidget(deckViewContainerWidget); mainWidget->addWidget(gamePlayAreaWidget); setCentralWidget(mainWidget); @@ -146,7 +144,6 @@ TabGame::TabGame(TabSupervisor *_tabSupervisor, const QMap &_roomGameTypes) : Tab(_tabSupervisor), userListProxy(_tabSupervisor->getUserListManager()) { - setObjectName("TabGame"); // THIS CTOR IS USED ON GAMES game = new Game(this, tabSupervisor->getIsLocalGame(), _clients, event, _roomGameTypes, &SettingsCache::instance().userInterface(), new SettingsCardPreferenceProvider(this)); @@ -163,7 +160,6 @@ TabGame::TabGame(TabSupervisor *_tabSupervisor, addDockWidget(Qt::RightDockWidgetArea, messageLayoutDock); mainWidget = new QStackedWidget(this); - mainWidget->setObjectName("mainWidget"); mainWidget->addWidget(deckViewContainerWidget); mainWidget->addWidget(gamePlayAreaWidget); mainWidget->setContentsMargins(0, 0, 0, 0); @@ -1122,7 +1118,6 @@ void TabGame::createMenuItems() connect(aFocusChat, &QAction::triggered, sayEdit, qOverload<>(&LineEditCompleter::setFocus)); phasesMenu = new TearOffMenu(this); - phasesMenu->setObjectName("phasesMenu"); for (int i = 0; i < phasesToolbar->phaseCount(); ++i) { auto *temp = new QAction(QString(), this); @@ -1133,16 +1128,12 @@ void TabGame::createMenuItems() phasesMenu->addSeparator(); phasesMenu->addAction(aNextPhase); - aNextPhase->setObjectName("aNextPhase"); phasesMenu->addAction(aNextPhaseAction); - aNextPhaseAction->setObjectName("aNextPhaseAction"); gameMenu = new QMenu(this); - gameMenu->setObjectName("gameMenu"); playersSeparator = gameMenu->addSeparator(); gameMenu->addMenu(phasesMenu); gameMenu->addAction(aNextTurn); - aNextTurn->setObjectName("aNextTurn"); gameMenu->addAction(aReverseTurn); gameMenu->addSeparator(); gameMenu->addAction(aRemoveLocalArrows); diff --git a/cockatrice/src/interface/widgets/tabs/tab_home.cpp b/cockatrice/src/interface/widgets/tabs/tab_home.cpp index 3ad16aa91..af9994fdd 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_home.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_home.cpp @@ -6,7 +6,6 @@ class TabSupervisor; TabHome::TabHome(TabSupervisor *_tabSupervisor, AbstractClient *_client) : Tab(_tabSupervisor), client(_client) { - setObjectName("TabHome"); homeWidget = new HomeWidget(this, tabSupervisor); setCentralWidget(homeWidget); } diff --git a/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp b/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp index cf71dd7ed..3316232a4 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp @@ -54,7 +54,6 @@ #include #include #include -#include #include #include #include @@ -158,15 +157,6 @@ TabSupervisor::TabSupervisor(AbstractClient *_client, QMenu *tabsMenu, QWidget * tabReplays(nullptr), tabAdmin(nullptr), tabCardArtRules(nullptr), tabLog(nullptr), tabReport(nullptr), tabModeration(nullptr), tabDeveloper(nullptr), isLocalGame(false) { - setObjectName("tabSupervisor"); - - // QTabWidget keeps tab pages in an internal stacked widget. Give it a stable - // name so the GUI tests can address the pages by a readable path - // (tabSupervisor/stackedWidget/). - if (auto *stackedWidget = findChild()) { - stackedWidget->setObjectName("stackedWidget"); - } - setElideMode(Qt::ElideRight); setMovable(true); setIconSize(QSize(15, 15)); @@ -472,9 +462,6 @@ void TabSupervisor::addCloseButtonToTab(Tab *tab, int tabIndex, QAction *manager auto closeSide = static_cast( tabBar()->style()->styleHint(QStyle::SH_TabBar_CloseButtonPosition, nullptr, tabBar())); auto *closeButton = new CloseButton(tab); - // No object name: the tab bar index shifts when tabs move or close, so an - // index-derived name would go stale (or collide). Callers find the button - // through the tab widget instead. if (manager) { // If managed, all close requests should go through the menu action connect(closeButton, &CloseButton::clicked, this, [manager] { checkAndTrigger(manager, false); }); diff --git a/cockatrice/src/interface/window_main.cpp b/cockatrice/src/interface/window_main.cpp index 79a802234..d546676bc 100644 --- a/cockatrice/src/interface/window_main.cpp +++ b/cockatrice/src/interface/window_main.cpp @@ -55,14 +55,6 @@ #include "widgets/server/handle_public_servers.h" #include "widgets/settings_page/abstract_settings_page.h" #include "widgets/tabs/api/commander_spellbook/handle_commander_brackets.h" -#ifdef WITH_GUI_TEST -#include "../game_graphics/pq_cockatrice_event_player.h" -#include "pqEventObserver.h" -#include "pqEventSource.h" -#include "pqEventTypes.h" -#include "pqTestUtility.h" -#include "recording.h" -#endif #include #include @@ -318,7 +310,6 @@ void MainWindow::actAbout() mb.setIconPixmap( themePixmap(QStringLiteral("cockatrice")).scaled(64, 64, Qt::KeepAspectRatio, Qt::SmoothTransformation)); mb.setTextInteractionFlags(Qt::TextBrowserInteraction); - mb.setObjectName("aboutDialog"); mb.exec(); } @@ -357,24 +348,6 @@ void MainWindow::actOpenSettingsFolder() QtUtils::openPathInFileManager(dir); } -#ifdef WITH_GUI_TEST -void MainWindow::actRecord() -{ - QString filename = QFileDialog::getSaveFileName(this, tr("Test File Name"), QString(), tr("XML Files (*.xml)")); - if (!filename.isEmpty()) { - testUtility->recordTests(filename); - } -} - -void MainWindow::actPlayRecording() -{ - QString filename = QFileDialog::getOpenFileName(this, tr("Test File Name"), QString(), tr("XML Files (*.xml)")); - if (!filename.isEmpty()) { - testUtility->playTests(filename); - } -} -#endif - void MainWindow::retranslateUi() { connectionController->refreshWindowTitle(); @@ -419,10 +392,6 @@ void MainWindow::retranslateUi() aViewLog->setText(tr("View &Debug Log")); aOpenSettingsFolder->setText(tr("Open Settings Folder")); aFirstRunWizard->setText(tr("Re-run Onboarding Wizard...")); -#ifdef WITH_GUI_TEST - aRecord->setText(tr("&Record Test...")); - aPlayRecording->setText(tr("&Play Test...")); -#endif aShow->setText(tr("Show/Hide")); @@ -432,33 +401,24 @@ void MainWindow::retranslateUi() void MainWindow::createActions() { aConnect = new QAction(this); - aConnect->setObjectName("actionConnect"); connect(aConnect, &QAction::triggered, this, &MainWindow::actConnect); aDisconnect = new QAction(this); - aDisconnect->setObjectName("actionDisconnect"); aDisconnect->setEnabled(false); connect(aDisconnect, &QAction::triggered, this, &MainWindow::actDisconnect); aSinglePlayer = new QAction(this); - aSinglePlayer->setObjectName("actionSinglePlayer"); connect(aSinglePlayer, &QAction::triggered, this, &MainWindow::actSinglePlayer); aWatchReplay = new QAction(this); - aWatchReplay->setObjectName("actionWatchReplay"); connect(aWatchReplay, &QAction::triggered, this, &MainWindow::actWatchReplay); aFullScreen = new QAction(this); - aFullScreen->setObjectName("actionFullScreen"); aFullScreen->setCheckable(true); connect(aFullScreen, &QAction::toggled, this, &MainWindow::actFullScreen); aRegister = new QAction(this); - aRegister->setObjectName("actionRegister"); connect(aRegister, &QAction::triggered, connectionController, &ConnectionController::registerToServer); aForgotPassword = new QAction(this); - aForgotPassword->setObjectName("actionForgotPassword"); connect(aForgotPassword, &QAction::triggered, connectionController, &ConnectionController::forgotPasswordRequest); aSettings = new QAction(this); - aSettings->setObjectName("actionSettings"); connect(aSettings, &QAction::triggered, this, &MainWindow::actSettings); aExit = new QAction(this); - aExit->setObjectName("actionExit"); connect(aExit, &QAction::triggered, this, &MainWindow::actExit); aManageSets = new QAction(QString(), this); @@ -495,12 +455,6 @@ void MainWindow::createActions() connect(aOpenSettingsFolder, &QAction::triggered, this, &MainWindow::actOpenSettingsFolder); aFirstRunWizard = new QAction(this); connect(aFirstRunWizard, &QAction::triggered, this, [this] { runFirstRunWizard(); }); -#ifdef WITH_GUI_TEST - aRecord = new QAction(this); - connect(aRecord, &QAction::triggered, this, &MainWindow::actRecord); - aPlayRecording = new QAction(this); - connect(aPlayRecording, &QAction::triggered, this, &MainWindow::actPlayRecording); -#endif aShow = new QAction(this); connect(aShow, &QAction::triggered, this, &MainWindow::actShow); @@ -541,9 +495,7 @@ void MainWindow::createActions() void MainWindow::createMenus() { - menuBar()->setObjectName("menubar"); cockatriceMenu = menuBar()->addMenu(QString()); - cockatriceMenu->setObjectName("cockatriceMenu"); cockatriceMenu->addAction(aConnect); cockatriceMenu->addAction(aDisconnect); cockatriceMenu->addAction(aRegister); @@ -559,7 +511,6 @@ void MainWindow::createMenus() cockatriceMenu->addAction(aExit); dbMenu = menuBar()->addMenu(QString()); - dbMenu->setObjectName("dbMenu"); dbMenu->addAction(aManageSets); dbMenu->addAction(aEditTokens); dbMenu->addSeparator(); @@ -570,10 +521,8 @@ void MainWindow::createMenus() dbMenu->addAction(aReloadCardDatabase); tabsMenu = menuBar()->addMenu(QString()); - tabsMenu->setObjectName("tabsMenu"); helpMenu = menuBar()->addMenu(QString()); - helpMenu->setObjectName("helpMenu"); helpMenu->addAction(aAbout); helpMenu->addAction(aTips); helpMenu->addSeparator(); @@ -586,19 +535,12 @@ void MainWindow::createMenus() helpMenu->addAction(aOpenSettingsFolder); helpMenu->addSeparator(); helpMenu->addAction(aFirstRunWizard); -#ifdef WITH_GUI_TEST - helpMenu->addSeparator(); - helpMenu->addAction(aRecord); - helpMenu->addAction(aPlayRecording); -#endif } MainWindow::MainWindow(QWidget *parent) - : QMainWindow(parent), localServer(nullptr), bHasActivated(false), askedForDbUpdater(false), noConnect(false), + : QMainWindow(parent), localServer(nullptr), bHasActivated(false), askedForDbUpdater(false), cardUpdateProcess(nullptr), logviewDialog(nullptr) { - setObjectName("MainWindow"); - connect(&SettingsCache::instance().cacheStorage(), &CacheStorageSettings::pixmapCacheSizeChanged, this, &MainWindow::pixmapCacheSizeChanged); pixmapCacheSizeChanged(SettingsCache::instance().cacheStorage().getPixmapCacheSize()); @@ -684,14 +626,6 @@ MainWindow::MainWindow(QWidget *parent) // run startup check async QTimer::singleShot(0, this, &MainWindow::startupConfigCheck); QTimer::singleShot(0, this, &MainWindow::applyStartupDestination); - -#ifdef WITH_GUI_TEST - testUtility = new pqTestUtility(this); - testUtility->setRecordWithDialog(true); - testUtility->addEventObserver("xml", new XMLEventObserver(this)); - testUtility->addEventSource("xml", new XMLEventSource(this)); - testUtility->eventPlayer()->addWidgetEventPlayer(new pqCockatriceEventPlayer(this)); -#endif } void MainWindow::startupConfigCheck() @@ -846,11 +780,6 @@ void MainWindow::runFirstRunWizard(bool firstRun) */ void MainWindow::applyStartupDestination() { - if (noConnect) { - qCInfo(WindowMainStartupAutoconnectLog) << "Startup destination suppressed (--no-connect)"; - return; - } - // An explicit command-line connect takes precedence over the startup destination. if (!connectTo.isEmpty()) { return; @@ -1120,10 +1049,6 @@ void MainWindow::handleCockatriceLink(const QString &url) void MainWindow::attemptStartupAutoConnect() { - if (noConnect) { - qCInfo(WindowMainStartupAutoconnectLog) << "Auto-connect suppressed (--no-connect)"; - return; - } if (startupAutoConnectAttempted || skipStartupAutoConnect) { return; } diff --git a/cockatrice/src/interface/window_main.h b/cockatrice/src/interface/window_main.h index 48d093a38..9e342e4b0 100644 --- a/cockatrice/src/interface/window_main.h +++ b/cockatrice/src/interface/window_main.h @@ -48,7 +48,6 @@ class QAction; class QToolButton; class QWidget; -class pqTestUtility; struct LocalGameOptions; inline Q_LOGGING_CATEGORY(WindowMainLog, "window_main"); @@ -105,11 +104,6 @@ private slots: void actUpdate(); void actViewLog(); void actOpenSettingsFolder(); -#ifdef WITH_GUI_TEST - void actRecord(); - void actPlayRecording(); -#endif - void actShow(); void showWindowIfHidden(); void onUrlChainFinished(bool connected); @@ -191,9 +185,6 @@ private: *aReloadCardDatabase; QAction *aTips, *aUpdate, *aCheckCardUpdates, *aCheckCardUpdatesBackground, *aFirstRunWizard, *aStatusBar, *aViewLog, *aOpenSettingsFolder; -#ifdef WITH_GUI_TEST - QAction *aRecord, *aPlayRecording; -#endif TabSupervisor *tabSupervisor; IntentUrlParser *urlParser; @@ -217,7 +208,6 @@ private: bool skipStartupAutoConnect = false; bool startupAutoConnectAttempted = false; bool firstRunWizardActive = false; - bool noConnect = false; ///< --no-connect: skip every startup connection QProcess *cardUpdateProcess; QByteArray cardUpdateOutputBuffer; QByteArray cardUpdateErrorOutput; ///< diagnostic output collected from the updater's stderr @@ -229,9 +219,6 @@ private: GameReplay *replay; DlgTipOfTheDay *tip; QUrl connectTo; -#ifdef WITH_GUI_TEST - pqTestUtility *testUtility; -#endif public: explicit MainWindow(QWidget *parent = nullptr); @@ -239,16 +226,6 @@ public: { connectTo = QUrl(QString("cockatrice://%1").arg(url)); } - /** - * @brief Suppresses every startup connection: --connect, auto-connect and the - * configured startup destination. Used by the GUI test harness so a - * playback run never dials out. - */ - void setNoConnect(bool value) - { - noConnect = value; - } - // When set, the window's own startup connection (--connect or auto-connect // on first activation) is skipped. Used for activation launches: the intent // chain triggered by a cockatrice:// URL owns the connection, and letting @@ -289,13 +266,6 @@ public: */ void requestInstallerClose(); -#ifdef WITH_GUI_TEST - pqTestUtility *getTestUtility() const - { - return testUtility; - } -#endif - protected: void closeEvent(QCloseEvent *event) override; void changeEvent(QEvent *event) override; diff --git a/cockatrice/src/main.cpp b/cockatrice/src/main.cpp index b4dc0610f..829a8ac42 100644 --- a/cockatrice/src/main.cpp +++ b/cockatrice/src/main.cpp @@ -39,10 +39,6 @@ #include "single_instance_manager.h" #include "version_string.h" -#ifdef WITH_GUI_TEST -#include "pqTestUtility.h" -#endif - #include #include #include @@ -66,12 +62,10 @@ #include #include #include -#include #include #include #include #include -#include #include #include #include @@ -349,16 +343,9 @@ int main(int argc, char *argv[]) parser.addOptions( {{{"c", "connect"}, QCoreApplication::translate("main", "Connect on startup"), "user:pass@host:port"}, {{"d", "debug-output"}, QCoreApplication::translate("main", "Debug to file")}, - {"simulate-crash", QCoreApplication::translate( - "main", "Show the crash-report dialog (saves a mock file when no DSN is configured)")}, - {"no-connect", QCoreApplication::translate("main", "Do not connect to any server on startup")} -#ifdef WITH_GUI_TEST - , - {{"t", "test"}, - QCoreApplication::translate("main", "Play back a QtTesting XML test file and exit"), - "test-file.xml"} -#endif - }); + {"simulate-crash", + QCoreApplication::translate("main", + "Show the crash-report dialog (saves a mock file when no DSN is configured)")}}); parser.process(app); @@ -496,9 +483,6 @@ int main(int argc, char *argv[]) if (parser.isSet("connect")) { ui.setConnectTo(parser.value("connect")); } - if (parser.isSet("no-connect")) { - ui.setNoConnect(true); - } qCInfo(MainLog) << "MainWindow constructor finished"; ui.setWindowIcon(themePixmap(QStringLiteral("cockatrice"))); @@ -539,27 +523,6 @@ int main(int argc, char *argv[]) handleActivation(file); } -#ifdef WITH_GUI_TEST - if (parser.isSet("test")) { - QString testFile = parser.value("test"); - qCInfo(MainLog) << "Playing test file:" << testFile; - QTimer::singleShot(0, &ui, [&ui, testFile]() { - bool success = ui.getTestUtility()->playTests(testFile); - qCInfo(MainLog) << "Test playback" << (success ? "succeeded" : "FAILED"); - qApp->exit(success ? 0 : 1); - - // An event that fails while a modal dialog is open aborts the dispatcher, - // but the dialog's nested event loop can then keep the application alive - // forever. Hard-exit after a grace period so a red test always terminates. - auto *watchdog = QThread::create([success] { - QThread::sleep(10); - ::_Exit(success ? 0 : 1); - }); - watchdog->start(); - }); - } -#endif - // Connect to future file/URL events from other instances QObject::connect(&instance, &SingleInstanceManager::filesReceived, [&handleActivation](const QStringList &files) { for (const QString &file : files) { diff --git a/external/qttesting b/external/qttesting deleted file mode 160000 index c7bb5d372..000000000 --- a/external/qttesting +++ /dev/null @@ -1 +0,0 @@ -Subproject commit c7bb5d372133e94c75cbdc70b04e039b864ea379 diff --git a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp index 636c603c6..fa75e356a 100644 --- a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp +++ b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp @@ -50,12 +50,11 @@ namespace // The server warns that this account still uses the legacy password hash while this client // holds a scrypt verifier. Sending the verifier would downgrade the proof, so the client // stops; the login only succeeds after the saved verifier is gone and the user types the -// password, which also upgrades the account. Unticking "Save password" drops the verifier -// for the next connection. +// password, which also upgrades the account. constexpr const char *const LOGIN_ABORT_AUTH_DOWNGRADE = QT_TRANSLATE_NOOP( "RemoteClient", "The server still uses an old password format for this account, so the saved login on this computer cannot be " - "used. Untick 'Save password' in the Connect window and log in with your password."); + "used. Untick 'Save password' in the Connect window and connect; when it opens again, type your password."); // The account is still on the legacy hash and only the plaintext password can produce it. constexpr const char *const LOGIN_ABORT_PASSWORD_REQUIRED = QT_TRANSLATE_NOOP("RemoteClient", diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c87bec627..39a3cbe97 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -178,18 +178,6 @@ cockatrice_add_test(NAME user_data_backup_test SOURCES user_data_backup_test.cpp cockatrice_add_test(NAME password_hash_test SOURCES password_hash_test.cpp LIBS libcockatrice_utility) cockatrice_add_test(NAME report_categories_test SOURCES report_categories_test.cpp LIBS libcockatrice_utility) cockatrice_add_test(NAME warning_categories_test SOURCES warning_categories_test.cpp LIBS libcockatrice_utility) -cockatrice_add_test( - NAME - user_games_model_test - SOURCES - ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/widgets/server/user/user_games_model.cpp - user_games_model_test.cpp - LIBS - libcockatrice_network_client_abstract - INCLUDE_DIRS - ${CMAKE_SOURCE_DIR}/cockatrice/src - ${CMAKE_SOURCE_DIR}/libcockatrice_network -) # The installer's holder probe is Windows-only, but the wording it produces and the # question which programs are the payload's own are plain C++ and are what a stuck user # has to act on, so they are tested everywhere. diff --git a/tests/user_games_model_test.cpp b/tests/user_games_model_test.cpp deleted file mode 100644 index 33c29d6d4..000000000 --- a/tests/user_games_model_test.cpp +++ /dev/null @@ -1,248 +0,0 @@ -#include "interface/widgets/server/user/user_games_model.h" - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -RNG_Abstract *rng = nullptr; // required by the transitive server library; this suite sends no game commands - -namespace -{ -class GamesClient : public AbstractClient -{ -public: - QList sent; - - GamesClient() - { - setStatus(StatusLoggedIn); - } - - ~GamesClient() override - { - qDeleteAll(pendingCommands); - pendingCommands.clear(); - } - - void changeStatus(ClientStatus status) - { - setStatus(status); - } - - void answer(int request, Response response) - { - response.set_cmd_id(sent.at(request).cmd_id()); - ServerMessage message; - message.set_message_type(ServerMessage::RESPONSE); - *message.mutable_response() = response; - processProtocolItem(message); - } - -protected: - void sendCommandContainer(const CommandContainer &command) override - { - sent.append(command); - } -}; - -Response gamesResponse(std::initializer_list ids) -{ - Response response; - response.set_response_code(Response::RespOk); - auto *games = response.MutableExtension(Response_GetGamesOfUser::ext); - for (int id : ids) { - auto *game = games->add_game_list(); - game->set_game_id(id); - game->set_room_id(7); - } - return response; -} - -using State = UserGamesModel::State; - -TEST(UserGamesModel, OfflineUserDoesNotSendARequest) -{ - GamesClient client; - UserGamesModel model(&client); - model.setUser("Alice", false); - model.refresh(); - EXPECT_TRUE(client.sent.isEmpty()); - EXPECT_EQ(State::Offline, model.getState()); - EXPECT_FALSE(model.canRefresh()); - EXPECT_EQ(0, model.rowCount()); -} - -TEST(UserGamesModel, PresenceLossInvalidatesTheOutstandingResponse) -{ - GamesClient client; - UserGamesModel model(&client); - model.setUser("Alice", true); - EXPECT_EQ(State::Loading, model.getState()); - EXPECT_FALSE(model.canRefresh()); - model.setUser("Alice", false); - client.answer(0, gamesResponse({1})); - EXPECT_EQ(State::Offline, model.getState()); - EXPECT_EQ(0, model.rowCount()); - - model.setUser("Alice", true); - client.answer(1, gamesResponse({2})); - EXPECT_EQ(State::Ready, model.getState()); - EXPECT_TRUE(model.canRefresh()); - EXPECT_EQ(2, model.data(model.index(0), UserGamesModel::GameRole).value().game_id()); -} - -TEST(UserGamesModel, SwitchingBackToTheSameUserDoesNotAcceptTheirOldRequest) -{ - GamesClient client; - UserGamesModel model(&client); - model.setUser("Alice", true); - model.setUser("Bob", true); - model.setUser("Alice", true); - client.answer(2, gamesResponse({3})); - client.answer(0, gamesResponse({1})); - client.answer(1, gamesResponse({2})); - EXPECT_EQ(1, model.rowCount()); - EXPECT_EQ(3, model.data(model.index(0), UserGamesModel::GameRole).value().game_id()); -} - -TEST(UserGamesModel, RefreshSupersedesThePreviousRequest) -{ - GamesClient client; - UserGamesModel model(&client); - model.setUser("Alice", true); - model.refresh(); - client.answer(0, gamesResponse({1})); - EXPECT_EQ(State::Loading, model.getState()); - EXPECT_EQ(0, model.rowCount()); - client.answer(1, gamesResponse({2})); - EXPECT_EQ(State::Ready, model.getState()); - EXPECT_EQ(2, model.data(model.index(0), UserGamesModel::GameRole).value().game_id()); -} - -TEST(UserGamesModel, EmptySuccessAndMissingPayloadAreDifferentStates) -{ - GamesClient client; - UserGamesModel model(&client); - model.setUser("Alice", true); - client.answer(0, gamesResponse({})); - EXPECT_EQ(State::Empty, model.getState()); - EXPECT_TRUE(model.canRefresh()); - - model.refresh(); - Response missingPayload; - missingPayload.set_response_code(Response::RespOk); - client.answer(1, missingPayload); - EXPECT_EQ(State::Error, model.getState()); - EXPECT_TRUE(model.canRefresh()); -} - -TEST(UserGamesModel, FailedRequestsCanBeRetried) -{ - GamesClient client; - UserGamesModel model(&client); - model.setUser("Alice", true); - Response error; - error.set_response_code(Response::RespNameNotFound); - client.answer(0, error); - EXPECT_EQ(State::Error, model.getState()); - model.refresh(); - client.answer(1, gamesResponse({4})); - EXPECT_EQ(State::Ready, model.getState()); -} - -TEST(UserGamesModel, DisconnectRejectsLateRepliesAndReconnectFetchesAgain) -{ - GamesClient client; - UserGamesModel model(&client); - model.setUser("Alice", true); - client.changeStatus(StatusDisconnecting); - client.answer(0, gamesResponse({1})); - EXPECT_EQ(State::Disconnected, model.getState()); - EXPECT_FALSE(model.canRefresh()); - model.refresh(); - EXPECT_EQ(1, client.sent.size()); - client.changeStatus(StatusLoggedIn); - client.answer(1, gamesResponse({2})); - EXPECT_EQ(State::Ready, model.getState()); -} - -TEST(UserGamesModel, ClientDestructionClearsGamesAndDisablesRefresh) -{ - auto client = std::make_unique(); - UserGamesModel model(client.get()); - model.setUser("Alice", true); - client->answer(0, gamesResponse({1})); - client.reset(); - EXPECT_EQ(State::Disconnected, model.getState()); - EXPECT_EQ(0, model.rowCount()); - EXPECT_FALSE(model.canRefresh()); - model.refresh(); - EXPECT_EQ(State::Disconnected, model.getState()); -} - -TEST(UserGamesModel, ResetInvalidatesAnOpenGameActionAndPreservesTheModelContract) -{ - GamesClient client; - UserGamesModel model(&client); - QAbstractItemModelTester tester(&model, QAbstractItemModelTester::FailureReportingMode::Fatal); - model.setUser("Alice", true); - client.answer(0, gamesResponse({1, 2, 3})); - QPersistentModelIndex selected(model.index(1)); - EXPECT_TRUE(selected.isValid()); - model.refresh(); - EXPECT_FALSE(selected.isValid()); - EXPECT_EQ(0, model.rowCount()); - client.answer(1, gamesResponse({4})); - EXPECT_EQ(1, model.rowCount()); -} - -TEST(UserGamesModel, StateListenerCanSelectAnotherUserBeforeTheRequestIsSent) -{ - GamesClient client; - UserGamesModel model(&client); - bool switched = false; - QObject::connect(&model, &UserGamesModel::stateChanged, &model, [&] { - if (!switched && model.getState() == State::Loading) { - switched = true; - model.setUser("Bob", false); - } - }); - model.setUser("Alice", true); - EXPECT_TRUE(client.sent.isEmpty()); - EXPECT_EQ(State::Offline, model.getState()); -} - -TEST(UserGamesModel, TimedOutResponseCannotReplaceARetry) -{ - GamesClient client; - UserGamesModel model(&client, 50); - model.setUser("Alice", true); - QSignalSpy changes(&model, &UserGamesModel::stateChanged); - ASSERT_TRUE(changes.wait(1000)); - ASSERT_EQ(State::Error, model.getState()); - EXPECT_TRUE(model.canRefresh()); - model.refresh(); - client.answer(0, gamesResponse({1})); - EXPECT_EQ(State::Loading, model.getState()); - client.answer(1, gamesResponse({2})); - EXPECT_EQ(State::Ready, model.getState()); - EXPECT_EQ(2, model.data(model.index(0), UserGamesModel::GameRole).value().game_id()); -} -} // namespace - -int main(int argc, char **argv) -{ - QCoreApplication app(argc, argv); - ::testing::InitGoogleTest(&argc, argv); - return RUN_ALL_TESTS(); -}