[fix] Floating panadapter and applet state not restored across restarts#2057
[fix] Floating panadapter and applet state not restored across restarts#2057chibondking wants to merge 1 commit into
Conversation
Two separate bugs caused popped-out windows to come back docked after every restart. --- Fix 1: Floating panadapter state --- PanadapterStack saved window geometry (position/size) but never recorded which pans were floating. There was no restore path. - PanadapterStack::saveFloatingState() writes the comma-separated list of currently-floating pan IDs to AppSettings["FloatingPanIds"]. Called automatically from floatPanadapter(), dockPanadapter(), and prepareShutdown(). - PanadapterStack::restoreFloatingState() reads "FloatingPanIds" and calls floatPanadapter() for each ID that exists and is not already floating. Safe on reconnect: unknown IDs are skipped, already-floating pans are no-ops. - MainWindow m_layoutRestoreTimer lambda: previously did an early return for single-pan sessions, blocking any restore. Restructured to wrap multi-pan layout code in if (panCount > 1) and always call restoreFloatingState() at the end, for any pan count. --- Fix 2: Floating applet state not restored --- ContainerManager::restoreState() was fully implemented and saveState() was already called on every float/dock transition, but restoreState() was never called anywhere. The ContainerTree JSON key was written but never read back on launch. - ContainerManager::prepareShutdown() now calls saveState() first so the ContainerTree JSON is always current before windows close, even on an unexpected quit. - MainWindow: after new AppletPanel(splitter), a QTimer::singleShot(0) schedules containerManager()->restoreState(). The one-event-loop delay lets AppletPanel's own legacy-migration singleShot(0) timers fire first (they write ContainerTree), then restoreState() reads the up-to-date JSON and re-floats containers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Nice fix, @chibondking — both bugs are clearly diagnosed in the description and the code follows through cleanly. A few observations:
Looks good:
- The restructuring of the
m_layoutRestoreTimerlambda from an earlyreturnto wrapping inif (panCount > 1)is the right approach — it's the minimal change to unblock single-pan floating restore without duplicating logic. saveFloatingState()being called fromfloatPanadapter(),dockPanadapter(), andprepareShutdown()gives good coverage. The belt-and-suspenderssaveState()inContainerManager::prepareShutdown()is similarly well-placed.- The
QTimer::singleShot(0)for container restore with the comment about legacy-migration timer ordering is clear and correct. - The null-guard
if (m_appletPanel && m_appletPanel->containerManager())in the deferred restore is good defensive coding.
One thing to consider (non-blocking):
restoreFloatingState() reads the saved "FloatingPanIds" string, then iterates calling floatPanadapter() — which itself calls saveFloatingState(), overwriting the setting with an intermediate partial list on each iteration. This is functionally correct because the local saved variable already captured the full list, and the final write after the last floatPanadapter() will contain all IDs. But if the app crashes during restore (say after floating pan A but before B and C), the persisted state would only contain "A", losing B and C on the next launch.
A lightweight guard would be to skip the save during restore:
void PanadapterStack::restoreFloatingState()
{
const QString saved =
AppSettings::instance().value("FloatingPanIds", "").toString();
if (saved.isEmpty()) return;
m_restoringFloatingState = true;
for (const QString& id : saved.split(',', Qt::SkipEmptyParts)) {
if (m_pans.contains(id) && !m_floatingWindows.contains(id))
floatPanadapter(id);
}
m_restoringFloatingState = false;
saveFloatingState(); // single write with the complete state
}...and in saveFloatingState():
if (m_restoringFloatingState) return;This is a minor edge case (crash during startup restore) so I'd leave it to your judgment whether it's worth the extra flag.
All four changed files are within scope. No concerns about conventions — uses AppSettings correctly, no raw new without ownership, clean C++20 style.
Thanks for the contribution!
|
Claude here on Jeremy's behalf — auto-closed via the merge trailer in #2065. Wanted to leave the context for posterity: When we pulled this PR locally and tested, the persistence logic brought forward fine but exposed a latent bug in the existing Fix was a 4-line guard in Both changes are credited to you in the merged commit (73b4fe8). Thanks for the careful split-out from #2008. #2058 and #2059 are next on the test list. 73, Jeremy KK7GWY & Claude (AI dev partner) |
Two separate bugs caused popped-out windows to come back docked after every restart.
--- Fix 1: Floating panadapter state ---
PanadapterStack saved window geometry (position/size) but never recorded which pans were floating. There was no restore path.
--- Fix 2: Floating applet state not restored ---
ContainerManager::restoreState() was fully implemented and saveState() was already called on every float/dock transition, but restoreState() was never called anywhere. The ContainerTree JSON key was written but never read back on launch.