From 68d73e7f2c753dceaf0af1850cf58a72a13312a3 Mon Sep 17 00:00:00 2001 From: Ted Turocy Date: Mon, 17 Nov 2025 14:37:59 +0000 Subject: [PATCH 1/3] Fix regression in GUI not properly numbering actions at a new information set. (#619) Due to a previous refactoring the GUI was not generating distinct labels at new information sets created by drag-and-drop of the player icon. Instead, the label "1" was assigned to all actions. This restores the correct behaviour. Internally, the action generation option has been refactored to be part of the game operation itself, in anticipation of future developments on the rules on action labels. Closes #618. --- ChangeLog | 2 ++ src/games/game.h | 6 ++++-- src/games/gametree.cc | 14 ++++++++++++-- src/games/gametree.h | 6 ++++-- src/gui/gamedoc.cc | 5 +---- 5 files changed, 23 insertions(+), 10 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9c7b10cc2..5af871f2d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -7,6 +7,8 @@ when changing the number of strategies in the game (#571) - Fixed improper shared pointer handling when writing a .nfg file based on a game in extensive form. +- Fixed a regression in the GUI in which unique action labels were not being generated when + adding a move via drag-and-drop of a player icon (#618) ## [16.3.2] - unreleased diff --git a/src/games/game.h b/src/games/game.h index 529dbc507..112b68d38 100644 --- a/src/games/game.h +++ b/src/games/game.h @@ -743,7 +743,8 @@ class GameRep : public std::enable_shared_from_this { { throw UndefinedException(); } - virtual GameInfoset AppendMove(GameNode p_node, GamePlayer p_player, int p_actions) + virtual GameInfoset AppendMove(GameNode p_node, GamePlayer p_player, int p_actions, + bool p_generateLabels = false) { throw UndefinedException(); } @@ -751,7 +752,8 @@ class GameRep : public std::enable_shared_from_this { { throw UndefinedException(); } - virtual GameInfoset InsertMove(GameNode p_node, GamePlayer p_player, int p_actions) + virtual GameInfoset InsertMove(GameNode p_node, GamePlayer p_player, int p_actions, + bool p_generateLabels = false) { throw UndefinedException(); } diff --git a/src/games/gametree.cc b/src/games/gametree.cc index de0c008a2..4fbebf5c5 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -569,7 +569,8 @@ GameInfoset GameTreeRep::LeaveInfoset(GameNode p_node) return node->m_infoset->shared_from_this(); } -GameInfoset GameTreeRep::AppendMove(GameNode p_node, GamePlayer p_player, int p_actions) +GameInfoset GameTreeRep::AppendMove(GameNode p_node, GamePlayer p_player, int p_actions, + bool p_generateLabels) { GameNodeRep *node = p_node.get(); if (p_actions <= 0 || !node->m_children.empty()) { @@ -583,6 +584,10 @@ GameInfoset GameTreeRep::AppendMove(GameNode p_node, GamePlayer p_player, int p_ auto newInfoset = std::make_shared(this, p_player->m_infosets.size() + 1, p_player.get(), p_actions); p_player->m_infosets.push_back(newInfoset); + if (p_generateLabels) { + std::for_each(newInfoset->m_actions.begin(), newInfoset->m_actions.end(), + [act = 1](const GameAction &a) mutable { a->SetLabel(std::to_string(act++)); }); + } return AppendMove(p_node, newInfoset); } @@ -609,7 +614,8 @@ GameInfoset GameTreeRep::AppendMove(GameNode p_node, GameInfoset p_infoset) return node->m_infoset->shared_from_this(); } -GameInfoset GameTreeRep::InsertMove(GameNode p_node, GamePlayer p_player, int p_actions) +GameInfoset GameTreeRep::InsertMove(GameNode p_node, GamePlayer p_player, int p_actions, + bool p_generateLabels) { if (p_actions <= 0) { throw UndefinedException(); @@ -622,6 +628,10 @@ GameInfoset GameTreeRep::InsertMove(GameNode p_node, GamePlayer p_player, int p_ auto newInfoset = std::make_shared(this, p_player->m_infosets.size() + 1, p_player.get(), p_actions); p_player->m_infosets.push_back(newInfoset); + if (p_generateLabels) { + std::for_each(newInfoset->m_actions.begin(), newInfoset->m_actions.end(), + [act = 1](const GameAction &a) mutable { a->SetLabel(std::to_string(act++)); }); + } return InsertMove(p_node, newInfoset); } diff --git a/src/games/gametree.h b/src/games/gametree.h index 085cd557b..8695bb3f3 100644 --- a/src/games/gametree.h +++ b/src/games/gametree.h @@ -120,9 +120,11 @@ class GameTreeRep : public GameExplicitRep { /// @name Modification //@{ - GameInfoset AppendMove(GameNode p_node, GamePlayer p_player, int p_actions) override; + GameInfoset AppendMove(GameNode p_node, GamePlayer p_player, int p_actions, + bool p_generateLabels = false) override; GameInfoset AppendMove(GameNode p_node, GameInfoset p_infoset) override; - GameInfoset InsertMove(GameNode p_node, GamePlayer p_player, int p_actions) override; + GameInfoset InsertMove(GameNode p_node, GamePlayer p_player, int p_actions, + bool p_generateLabels = false) override; GameInfoset InsertMove(GameNode p_node, GameInfoset p_infoset) override; void CopyTree(GameNode dest, GameNode src) override; void MoveTree(GameNode dest, GameNode src) override; diff --git a/src/gui/gamedoc.cc b/src/gui/gamedoc.cc index 326e5f0b9..2b17fa9fe 100644 --- a/src/gui/gamedoc.cc +++ b/src/gui/gamedoc.cc @@ -639,10 +639,7 @@ void gbtGameDocument::DoAppendMove(GameNode p_node, GameInfoset p_infoset) void gbtGameDocument::DoInsertMove(GameNode p_node, GamePlayer p_player, unsigned int p_actions) { - const GameInfoset infoset = m_game->InsertMove(p_node, p_player, p_actions); - auto actions = infoset->GetActions(); - std::for_each(actions.begin(), actions.end(), - [act = 1](const GameAction &a) mutable { a->SetLabel(std::to_string(act)); }); + m_game->InsertMove(p_node, p_player, p_actions, true); m_game->SortInfosets(); UpdateViews(GBT_DOC_MODIFIED_GAME); } From 9d28e17ac1719208490587842e9c7cbefe397d38 Mon Sep 17 00:00:00 2001 From: Ted Turocy Date: Thu, 4 Dec 2025 11:45:44 +0000 Subject: [PATCH 2/3] Correct null pointer dereference in setting a null outcome (#666) This fixes a regression introduced when moving to using `std::shared_ptr` to handle reference-counting for game objects. Fixes #625 and #647. --- ChangeLog | 2 ++ src/games/game.h | 5 ++++- src/games/gameobject.h | 20 ++++++++------------ src/games/gametree.cc | 8 ++++---- src/games/gametree.h | 2 +- tests/test_node.py | 9 ++++++++- 6 files changed, 27 insertions(+), 19 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5af871f2d..e319a0744 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,6 +9,8 @@ extensive form. - Fixed a regression in the GUI in which unique action labels were not being generated when adding a move via drag-and-drop of a player icon (#618) +- Fixed a regression generating null pointer dereference errors when setting the outcome of + a node to the null outcome (#625, #647) ## [16.3.2] - unreleased diff --git a/src/games/game.h b/src/games/game.h index 112b68d38..055fdceb7 100644 --- a/src/games/game.h +++ b/src/games/game.h @@ -775,7 +775,10 @@ class GameRep : public std::enable_shared_from_this { throw UndefinedException(); } virtual void DeleteAction(GameAction) { throw UndefinedException(); } - virtual void SetOutcome(GameNode, const GameOutcome &p_outcome) { throw UndefinedException(); } + virtual void SetOutcome(const GameNode &p_node, const GameOutcome &p_outcome) + { + throw UndefinedException(); + } /// @name Dimensions of the game //@{ diff --git a/src/games/gameobject.h b/src/games/gameobject.h index 2246caf02..9e9338cb7 100644 --- a/src/games/gameobject.h +++ b/src/games/gameobject.h @@ -96,14 +96,10 @@ template class GameObjectPtr { /// game element held by this object and throws an exception if the object is the /// null object, or is no longer valid (has been removed from the game) /// - /// @exception NullException if the object holds a reference to a null element /// @exception InvalidObjectException if the element referred to has been deleted from its game std::shared_ptr get_shared() const { - if (!m_rep) { - throw NullException(); - } - if (!m_rep->IsValid()) { + if (m_rep && !m_rep->IsValid()) { throw InvalidObjectException(); } return m_rep; @@ -129,16 +125,16 @@ template class GameObjectPtr { } bool operator==(const GameObjectPtr &r) const { return (m_rep == r.m_rep); } - bool operator==(const std::shared_ptr r) const { return (m_rep == r); } - bool operator==(const std::shared_ptr r) const { return (m_rep == r); } - bool operator==(const std::nullptr_t) const { return !bool(m_rep); } + bool operator==(const std::shared_ptr &r) const { return (m_rep == r); } + bool operator==(const std::shared_ptr &r) const { return (m_rep == r); } + bool operator==(const std::nullptr_t &) const { return m_rep == nullptr; } bool operator!=(const GameObjectPtr &r) const { return (m_rep != r.m_rep); } - bool operator!=(const std::shared_ptr r) const { return (m_rep != r); } - bool operator!=(const std::shared_ptr r) const { return (m_rep != r); } - bool operator!=(const std::nullptr_t) const { return bool(m_rep); } + bool operator!=(const std::shared_ptr &r) const { return (m_rep != r); } + bool operator!=(const std::shared_ptr &r) const { return (m_rep != r); } + bool operator!=(const std::nullptr_t &) const { return m_rep != nullptr; } bool operator<(const GameObjectPtr &r) const { return (m_rep < r.m_rep); } - operator bool() const noexcept { return bool(m_rep); } + operator bool() const noexcept { return m_rep != nullptr; } operator std::shared_ptr() const { return m_rep; } }; diff --git a/src/games/gametree.cc b/src/games/gametree.cc index 4fbebf5c5..518dc79cb 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -334,17 +334,17 @@ void GameNodeRep::DeleteOutcome(GameOutcomeRep *outc) } } -void GameTreeRep::SetOutcome(GameNode p_node, const GameOutcome &p_outcome) +void GameTreeRep::SetOutcome(const GameNode &p_node, const GameOutcome &p_outcome) { - IncrementVersion(); if (p_node->m_game != this) { throw MismatchException(); } if (p_outcome && p_outcome->m_game != this) { throw MismatchException(); } - if (p_outcome.get() != p_node->m_outcome) { - p_node->m_outcome = p_outcome.get(); + if (const auto newOutcome = p_outcome.get_shared().get(); newOutcome != p_node->m_outcome) { + p_node->m_outcome = newOutcome; + IncrementVersion(); ClearComputedValues(); } } diff --git a/src/games/gametree.h b/src/games/gametree.h index 8695bb3f3..703ce593e 100644 --- a/src/games/gametree.h +++ b/src/games/gametree.h @@ -137,7 +137,7 @@ class GameTreeRep : public GameExplicitRep { Game SetChanceProbs(const GameInfoset &, const Array &) override; GameAction InsertAction(GameInfoset, GameAction p_where = nullptr) override; void DeleteAction(GameAction) override; - void SetOutcome(GameNode, const GameOutcome &p_outcome) override; + void SetOutcome(const GameNode &p_node, const GameOutcome &p_outcome) override; std::vector GetPlays(GameNode node) const override; std::vector GetPlays(GameInfoset infoset) const override; diff --git a/tests/test_node.py b/tests/test_node.py index 4104e0e64..48c2740a9 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -19,10 +19,17 @@ def test_get_infoset(): def test_get_outcome(): """Test to ensure that we can retrieve an outcome for a given node""" game = games.read_from_file("basic_extensive_game.efg") - assert game.root.children[0].children[1].children[0].outcome == game.outcomes[1] + assert game.root.children[0].children[1].children[0].outcome == game.outcomes["Outcome 1"] assert game.root.outcome is None +def test_set_outcome_null(): + """Test to set an outcome to the null outcome.""" + game = games.read_from_file("basic_extensive_game.efg") + game.set_outcome(game.root.children[0].children[0].children[0], None) + assert game.root.children[0].children[0].children[0].outcome is None + + def test_get_player(): """Test to ensure that we can retrieve a player for a given node""" game = games.read_from_file("basic_extensive_game.efg") From 39e4286f9eb3cac92a7a7cc46d8e9d00c68d80a4 Mon Sep 17 00:00:00 2001 From: Ted Turocy Date: Thu, 4 Dec 2025 18:17:26 +0000 Subject: [PATCH 3/3] Fix caching in MixedStrategyProfile (#669) For mixed strategy profiles which are defined on trees, a behavior strategy profile is computed and cached. This was not correctly being cleared when changing the strategy probabilities, resulting in incorrect output. Closes #616. --- ChangeLog | 2 ++ src/games/stratmixed.h | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index e319a0744..da05cf682 100644 --- a/ChangeLog +++ b/ChangeLog @@ -11,6 +11,8 @@ adding a move via drag-and-drop of a player icon (#618) - Fixed a regression generating null pointer dereference errors when setting the outcome of a node to the null outcome (#625, #647) +- Fixed a regression in calculating payoff quantities for mixed strategy profiles derived from + mixed behavior profiles (#616) ## [16.3.2] - unreleased diff --git a/src/games/stratmixed.h b/src/games/stratmixed.h index 7c7b13e08..652b1062e 100644 --- a/src/games/stratmixed.h +++ b/src/games/stratmixed.h @@ -49,7 +49,11 @@ template class MixedStrategyProfileRep { return m_probs[m_profileIndex.at(p_strategy)]; } /// Returns the probability the strategy is played - T &operator[](const GameStrategy &p_strategy) { return m_probs[m_profileIndex.at(p_strategy)]; } + T &operator[](const GameStrategy &p_strategy) + { + InvalidateCache(); + return m_probs[m_profileIndex.at(p_strategy)]; + } /// Set the strategy of the corresponding player to a pure strategy void SetStrategy(const GameStrategy &p_strategy) {