From 6b655205a48519f32f9621c4ec89482876e29fa4 Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Mon, 26 Oct 2026 15:28:53 +0000 Subject: [PATCH 1/4] Candidate fix for outcome editor problem on MSW. --- src/gui/efgtooltip.cc | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/gui/efgtooltip.cc b/src/gui/efgtooltip.cc index 0d5d9104b6..7b5acd6620 100644 --- a/src/gui/efgtooltip.cc +++ b/src/gui/efgtooltip.cc @@ -263,7 +263,12 @@ void OutcomeEditorPopup::BuildControls() hintText->Wrap(FromDIP(260)); outerSizer->Add(hintText, 0, wxALL, FromDIP(10)); - m_errorText = new wxStaticText(m_contentPanel, wxID_STATIC, wxEmptyString); + // wxST_NO_AUTORESIZE: SetLabel() on MSW would otherwise measure the text immediately through a + // wxClientDC, which on a hidden control inside a not-yet-shown dialog hits a wx assertion + // (textmeasure.cpp, "Must not be used with non-native wxDCs"). Sizing is left to the layout + // pass instead (see InvalidateBestSize() at the SetLabel call sites). + m_errorText = new wxStaticText(m_contentPanel, wxID_STATIC, wxEmptyString, wxDefaultPosition, + wxDefaultSize, wxST_NO_AUTORESIZE); m_errorText->SetForegroundColour(*wxRED); m_errorText->Wrap(FromDIP(260)); m_errorText->Hide(); @@ -278,6 +283,9 @@ void OutcomeEditorPopup::BuildControls() void OutcomeEditorPopup::LoadValues() { + // ChangeValue() rather than SetValue(): loading must not fire wxEVT_TEXT, or UpdateValidation() + // runs against a half-loaded form (label set, payoffs still empty) and reports a spurious + // error. BeginEdit() validates once, after everything is loaded. const GameOutcome outcome = m_node ? m_node->GetOutcome() : nullptr; if (!outcome || outcome->IsNull()) { @@ -285,15 +293,15 @@ void OutcomeEditorPopup::LoadValues() // or duplicate label outright -- pre-filling a fresh, unique one here means the dialog never // opens already showing that as an error the user has to notice and fix before they can // accept anything else. - m_labelCtrl->SetValue(wxString::FromUTF8(GenerateOutcomeLabel(m_doc->GetGame()))); + m_labelCtrl->ChangeValue(wxString::FromUTF8(GenerateOutcomeLabel(m_doc->GetGame()))); for (auto &row : m_payoffRows) { *row.payoffValue = wxT("0"); - row.payoffCtrl->SetValue(*row.payoffValue); + row.payoffCtrl->ChangeValue(*row.payoffValue); } return; } - m_labelCtrl->SetValue(wxString::FromUTF8(outcome->GetLabel())); + m_labelCtrl->ChangeValue(wxString::FromUTF8(outcome->GetLabel())); const Game game = m_doc->GetGame(); @@ -306,7 +314,7 @@ void OutcomeEditorPopup::LoadValues() // this dialog actually intends to display rather than a stale one from construction or the // previous node edited. *row.payoffValue = wxString(payoff.c_str(), *wxConvCurrent); - row.payoffCtrl->SetValue(*row.payoffValue); + row.payoffCtrl->ChangeValue(*row.payoffValue); } } @@ -497,6 +505,7 @@ void OutcomeEditorPopup::UpdateValidation() else { m_errorText->SetLabel(message); m_errorText->Wrap(FromDIP(260)); + m_errorText->InvalidateBestSize(); // wxST_NO_AUTORESIZE: the layout below must re-measure m_errorText->Show(); m_contentPanel->Layout(); } @@ -560,6 +569,7 @@ void OutcomeEditorPopup::ShowValidationFailure(const wxString &p_message, wxText if (m_errorText) { m_errorText->SetLabel(p_message); m_errorText->Wrap(FromDIP(260)); + m_errorText->InvalidateBestSize(); m_errorText->Show(); } @@ -842,7 +852,9 @@ void AppendMovePopup::BuildControls() m_hintText->SetFont(hintFont); outerSizer->Add(m_hintText, 0, wxALL, FromDIP(10)); - m_errorText = new wxStaticText(m_contentPanel, wxID_ANY, wxEmptyString); + // wxST_NO_AUTORESIZE for the same reason as OutcomeEditorPopup's error text. + m_errorText = new wxStaticText(m_contentPanel, wxID_ANY, wxEmptyString, wxDefaultPosition, + wxDefaultSize, wxST_NO_AUTORESIZE); m_errorText->SetForegroundColour(*wxRED); m_errorText->Wrap(FromDIP(260)); m_errorText->Hide(); @@ -1229,6 +1241,7 @@ void AppendMovePopup::UpdateValidation() else { m_errorText->SetLabel(message); m_errorText->Wrap(FromDIP(260)); + m_errorText->InvalidateBestSize(); // wxST_NO_AUTORESIZE: the layout must re-measure m_errorText->Show(); m_contentPanel->Layout(); } @@ -1306,6 +1319,7 @@ void AppendMovePopup::ShowValidationFailure(const wxString &p_message, wxTextCtr if (m_errorText) { m_errorText->SetLabel(p_message); m_errorText->Wrap(FromDIP(260)); + m_errorText->InvalidateBestSize(); // wxST_NO_AUTORESIZE: the layout must re-measure m_errorText->Show(); } From ab1e2e2042d71da87e7112d8a6542be480b4d134 Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Mon, 26 Oct 2026 16:04:08 +0000 Subject: [PATCH 2/4] Another possible fix for MSW. --- src/gui/efgtooltip.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/gui/efgtooltip.cc b/src/gui/efgtooltip.cc index 7b5acd6620..c48b317288 100644 --- a/src/gui/efgtooltip.cc +++ b/src/gui/efgtooltip.cc @@ -503,10 +503,10 @@ void OutcomeEditorPopup::UpdateValidation() ClearValidationFailure(); } else { + m_errorText->Show(); // wx's MSW text measurement fails on a hidden control: show it first m_errorText->SetLabel(message); m_errorText->Wrap(FromDIP(260)); m_errorText->InvalidateBestSize(); // wxST_NO_AUTORESIZE: the layout below must re-measure - m_errorText->Show(); m_contentPanel->Layout(); } Fit(); @@ -567,10 +567,10 @@ void OutcomeEditorPopup::ShowValidationFailure(const wxString &p_message, wxText wxBell(); if (m_errorText) { + m_errorText->Show(); // wx's MSW text measurement fails on a hidden control: show it first m_errorText->SetLabel(p_message); m_errorText->Wrap(FromDIP(260)); m_errorText->InvalidateBestSize(); - m_errorText->Show(); } if (m_contentPanel) { @@ -1239,10 +1239,10 @@ void AppendMovePopup::UpdateValidation() ClearValidationFailure(); } else { + m_errorText->Show(); // wx's MSW text measurement fails on a hidden control: show it first m_errorText->SetLabel(message); m_errorText->Wrap(FromDIP(260)); m_errorText->InvalidateBestSize(); // wxST_NO_AUTORESIZE: the layout must re-measure - m_errorText->Show(); m_contentPanel->Layout(); } Fit(); @@ -1317,10 +1317,10 @@ void AppendMovePopup::ShowValidationFailure(const wxString &p_message, wxTextCtr wxBell(); if (m_errorText) { + m_errorText->Show(); // wx's MSW text measurement fails on a hidden control: show it first m_errorText->SetLabel(p_message); m_errorText->Wrap(FromDIP(260)); m_errorText->InvalidateBestSize(); // wxST_NO_AUTORESIZE: the layout must re-measure - m_errorText->Show(); } if (m_contentPanel) { From 40df5365c48caec1ed4e6d3433c6a1d385338ec7 Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Mon, 26 Oct 2026 16:40:42 +0000 Subject: [PATCH 3/4] Fix for MSW bug, incorrectly setting the parents of popup dialogs --- src/gui/efgdisplay.h | 11 ++++++----- src/gui/efgtooltip.cc | 43 ++++++++++++++++++++++--------------------- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/gui/efgdisplay.h b/src/gui/efgdisplay.h index ab002c893f..8384955f06 100644 --- a/src/gui/efgdisplay.h +++ b/src/gui/efgdisplay.h @@ -76,11 +76,12 @@ class EfgDisplay final : public wxScrolledWindow, public GameView { GameNode m_dragOverNode; bool m_dragOverValid{false}; - // Constructs m_outcomeEditor/m_appendMoveEditor/m_nodeInfoPopup (efgtooltip.cc) and installs - // the tree drop target (efgdragdrop.cc), respectively. Called from the constructor body -- - // rather than its initializer list -- since OutcomeEditorPopup/AppendMovePopup/NodeInfoPopup/ - // TreeDropTarget are only forward-declared here; InitPopups() must run before OnUpdate(), - // which dereferences m_nodeInfoPopup via DismissNodeInfo(). + // Constructs m_nodeInfoPopup (efgtooltip.cc) and installs the tree drop target + // (efgdragdrop.cc), respectively. Called from the constructor body -- rather than its + // initializer list -- since NodeInfoPopup/TreeDropTarget are only forward-declared here; + // InitPopups() must run before OnUpdate(), which dereferences m_nodeInfoPopup via + // DismissNodeInfo(). m_outcomeEditor and m_appendMoveEditor are created lazily, on first use + // (see InitPopups()). void InitPopups(); void InitDropTarget(); diff --git a/src/gui/efgtooltip.cc b/src/gui/efgtooltip.cc index c48b317288..c69ff583e6 100644 --- a/src/gui/efgtooltip.cc +++ b/src/gui/efgtooltip.cc @@ -263,12 +263,7 @@ void OutcomeEditorPopup::BuildControls() hintText->Wrap(FromDIP(260)); outerSizer->Add(hintText, 0, wxALL, FromDIP(10)); - // wxST_NO_AUTORESIZE: SetLabel() on MSW would otherwise measure the text immediately through a - // wxClientDC, which on a hidden control inside a not-yet-shown dialog hits a wx assertion - // (textmeasure.cpp, "Must not be used with non-native wxDCs"). Sizing is left to the layout - // pass instead (see InvalidateBestSize() at the SetLabel call sites). - m_errorText = new wxStaticText(m_contentPanel, wxID_STATIC, wxEmptyString, wxDefaultPosition, - wxDefaultSize, wxST_NO_AUTORESIZE); + m_errorText = new wxStaticText(m_contentPanel, wxID_STATIC, wxEmptyString); m_errorText->SetForegroundColour(*wxRED); m_errorText->Wrap(FromDIP(260)); m_errorText->Hide(); @@ -503,10 +498,9 @@ void OutcomeEditorPopup::UpdateValidation() ClearValidationFailure(); } else { - m_errorText->Show(); // wx's MSW text measurement fails on a hidden control: show it first m_errorText->SetLabel(message); m_errorText->Wrap(FromDIP(260)); - m_errorText->InvalidateBestSize(); // wxST_NO_AUTORESIZE: the layout below must re-measure + m_errorText->Show(); m_contentPanel->Layout(); } Fit(); @@ -567,10 +561,9 @@ void OutcomeEditorPopup::ShowValidationFailure(const wxString &p_message, wxText wxBell(); if (m_errorText) { - m_errorText->Show(); // wx's MSW text measurement fails on a hidden control: show it first m_errorText->SetLabel(p_message); m_errorText->Wrap(FromDIP(260)); - m_errorText->InvalidateBestSize(); + m_errorText->Show(); } if (m_contentPanel) { @@ -852,9 +845,7 @@ void AppendMovePopup::BuildControls() m_hintText->SetFont(hintFont); outerSizer->Add(m_hintText, 0, wxALL, FromDIP(10)); - // wxST_NO_AUTORESIZE for the same reason as OutcomeEditorPopup's error text. - m_errorText = new wxStaticText(m_contentPanel, wxID_ANY, wxEmptyString, wxDefaultPosition, - wxDefaultSize, wxST_NO_AUTORESIZE); + m_errorText = new wxStaticText(m_contentPanel, wxID_ANY, wxEmptyString); m_errorText->SetForegroundColour(*wxRED); m_errorText->Wrap(FromDIP(260)); m_errorText->Hide(); @@ -1239,10 +1230,9 @@ void AppendMovePopup::UpdateValidation() ClearValidationFailure(); } else { - m_errorText->Show(); // wx's MSW text measurement fails on a hidden control: show it first m_errorText->SetLabel(message); m_errorText->Wrap(FromDIP(260)); - m_errorText->InvalidateBestSize(); // wxST_NO_AUTORESIZE: the layout must re-measure + m_errorText->Show(); m_contentPanel->Layout(); } Fit(); @@ -1317,10 +1307,9 @@ void AppendMovePopup::ShowValidationFailure(const wxString &p_message, wxTextCtr wxBell(); if (m_errorText) { - m_errorText->Show(); // wx's MSW text measurement fails on a hidden control: show it first m_errorText->SetLabel(p_message); m_errorText->Wrap(FromDIP(260)); - m_errorText->InvalidateBestSize(); // wxST_NO_AUTORESIZE: the layout must re-measure + m_errorText->Show(); } if (m_contentPanel) { @@ -1547,33 +1536,45 @@ void NodeInfoPopup::ShowForNode(const GameNode &p_node) void EfgDisplay::InitPopups() { - m_outcomeEditor = new OutcomeEditorPopup(this, m_doc); - m_appendMoveEditor = new AppendMovePopup(this, m_doc); + // m_outcomeEditor and m_appendMoveEditor are wxDialogs, created on first use rather than here: + // on MSW a dialog's native owner is chosen at creation, and when this EfgDisplay's frame is not + // yet shown (as it isn't during GameFrame's constructor) wx falls back to whichever window is + // active -- typically the welcome frame, which is destroyed straight after, taking the dialogs' + // native windows with it. m_nodeInfoPopup = new NodeInfoPopup(this, m_doc); } void EfgDisplay::PostPendingChanges() { - if (m_outcomeEditor->IsShown()) { + if (m_outcomeEditor && m_outcomeEditor->IsShown()) { m_outcomeEditor->Commit(); } - if (m_appendMoveEditor->IsShown()) { + if (m_appendMoveEditor && m_appendMoveEditor->IsShown()) { m_appendMoveEditor->Commit(); } } void EfgDisplay::BeginEditOutcome(const GameNode &p_node, int p_initialPlayer) { + if (!m_outcomeEditor) { + m_outcomeEditor = new OutcomeEditorPopup(this, m_doc); + } m_outcomeEditor->BeginEdit(p_node, p_initialPlayer); } void EfgDisplay::BeginAppendMove(const GameNode &p_node, const GamePlayer &p_player) { + if (!m_appendMoveEditor) { + m_appendMoveEditor = new AppendMovePopup(this, m_doc); + } m_appendMoveEditor->BeginAppend(p_node, p_player); } void EfgDisplay::BeginInsertMove(const GameNode &p_node, const GamePlayer &p_player) { + if (!m_appendMoveEditor) { + m_appendMoveEditor = new AppendMovePopup(this, m_doc); + } m_appendMoveEditor->BeginInsert(p_node, p_player); } From f90f826d15962d3683f8fc970585f072943c0f01 Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Mon, 26 Oct 2026 16:58:02 +0000 Subject: [PATCH 4/4] Fix null dereferencing --- src/gui/app.cc | 22 ++++++++++++++++++++++ src/gui/app.h | 1 + src/gui/efgtooltip.cc | 19 +++++++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/src/gui/app.cc b/src/gui/app.cc index 05a8e8b0f1..690aad55e4 100644 --- a/src/gui/app.cc +++ b/src/gui/app.cc @@ -32,6 +32,7 @@ #include "games.h" #include "app.h" +#include "dlexcept.h" #include "gameframe.h" #include "welcome.h" @@ -228,6 +229,27 @@ std::vector Application::GetRecentFiles() const return files; } +// Reached when an exception escapes an event handler. The wx default is a bare Abort/Retry/Ignore +// box that says nothing about what was thrown; report the message instead and carry on, as +// "Ignore" would. +bool Application::OnExceptionInMainLoop() +{ + std::string message; + try { + throw; + } + catch (const std::exception &ex) { + message = ex.what(); + } + catch (...) { + message = "An unknown exception was thrown."; + } + ExceptionDialog dialog(GetTopWindow(), message); + dialog.SetExtendedMessage(_("The operation was abandoned. Gambit will continue running.")); + dialog.ShowModal(); + return true; +} + bool Application::ForgetRecentFile(const wxString &p_filename) { for (size_t i = 0; i < m_fileHistory.GetCount(); i++) { diff --git a/src/gui/app.h b/src/gui/app.h index b4bb763297..97a33c2e21 100644 --- a/src/gui/app.h +++ b/src/gui/app.h @@ -50,6 +50,7 @@ class Application final : public wxApp { wxTimer m_splashDismissTimer; bool OnInit() override; + bool OnExceptionInMainLoop() override; void DismissSplash(); void OnSplashDismissTimer(wxTimerEvent &) { DismissSplash(); } //! Drop `p_filename` from the recently-used list; returns whether it was there. diff --git a/src/gui/efgtooltip.cc b/src/gui/efgtooltip.cc index c69ff583e6..081d07d497 100644 --- a/src/gui/efgtooltip.cc +++ b/src/gui/efgtooltip.cc @@ -315,6 +315,10 @@ void OutcomeEditorPopup::LoadValues() void OutcomeEditorPopup::PositionPopup() { + if (!m_node) { + return; + } + auto entry = m_owner->GetLayout().GetNodeEntry(m_node); if (!entry) { return; @@ -488,6 +492,12 @@ wxString OutcomeEditorPopup::ValidatePayoffs() void OutcomeEditorPopup::UpdateValidation() { + // A focus-loss event delivered while the window is hiding on cancel arrives after OnDismiss() + // has cleared m_node, with nothing left to validate or position against. + if (!m_node) { + return; + } + wxString message = ValidateLabel(); const wxString payoffMessage = ValidatePayoffs(); // always run, to keep every field's // highlighting current, not just the first @@ -1025,6 +1035,10 @@ void AppendMovePopup::OnFieldKeyDown(wxKeyEvent &p_event, wxTextCtrl *p_ctrl) void AppendMovePopup::PositionPopup() { + if (!m_node) { + return; + } + auto entry = m_owner->GetLayout().GetNodeEntry(m_node); if (!entry) { return; @@ -1218,6 +1232,11 @@ wxString AppendMovePopup::ValidateProbabilities(std::vector &p_probs) void AppendMovePopup::UpdateValidation() { + // See OutcomeEditorPopup::UpdateValidation(). + if (!m_node) { + return; + } + wxString message = ValidateLabels(); if (m_isChance) { std::vector probs;