From d6d3fa65cf64b5117ce200f77432caa65d7f2c30 Mon Sep 17 00:00:00 2001 From: Stefan Reinauer Date: Fri, 12 Jun 2026 00:21:03 -0700 Subject: [PATCH] od-unix: fix config values lost in the Qt launcher round trip Loading a configuration file could silently change it (GitHub issue #1, "CPU and RAM are often not the ones I set"): - The chipset_compatible handler synced the quickstart model combo without blocking its signals, so loading any config with a chipset_compatible value re-applied the whole quickstart model preset over the explicit settings (FPU, compatibility flags, floppy count). - FpuInternal was -1, which QButtonGroup treats as auto-assign and checkedId() returns for "no selection": the CPU-internal FPU button could never be selected programmatically, so fpu_model vanished from every 68040/68060 configuration on save, start, and each F12 visit. - nr_floppies had no load handler, so drive-enable states never followed the loaded config. Configs without a quickstart line also no longer gain a synthesized quickstart key: quickstart mode now turns off when loading a full configuration and is re-enabled by an explicit quickstart setting. Verified by a new roundtrip harness: WINUAE_QT_CONFIG_ROUNDTRIP_OUT makes the launcher load the given config, export the merged config, and exit; unix-smoke-config-roundtrip.sh (in winuae_unix_smoke_basic) compares CPU/FPU/MMU/memory/floppy keys across three machine profiles. --- CMakeLists.txt | 2 + od-unix/qt/launcher.cpp | 53 +++++++++++- tools/unix-smoke-config-roundtrip.sh | 125 +++++++++++++++++++++++++++ 3 files changed, 178 insertions(+), 2 deletions(-) create mode 100755 tools/unix-smoke-config-roundtrip.sh diff --git a/CMakeLists.txt b/CMakeLists.txt index e2b3c4c2..8541987c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1450,11 +1450,13 @@ if(WINUAE_UNIX_BUILD_EXECUTABLE) add_winuae_unix_smoke_target(winuae_unix_smoke_ripple_hdf unix-smoke-ripple-hdf.sh) add_winuae_unix_smoke_target(winuae_unix_smoke_serial_tcp unix-smoke-serial-tcp.sh) add_winuae_unix_smoke_target(winuae_unix_smoke_path_config unix-smoke-path-config.sh) + add_winuae_unix_smoke_target(winuae_unix_smoke_config_roundtrip unix-smoke-config-roundtrip.sh) if(WINUAE_UNIX_ENABLE_JIT) add_winuae_unix_smoke_target(winuae_unix_smoke_jit unix-smoke-jit.sh) endif() add_custom_target(winuae_unix_smoke_basic DEPENDS winuae_unix_smoke_a1200 winuae_unix_smoke_path_config + winuae_unix_smoke_config_roundtrip ) endif() diff --git a/od-unix/qt/launcher.cpp b/od-unix/qt/launcher.cpp index 602e34a1..8e044556 100644 --- a/od-unix/qt/launcher.cpp +++ b/od-unix/qt/launcher.cpp @@ -105,7 +105,9 @@ static bool systemPrefersDarkMode(); static void applyApplicationColors(QApplication &app, bool dark); -static constexpr int FpuInternal = -1; +/* QButtonGroup ids must not be -1 (it means auto-assign, and checkedId() + * returns -1 for "no selection"). */ +static constexpr int FpuInternal = 1; static constexpr int MaxMountEntries = 8; static constexpr int MaxControllerUnits = 8; static constexpr int MaxCdSlots = 8; @@ -5434,6 +5436,17 @@ public: activateWindow(); } + bool exportMergedConfig(const QString &path) + { + WinUaeQtConfig config = mergedConfig(); + QString error; + if (!config.save(path, &error)) { + std::fprintf(stderr, "config roundtrip export failed: %s\n", error.toLocal8Bit().constData()); + return false; + } + return true; + } + private: WinUaeQtHardwareInfoProvider hardwareProvider; WinUaeQtBoardCatalog boardCatalog; @@ -15916,6 +15929,16 @@ private: inputMappingSettings.clear(); inputOwnedMappingKeys.clear(); hardwareOrderOwnedKeys.clear(); + /* A config without a quickstart line is a full configuration: leave + * quickstart mode off so no quickstart key gets synthesized into the + * merged config. The quickstart key handler re-enables it. */ + if (quickstartMode) { + const QSignalBlocker quickstartBlocker(quickstartMode); + quickstartMode->setChecked(false); + if (quickstartSetConfig) { + quickstartSetConfig->setVisible(true); + } + } moveQuickstartBeforeOverrides(config); for (const WinUaeQtConfig::Setting &setting : config.orderedSettings()) { applySetting(setting.key, setting.value); @@ -16134,7 +16157,14 @@ private: chipset->setCurrentText(chipsetText(value)); } else if (key == QStringLiteral("chipset_compatible")) { chipsetCompatible->setCurrentText(value); - quickModel->setCurrentText(value); + /* Sync the quickstart model display only; applying the model + * preset here would overwrite the explicit settings of the + * config being loaded. */ + { + const QSignalBlocker modelBlocker(quickModel); + quickModel->setCurrentText(value); + } + refreshQuickstartConfigurationChoices(); } else if (key == QStringLiteral("ntsc")) { chipsetNtsc->setChecked(configBoolValue(value)); ntsc->setChecked(configBoolValue(value)); @@ -16262,6 +16292,11 @@ private: updateCpuControlState(); } else if (key == QStringLiteral("fpu_model")) { setFpuButton(value.toInt()); + } else if (key == QStringLiteral("nr_floppies")) { + const int count = qBound(0, value.toInt(), 4); + for (int i = 0; i < 4; i++) { + dfEnable[i]->setChecked(i < count); + } } else if (key == QStringLiteral("cpu_24bit_addressing")) { cpu24Bit->setChecked(value.compare(QStringLiteral("true"), Qt::CaseInsensitive) == 0); updateCpuControlState(); @@ -16970,6 +17005,20 @@ WinUaeQtLauncherResult runWinUaeQtLauncherForConfig(QApplication &app, const QSt dialog.openConfigFile(path); }); } + /* Test hook: load the initial config, immediately export the merged + * config, and exit. Lets the smoke tests verify that configuration + * values survive the trip through the UI widgets. */ + const QByteArray roundtripOut = qgetenv("WINUAE_QT_CONFIG_ROUNDTRIP_OUT"); + if (!roundtripOut.isEmpty()) { + WinUaeQtLauncherResult roundtripResult; + roundtripResult.status = WinUaeQtLauncherStatus::Canceled; + if (!dialog.exportMergedConfig(QString::fromLocal8Bit(roundtripOut))) { + roundtripResult.status = WinUaeQtLauncherStatus::Error; + roundtripResult.exitCode = 1; + roundtripResult.error = QStringLiteral("config roundtrip export failed"); + } + return roundtripResult; + } armQtSmokeExit(dialog); if (dialog.exec() == QDialog::Accepted) { return dialog.launcherResult(); diff --git a/tools/unix-smoke-config-roundtrip.sh b/tools/unix-smoke-config-roundtrip.sh new file mode 100755 index 00000000..20858a71 --- /dev/null +++ b/tools/unix-smoke-config-roundtrip.sh @@ -0,0 +1,125 @@ +#!/usr/bin/env sh +set -eu + +# Verifies that configuration values survive the trip through the Qt +# launcher's widgets: load a config, export the merged config, and compare +# the keys that GitHub issue #1 reported as unreliable (CPU, FPU, memory). + +ROOT_DIR=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd) +BUILD_DIR=${WINUAE_BUILD_DIR:-/tmp/winuae_cmake_build} +EXE=${WINUAE_EXE:-"$BUILD_DIR/winuae"} +WORK=$(mktemp -d /tmp/winuae_config_roundtrip.XXXXXX) +trap 'rm -rf "$WORK"' EXIT INT TERM + +if [ ! -x "$EXE" ]; then + echo "error: emulator executable not found: $EXE" >&2 + exit 1 +fi + +config_value() { + key=$1 + file=$2 + sed -n "s/^${key}=//p" "$file" | head -n 1 +} + +normalize_bool() { + case "$1" in + true|yes|1) echo "true" ;; + false|no|0|"") echo "false" ;; + *) echo "$1" ;; + esac +} + +run_case() { + name=$1 + input=$2 + keys=$3 + output="$WORK/$name-out.uae" + + env QT_QPA_PLATFORM=offscreen \ + WINUAE_INI="$WORK/$name.ini" \ + WINUAE_QT_CONFIG_ROUNDTRIP_OUT="$output" \ + "$EXE" -f "$input" >/dev/null 2>&1 || { + echo "error: $name: emulator exited with failure" >&2 + exit 1 + } + if [ ! -f "$output" ]; then + echo "error: $name: no merged config was exported" >&2 + exit 1 + fi + + failed=0 + for key in $keys; do + in_value=$(config_value "$key" "$input") + out_value=$(config_value "$key" "$output") + case "$in_value" in + true|false|yes|no) + in_value=$(normalize_bool "$in_value") + out_value=$(normalize_bool "$out_value") + ;; + esac + if [ "$in_value" != "$out_value" ]; then + echo "FAIL: $name: $key: '$in_value' became '$out_value'" >&2 + failed=1 + fi + done + if [ "$failed" -ne 0 ]; then + echo "--- merged config ---" >&2 + cat "$output" >&2 + exit 1 + fi + echo "ok: $name" +} + +cat > "$WORK/a4000.uae" < "$WORK/a500.uae" < "$WORK/a3000.uae" <