diff --git a/ChangeLog b/ChangeLog index 9c7b10cc2..da05cf682 100644 --- a/ChangeLog +++ b/ChangeLog @@ -7,6 +7,12 @@ 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) +- 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/game.h b/src/games/game.h index 529dbc507..055fdceb7 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(); } @@ -773,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 de0c008a2..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(); } } @@ -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..703ce593e 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; @@ -135,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/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) { 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); } 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")