Merge maint16_4 changes into master

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

src/games/game.h

@@ -743,15 +743,17 @@ class GameRep : public std::enable_shared_from_this<GameRep> {
   {
     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();
   }
   virtual GameInfoset AppendMove(GameNode p_node, GameInfoset p_infoset)
   {
     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<GameRep> {
     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
   //@{

src/games/gameobject.h

@@ -96,14 +96,10 @@ template <class T> 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<T> 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 T> class GameObjectPtr {
   }
 
   bool operator==(const GameObjectPtr<T> &r) const { return (m_rep == r.m_rep); }
-  bool operator==(const std::shared_ptr<T> r) const { return (m_rep == r); }
-  bool operator==(const std::shared_ptr<const T> r) const { return (m_rep == r); }
-  bool operator==(const std::nullptr_t) const { return !bool(m_rep); }
+  bool operator==(const std::shared_ptr<T> &r) const { return (m_rep == r); }
+  bool operator==(const std::shared_ptr<const T> &r) const { return (m_rep == r); }
+  bool operator==(const std::nullptr_t &) const { return m_rep == nullptr; }
   bool operator!=(const GameObjectPtr<T> &r) const { return (m_rep != r.m_rep); }
-  bool operator!=(const std::shared_ptr<T> r) const { return (m_rep != r); }
-  bool operator!=(const std::shared_ptr<const T> r) const { return (m_rep != r); }
-  bool operator!=(const std::nullptr_t) const { return bool(m_rep); }
+  bool operator!=(const std::shared_ptr<T> &r) const { return (m_rep != r); }
+  bool operator!=(const std::shared_ptr<const T> &r) const { return (m_rep != r); }
+  bool operator!=(const std::nullptr_t &) const { return m_rep != nullptr; }
   bool operator<(const GameObjectPtr<T> &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<T>() const { return m_rep; }
 };
 

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<GameInfosetRep>(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<GameInfosetRep>(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);
 }
 

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<Number> &) 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<GameNode> GetPlays(GameNode node) const override;
   std::vector<GameNode> GetPlays(GameInfoset infoset) const override;

src/games/stratmixed.h

@@ -49,7 +49,11 @@ template <class T> 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)
   {

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);
 }

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")