From 21fb20156fa4f546b918ef57507ff729acb69531 Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Tue, 9 Dec 2025 09:15:20 +0000 Subject: [PATCH 1/6] Modernise reveal dialog and correct logic reversal (was not showing player names!) --- src/gui/dlefgreveal.cc | 64 +++++++++++++++++++----------------------- src/gui/dlefgreveal.h | 17 ++++++----- 2 files changed, 39 insertions(+), 42 deletions(-) diff --git a/src/gui/dlefgreveal.cc b/src/gui/dlefgreveal.cc index c2034e7a9..f76c5aad4 100644 --- a/src/gui/dlefgreveal.cc +++ b/src/gui/dlefgreveal.cc @@ -29,60 +29,54 @@ #include "dlefgreveal.h" namespace Gambit::GUI { -//========================================================================= -// RevealMoveDialog: Member functions -//========================================================================= RevealMoveDialog::RevealMoveDialog(wxWindow *p_parent, GameDocument *p_doc) - : wxDialog(p_parent, wxID_ANY, _("Reveal this move to players"), wxDefaultPosition), m_doc(p_doc) + : wxDialog(p_parent, wxID_ANY, _("Reveal this move to players"), wxDefaultPosition, + wxDefaultSize, wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER), + m_doc(p_doc) { auto *topSizer = new wxBoxSizer(wxVERTICAL); - auto *playerBox = new wxStaticBoxSizer(wxHORIZONTAL, this, _("Reveal the move to players")); + auto *playerBox = new wxStaticBoxSizer(wxVERTICAL, this, _("Reveal the move to players")); - auto *boxSizer = new wxBoxSizer(wxVERTICAL); + const auto &players = m_doc->GetGame()->GetPlayers(); + m_entries.reserve(players.size()); - for (size_t pl = 1; pl <= m_doc->NumPlayers(); pl++) { - auto player = m_doc->GetGame()->GetPlayer(pl); - if (player->GetLabel().empty()) { - m_players.push_back( - new wxCheckBox(this, wxID_ANY, wxString(player->GetLabel().c_str(), *wxConvCurrent))); + for (const auto &player : players) { + wxString label; + if (!player->GetLabel().empty()) { + label = wxString::FromUTF8(player->GetLabel()); } else { - m_players.push_back(new wxCheckBox(this, wxID_ANY, wxString::Format(_T("Player %d"), pl))); + label = wxString::Format("Player %u", player->GetNumber()); } - m_players[pl]->SetValue(true); - m_players[pl]->SetForegroundColour(m_doc->GetStyle().GetPlayerColor(player)); - boxSizer->Add(m_players[pl], 1, wxALL | wxEXPAND, 0); + auto *cb = new wxCheckBox(this, wxID_ANY, label); + cb->SetValue(true); + cb->SetForegroundColour(m_doc->GetStyle().GetPlayerColor(player)); + + m_entries.push_back({player, cb}); + playerBox->Add(cb, wxSizerFlags().Expand().Border(wxALL, 2)); } - playerBox->Add(boxSizer, 1, wxALL | wxEXPAND, 5); - topSizer->Add(playerBox, 1, wxALL | wxEXPAND, 5); - auto *buttonSizer = new wxBoxSizer(wxHORIZONTAL); - buttonSizer->Add(new wxButton(this, wxID_CANCEL, _("Cancel")), 0, wxALL, 5); - auto *okButton = new wxButton(this, wxID_OK, _("OK")); - okButton->SetDefault(); - buttonSizer->Add(okButton, 0, wxALL, 5); - topSizer->Add(buttonSizer, 0, wxALL | wxALIGN_RIGHT, 5); + topSizer->Add(playerBox, wxSizerFlags(1).Expand().Border()); + + auto *buttonSizer = CreateStdDialogButtonSizer(wxOK | wxCANCEL); + topSizer->Add(buttonSizer, wxSizerFlags().Right().Border()); - SetSizer(topSizer); - topSizer->Fit(this); - topSizer->SetSizeHints(this); - wxTopLevelWindowBase::Layout(); + SetSizerAndFit(topSizer); CenterOnParent(); } -Array RevealMoveDialog::GetPlayers() const +std::vector RevealMoveDialog::GetPlayers() const { - Array players; - - for (size_t pl = 1; pl <= m_doc->NumPlayers(); pl++) { - if (m_players[pl]->GetValue()) { - players.push_back(m_doc->GetGame()->GetPlayer(pl)); + std::vector result; + result.reserve(m_entries.size()); + for (const auto &[player, checkbox] : m_entries) { + if (checkbox->GetValue()) { + result.push_back(player); } } - - return players; + return result; } } // namespace Gambit::GUI diff --git a/src/gui/dlefgreveal.h b/src/gui/dlefgreveal.h index 93acb8037..786be739e 100644 --- a/src/gui/dlefgreveal.h +++ b/src/gui/dlefgreveal.h @@ -28,15 +28,18 @@ namespace Gambit::GUI { class RevealMoveDialog final : public wxDialog { - GameDocument *m_doc; - Array m_players; + GameDocument *m_doc{nullptr}; -public: - // Lifecycle - RevealMoveDialog(wxWindow *, GameDocument *); + struct PlayerEntry { + GamePlayer player; + wxCheckBox *checkbox; + }; + + std::vector m_entries; - // Data access (only valid when ShowModal() returns with wxID_OK) - Array GetPlayers() const; +public: + RevealMoveDialog(wxWindow *p_parent, GameDocument *p_doc); + std::vector GetPlayers() const; }; } // end namespace Gambit::GUI From 6e9f873ea23ab26e69f7787146b88e8164235ad6 Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Tue, 9 Dec 2025 09:23:32 +0000 Subject: [PATCH 2/6] Hide implementation detail of reveal dialog --- src/gui/dlefgreveal.cc | 24 ++++++++++++++++++++++++ src/gui/dlefgreveal.h | 15 +-------------- src/gui/gameframe.cc | 9 ++++----- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/gui/dlefgreveal.cc b/src/gui/dlefgreveal.cc index f76c5aad4..4e2c62488 100644 --- a/src/gui/dlefgreveal.cc +++ b/src/gui/dlefgreveal.cc @@ -30,6 +30,21 @@ namespace Gambit::GUI { +class RevealMoveDialog final : public wxDialog { + GameDocument *m_doc{nullptr}; + + struct PlayerEntry { + GamePlayer player; + wxCheckBox *checkbox; + }; + + std::vector m_entries; + +public: + RevealMoveDialog(wxWindow *p_parent, GameDocument *p_doc); + std::vector GetPlayers() const; +}; + RevealMoveDialog::RevealMoveDialog(wxWindow *p_parent, GameDocument *p_doc) : wxDialog(p_parent, wxID_ANY, _("Reveal this move to players"), wxDefaultPosition, wxDefaultSize, wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER), @@ -79,4 +94,13 @@ std::vector RevealMoveDialog::GetPlayers() const return result; } +std::optional> RevealMove(wxWindow *p_parent, GameDocument *p_doc) +{ + RevealMoveDialog dialog(p_parent, p_doc); + if (dialog.ShowModal() == wxID_OK) { + return dialog.GetPlayers(); + } + return std::nullopt; +} + } // namespace Gambit::GUI diff --git a/src/gui/dlefgreveal.h b/src/gui/dlefgreveal.h index 786be739e..54babb593 100644 --- a/src/gui/dlefgreveal.h +++ b/src/gui/dlefgreveal.h @@ -27,20 +27,7 @@ namespace Gambit::GUI { -class RevealMoveDialog final : public wxDialog { - GameDocument *m_doc{nullptr}; - - struct PlayerEntry { - GamePlayer player; - wxCheckBox *checkbox; - }; - - std::vector m_entries; - -public: - RevealMoveDialog(wxWindow *p_parent, GameDocument *p_doc); - std::vector GetPlayers() const; -}; +std::optional> RevealMove(wxWindow *p_parent, GameDocument *p_doc); } // end namespace Gambit::GUI diff --git a/src/gui/gameframe.cc b/src/gui/gameframe.cc index e66aa63f2..7870805e8 100644 --- a/src/gui/gameframe.cc +++ b/src/gui/gameframe.cc @@ -980,12 +980,11 @@ void GameFrame::OnEditRemoveOutcome(wxCommandEvent &) void GameFrame::OnEditReveal(wxCommandEvent &) { - RevealMoveDialog dialog(this, m_doc); - - if (dialog.ShowModal() == wxID_OK) { + if (const auto players = RevealMove(this, m_doc); players) { try { - for (const auto &player : dialog.GetPlayers()) { - m_doc->DoRevealAction(m_doc->GetSelectNode()->GetInfoset(), player); + const auto &infoset = m_doc->GetSelectNode()->GetInfoset(); + for (const auto &player : *players) { + m_doc->DoRevealAction(infoset, player); } } catch (std::exception &ex) { From 0cef7a5ce7c90915dc979482669beaae3f00d63f Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Tue, 9 Dec 2025 10:52:29 +0000 Subject: [PATCH 3/6] Some dialog tidying. --- src/gui/dlefgreveal.cc | 42 +++++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/src/gui/dlefgreveal.cc b/src/gui/dlefgreveal.cc index 4e2c62488..bb94e85e4 100644 --- a/src/gui/dlefgreveal.cc +++ b/src/gui/dlefgreveal.cc @@ -28,18 +28,22 @@ #include "gambit.h" #include "dlefgreveal.h" -namespace Gambit::GUI { +namespace { -class RevealMoveDialog final : public wxDialog { - GameDocument *m_doc{nullptr}; +using namespace Gambit; +using namespace Gambit::GUI; +class RevealMoveDialog final : public wxDialog { struct PlayerEntry { GamePlayer player; wxCheckBox *checkbox; }; - + GameDocument *m_doc{nullptr}; std::vector m_entries; + void OnCheckbox(wxCommandEvent &) { UpdateButtonState(); } + void UpdateButtonState(); + public: RevealMoveDialog(wxWindow *p_parent, GameDocument *p_doc); std::vector GetPlayers() const; @@ -52,7 +56,14 @@ RevealMoveDialog::RevealMoveDialog(wxWindow *p_parent, GameDocument *p_doc) { auto *topSizer = new wxBoxSizer(wxVERTICAL); - auto *playerBox = new wxStaticBoxSizer(wxVERTICAL, this, _("Reveal the move to players")); + auto *groupLabel = new wxStaticText(this, wxID_ANY, _("Reveal this move to players")); + auto f = groupLabel->GetFont(); + f.SetWeight(wxFONTWEIGHT_BOLD); + groupLabel->SetFont(f); + topSizer->Add(groupLabel, wxSizerFlags().Border(wxLEFT | wxTOP | wxRIGHT, 10)); + + auto *playerBox = new wxBoxSizer(wxVERTICAL); + playerBox->AddSpacer(3); const auto &players = m_doc->GetGame()->GetPlayers(); m_entries.reserve(players.size()); @@ -68,18 +79,28 @@ RevealMoveDialog::RevealMoveDialog(wxWindow *p_parent, GameDocument *p_doc) auto *cb = new wxCheckBox(this, wxID_ANY, label); cb->SetValue(true); cb->SetForegroundColour(m_doc->GetStyle().GetPlayerColor(player)); - + cb->Bind(wxEVT_CHECKBOX, &RevealMoveDialog::OnCheckbox, this); m_entries.push_back({player, cb}); - playerBox->Add(cb, wxSizerFlags().Expand().Border(wxALL, 2)); + playerBox->Add(cb, wxSizerFlags().Expand().Border(wxLEFT | wxRIGHT | wxTOP, 4)); } - topSizer->Add(playerBox, wxSizerFlags(1).Expand().Border()); + topSizer->Add(playerBox, wxSizerFlags(1).Expand().Border(wxALL, 5)); auto *buttonSizer = CreateStdDialogButtonSizer(wxOK | wxCANCEL); - topSizer->Add(buttonSizer, wxSizerFlags().Right().Border()); + buttonSizer->Realize(); + topSizer->Add(buttonSizer, wxSizerFlags().Right().Border(wxALL, 10)); SetSizerAndFit(topSizer); CenterOnParent(); + UpdateButtonState(); +} + +void RevealMoveDialog::UpdateButtonState() +{ + const bool anyChecked = + std::any_of(m_entries.begin(), m_entries.end(), + [](const PlayerEntry &entry) { return entry.checkbox->IsChecked(); }); + FindWindow(wxID_OK)->Enable(anyChecked); } std::vector RevealMoveDialog::GetPlayers() const @@ -93,6 +114,9 @@ std::vector RevealMoveDialog::GetPlayers() const } return result; } +} // anonymous namespace + +namespace Gambit::GUI { std::optional> RevealMove(wxWindow *p_parent, GameDocument *p_doc) { From 41804cb4e0e073238d6cdb727377939a32283a41 Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Tue, 9 Dec 2025 10:55:19 +0000 Subject: [PATCH 4/6] Remove trivial dialog header --- Makefile.am | 1 - src/gui/dlefgreveal.cc | 2 +- src/gui/dlefgreveal.h | 34 ---------------------------------- src/gui/gameframe.cc | 3 ++- 4 files changed, 3 insertions(+), 37 deletions(-) delete mode 100644 src/gui/dlefgreveal.h diff --git a/Makefile.am b/Makefile.am index 95b2ed961..654344066 100644 --- a/Makefile.am +++ b/Makefile.am @@ -568,7 +568,6 @@ gambit_SOURCES = \ src/gui/dlefglogit.cc \ src/gui/dlefglogit.h \ src/gui/dlefgreveal.cc \ - src/gui/dlefgreveal.h \ src/gui/dlexcept.h \ src/gui/dlgameprop.cc \ src/gui/dlgameprop.h \ diff --git a/src/gui/dlefgreveal.cc b/src/gui/dlefgreveal.cc index bb94e85e4..4439e0fe8 100644 --- a/src/gui/dlefgreveal.cc +++ b/src/gui/dlefgreveal.cc @@ -26,7 +26,7 @@ #endif // WX_PRECOMP #include "gambit.h" -#include "dlefgreveal.h" +#include "gamedoc.h" namespace { diff --git a/src/gui/dlefgreveal.h b/src/gui/dlefgreveal.h deleted file mode 100644 index 54babb593..000000000 --- a/src/gui/dlefgreveal.h +++ /dev/null @@ -1,34 +0,0 @@ -// -// This file is part of Gambit -// Copyright (c) 1994-2025, The Gambit Project (https://www.gambit-project.org) -// -// FILE: src/gui/dlefgreveal.h -// Dialog for revealing actions to players -// -// This program is free software; you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation; either version 2 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program; if not, write to the Free Software -// Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. -// - -#ifndef GAMBIT_GUI_DLEFGREVEAL_H -#define GAMBIT_GUI_DLEFGREVEAL_H - -#include "gamedoc.h" - -namespace Gambit::GUI { - -std::optional> RevealMove(wxWindow *p_parent, GameDocument *p_doc); - -} // end namespace Gambit::GUI - -#endif // GAMBIT_GUI_DLEFGREVEAL_H diff --git a/src/gui/gameframe.cc b/src/gui/gameframe.cc index 7870805e8..0142a7498 100644 --- a/src/gui/gameframe.cc +++ b/src/gui/gameframe.cc @@ -54,7 +54,6 @@ #include "dlabout.h" #include "dlinsertmove.h" -#include "dlefgreveal.h" #include "dleditnode.h" #include "dleditmove.h" #include "dlefglayout.h" @@ -978,6 +977,8 @@ void GameFrame::OnEditRemoveOutcome(wxCommandEvent &) } } +std::optional> RevealMove(wxWindow *p_parent, GameDocument *p_doc); + void GameFrame::OnEditReveal(wxCommandEvent &) { if (const auto players = RevealMove(this, m_doc); players) { From aa3653efc9b3f7d61975bdaae98b5d5f6a88a566 Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Tue, 9 Dec 2025 12:45:05 +0000 Subject: [PATCH 5/6] Remove dependency on game document. --- src/gui/dlefgreveal.cc | 16 ++++++---------- src/gui/gameframe.cc | 4 ++-- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/gui/dlefgreveal.cc b/src/gui/dlefgreveal.cc index 4439e0fe8..c0284d10b 100644 --- a/src/gui/dlefgreveal.cc +++ b/src/gui/dlefgreveal.cc @@ -38,21 +38,19 @@ class RevealMoveDialog final : public wxDialog { GamePlayer player; wxCheckBox *checkbox; }; - GameDocument *m_doc{nullptr}; std::vector m_entries; void OnCheckbox(wxCommandEvent &) { UpdateButtonState(); } void UpdateButtonState(); public: - RevealMoveDialog(wxWindow *p_parent, GameDocument *p_doc); + RevealMoveDialog(wxWindow *p_parent, const Game &p_game); std::vector GetPlayers() const; }; -RevealMoveDialog::RevealMoveDialog(wxWindow *p_parent, GameDocument *p_doc) +RevealMoveDialog::RevealMoveDialog(wxWindow *p_parent, const Game &p_game) : wxDialog(p_parent, wxID_ANY, _("Reveal this move to players"), wxDefaultPosition, - wxDefaultSize, wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER), - m_doc(p_doc) + wxDefaultSize, wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER) { auto *topSizer = new wxBoxSizer(wxVERTICAL); @@ -65,7 +63,7 @@ RevealMoveDialog::RevealMoveDialog(wxWindow *p_parent, GameDocument *p_doc) auto *playerBox = new wxBoxSizer(wxVERTICAL); playerBox->AddSpacer(3); - const auto &players = m_doc->GetGame()->GetPlayers(); + const auto &players = p_game->GetPlayers(); m_entries.reserve(players.size()); for (const auto &player : players) { @@ -78,7 +76,6 @@ RevealMoveDialog::RevealMoveDialog(wxWindow *p_parent, GameDocument *p_doc) } auto *cb = new wxCheckBox(this, wxID_ANY, label); cb->SetValue(true); - cb->SetForegroundColour(m_doc->GetStyle().GetPlayerColor(player)); cb->Bind(wxEVT_CHECKBOX, &RevealMoveDialog::OnCheckbox, this); m_entries.push_back({player, cb}); playerBox->Add(cb, wxSizerFlags().Expand().Border(wxLEFT | wxRIGHT | wxTOP, 4)); @@ -118,10 +115,9 @@ std::vector RevealMoveDialog::GetPlayers() const namespace Gambit::GUI { -std::optional> RevealMove(wxWindow *p_parent, GameDocument *p_doc) +std::optional> RevealMove(wxWindow *p_parent, const Game &p_game) { - RevealMoveDialog dialog(p_parent, p_doc); - if (dialog.ShowModal() == wxID_OK) { + if (RevealMoveDialog dialog(p_parent, p_game); dialog.ShowModal() == wxID_OK) { return dialog.GetPlayers(); } return std::nullopt; diff --git a/src/gui/gameframe.cc b/src/gui/gameframe.cc index 0142a7498..c26b5a303 100644 --- a/src/gui/gameframe.cc +++ b/src/gui/gameframe.cc @@ -977,11 +977,11 @@ void GameFrame::OnEditRemoveOutcome(wxCommandEvent &) } } -std::optional> RevealMove(wxWindow *p_parent, GameDocument *p_doc); +std::optional> RevealMove(wxWindow *p_parent, const Game &p_game); void GameFrame::OnEditReveal(wxCommandEvent &) { - if (const auto players = RevealMove(this, m_doc); players) { + if (const auto players = RevealMove(this, m_doc->GetGame()); players) { try { const auto &infoset = m_doc->GetSelectNode()->GetInfoset(); for (const auto &player : *players) { From 44b322fdd09e924fe275a4f547f52e3d42ee8ccf Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Tue, 9 Dec 2025 13:48:45 +0000 Subject: [PATCH 6/6] Remember to include optional --- src/gui/dlefgreveal.cc | 2 ++ src/gui/gameframe.cc | 1 + 2 files changed, 3 insertions(+) diff --git a/src/gui/dlefgreveal.cc b/src/gui/dlefgreveal.cc index c0284d10b..0844b43f8 100644 --- a/src/gui/dlefgreveal.cc +++ b/src/gui/dlefgreveal.cc @@ -20,6 +20,8 @@ // Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. // +#include + #include #ifndef WX_PRECOMP #include diff --git a/src/gui/gameframe.cc b/src/gui/gameframe.cc index c26b5a303..5d9d9e1b1 100644 --- a/src/gui/gameframe.cc +++ b/src/gui/gameframe.cc @@ -21,6 +21,7 @@ // #include +#include #include #ifndef WX_PRECOMP