Define and enforce a valid-label criterion for game objects (#944)

ChangeLog

@@ -39,6 +39,8 @@
 - Payoff editing in extensive games in the graphical interface is now done via a context popup window
   rather than text controls drawn (not always well!) over the game tree display.  (#947)
 - In `pygambit`, indexing game object collections by integer position has been removed.  (#942)
+- Validity of game object labels is enforced (printable ASCII and spaces only, no leading/trailing or
+  double spaces); invalid labels raise `ValueError` in `pygambit`. (#944)
 
 ### Removed
 - Built-in plotting of logit QRE for strategic games has been removed in the GUI (#809)

src/games/game.h

@@ -111,6 +111,58 @@ class InvalidFileException : public std::runtime_error {
   ~InvalidFileException() noexcept override = default;
 };
 
+//=======================================================================
+//                       Validation of labels
+//=======================================================================
+
+/// @brief Returns whether p_label is a valid label for a game object.
+///
+/// A valid label either is the empty string (denoting the absence of a label),
+/// or consists only of printable ASCII characters and spaces, begins and ends
+/// with a printable character, and contains no two consecutive spaces.
+///
+/// @note The set of valid labels is intended to be widened to permit Unicode in
+///       a future version; this function is the single point at which the
+///       definition is enforced.
+inline bool IsValidLabel(const std::string &p_label)
+{
+  if (p_label.empty()) {
+    return true;
+  }
+  auto is_printable = [](unsigned char c) { return c >= 0x21 && c <= 0x7e; };
+  if (!is_printable(p_label.front()) || !is_printable(p_label.back())) {
+    return false;
+  }
+  bool previous_was_space = false;
+  for (const char ch : p_label) {
+    const auto c = static_cast<unsigned char>(ch);
+    if (c == ' ') {
+      if (previous_was_space) {
+        return false; // two consecutive spaces
+      }
+      previous_was_space = true;
+    }
+    else if (is_printable(c)) {
+      previous_was_space = false;
+    }
+    else {
+      return false; // tab, newline, other control, or non-ASCII byte
+    }
+  }
+  return true;
+}
+
+/// @brief Throws ValueException if p_label is not a valid label.
+/// @sa IsValidLabel
+inline void CheckLabel(const std::string &p_label)
+{
+  if (!IsValidLabel(p_label)) {
+    throw ValueException("Invalid label: a label may contain only printable ASCII "
+                         "characters and spaces, must not begin or end with a space, "
+                         "and must not contain two consecutive spaces");
+  }
+}
+
 //=======================================================================
 //             Classes representing objects in a game
 //=======================================================================
@@ -149,7 +201,11 @@ class GameOutcomeRep : public std::enable_shared_from_this<GameOutcomeRep> {
   /// Returns the text label associated with the outcome
   const std::string &GetLabel() const { return m_label; }
   /// Sets the text label associated with the outcome
-  void SetLabel(const std::string &p_label) { m_label = p_label; }
+  void SetLabel(const std::string &p_label)
+  {
+    CheckLabel(p_label);
+    m_label = p_label;
+  }
 
   /// Gets the payoff associated with the outcome to the player
   template <class T> const T &GetPayoff(const GamePlayer &p_player) const;
@@ -184,7 +240,11 @@ class GameActionRep : public std::enable_shared_from_this<GameActionRep> {
   GameInfoset GetInfoset() const;
 
   const std::string &GetLabel() const { return m_label; }
-  void SetLabel(const std::string &p_label) { m_label = p_label; }
+  void SetLabel(const std::string &p_label)
+  {
+    CheckLabel(p_label);
+    m_label = p_label;
+  }
 
   bool Precedes(const GameNode &) const;
 };
@@ -229,7 +289,11 @@ class GameInfosetRep : public std::enable_shared_from_this<GameInfosetRep> {
 
   bool IsChanceInfoset() const;
 
-  void SetLabel(const std::string &p_label) { m_label = p_label; }
+  void SetLabel(const std::string &p_label)
+  {
+    CheckLabel(p_label);
+    m_label = p_label;
+  }
   const std::string &GetLabel() const { return m_label; }
 
   /// @name Actions
@@ -297,6 +361,7 @@ class GameStrategyRep : public std::enable_shared_from_this<GameStrategyRep> {
   explicit GameStrategyRep(GamePlayerRep *p_player, int p_number, const std::string &p_label)
     : m_player(p_player), m_number(p_number), m_label(p_label)
   {
+    CheckLabel(p_label);
   }
   //@}
 
@@ -308,7 +373,11 @@ class GameStrategyRep : public std::enable_shared_from_this<GameStrategyRep> {
   /// Returns the text label associated with the strategy
   const std::string &GetLabel() const { return m_label; }
   /// Sets the text label associated with the strategy
-  void SetLabel(const std::string &p_label) { m_label = p_label; }
+  void SetLabel(const std::string &p_label)
+  {
+    CheckLabel(p_label);
+    m_label = p_label;
+  }
 
   /// Returns the game on which the strategy is defined
   Game GetGame() const;
@@ -406,7 +475,11 @@ class GamePlayerRep : public std::enable_shared_from_this<GamePlayerRep> {
   Game GetGame() const;
 
   const std::string &GetLabel() const { return m_label; }
-  void SetLabel(const std::string &p_label) { m_label = p_label; }
+  void SetLabel(const std::string &p_label)
+  {
+    CheckLabel(p_label);
+    m_label = p_label;
+  }
 
   bool IsChance() const { return (m_number == 0); }
 
@@ -481,7 +554,11 @@ class GameNodeRep : public std::enable_shared_from_this<GameNodeRep> {
   Game GetGame() const;
 
   const std::string &GetLabel() const { return m_label; }
-  void SetLabel(const std::string &p_label) { m_label = p_label; }
+  void SetLabel(const std::string &p_label)
+  {
+    CheckLabel(p_label);
+    m_label = p_label;
+  }
 
   int GetNumber() const;
   GameNode GetChild(const GameAction &p_action)

src/gui/efgpanel.cc

@@ -32,6 +32,8 @@
 #include <wx/dcsvg.h>    // for SVG output
 
 #include "efgpanel.h"
+
+#include "dlexcept.h"
 #include "efgdisplay.h" // FIXME: communicate with tree window via events.
 #include "menuconst.h"
 #include "edittext.h"
@@ -282,14 +284,24 @@ void gbtTreePlayerPanel::OnEditPlayerLabel(wxCommandEvent &)
 
 void gbtTreePlayerPanel::OnAcceptPlayerLabel(wxCommandEvent &)
 {
-  m_doc->DoSetPlayerLabel(m_doc->GetGame()->GetPlayer(m_player), m_playerLabel->GetValue());
+  try {
+    m_doc->DoSetPlayerLabel(m_doc->GetGame()->GetPlayer(m_player), m_playerLabel->GetValue());
+  }
+  catch (std::exception &ex) {
+    ExceptionDialog(this, ex.what()).ShowModal();
+  }
 }
 
 void gbtTreePlayerPanel::PostPendingChanges()
 {
   if (m_playerLabel->IsEditing()) {
     m_playerLabel->EndEdit(true);
-    m_doc->DoSetPlayerLabel(m_doc->GetGame()->GetPlayer(m_player), m_playerLabel->GetValue());
+    try {
+      m_doc->DoSetPlayerLabel(m_doc->GetGame()->GetPlayer(m_player), m_playerLabel->GetValue());
+    }
+    catch (std::exception &ex) {
+      ExceptionDialog(this, ex.what()).ShowModal();
+    }
   }
 }
 

src/gui/nfgpanel.cc

@@ -29,6 +29,8 @@
 
 #include "gamedoc.h"
 #include "nfgpanel.h"
+
+#include "dlexcept.h"
 #include "nfgtable.h"
 #include "menuconst.h"
 #include "edittext.h"
@@ -219,14 +221,24 @@ void TablePlayerPanel::OnEditPlayerLabel(wxCommandEvent &)
 
 void TablePlayerPanel::OnAcceptPlayerLabel(wxCommandEvent &)
 {
-  m_doc->DoSetPlayerLabel(m_doc->GetGame()->GetPlayer(m_player), m_playerLabel->GetValue());
+  try {
+    m_doc->DoSetPlayerLabel(m_doc->GetGame()->GetPlayer(m_player), m_playerLabel->GetValue());
+  }
+  catch (std::exception &ex) {
+    ExceptionDialog(this, ex.what()).ShowModal();
+  }
 }
 
 void TablePlayerPanel::PostPendingChanges()
 {
   if (m_playerLabel->IsEditing()) {
     m_playerLabel->EndEdit(true);
-    m_doc->DoSetPlayerLabel(m_doc->GetGame()->GetPlayer(m_player), m_playerLabel->GetValue());
+    try {
+      m_doc->DoSetPlayerLabel(m_doc->GetGame()->GetPlayer(m_player), m_playerLabel->GetValue());
+    }
+    catch (std::exception &ex) {
+      ExceptionDialog(this, ex.what()).ShowModal();
+    }
   }
 }
 

src/gui/nfgtable.cc

@@ -1332,15 +1332,25 @@ void TableWidget::RenameRowHeaderStrategy(int headerCol, int headerRow, const wx
   const int player = GetRowHeaderPlayer(headerCol);
   const int strat = GetRowHeaderStrategy(headerCol, headerRow);
 
-  m_doc->DoSetStrategyLabel(GetStrategyByPlayerAndIndex(player, strat), value);
+  try {
+    m_doc->DoSetStrategyLabel(GetStrategyByPlayerAndIndex(player, strat), value);
+  }
+  catch (std::exception &ex) {
+    ExceptionDialog(this, ex.what()).ShowModal();
+  }
 }
 
 void TableWidget::RenameColHeaderStrategy(int headerRow, int headerCol, const wxString &value)
 {
   const int player = GetColHeaderPlayer(headerRow);
   const int strat = GetColHeaderStrategy(headerRow, headerCol);
 
-  m_doc->DoSetStrategyLabel(GetStrategyByPlayerAndIndex(player, strat), value);
+  try {
+    m_doc->DoSetStrategyLabel(GetStrategyByPlayerAndIndex(player, strat), value);
+  }
+  catch (std::exception &ex) {
+    ExceptionDialog(this, ex.what()).ShowModal();
+  }
 }
 
 void TableWidget::DeleteRowHeaderStrategy(int headerCol, int headerRow)

src/pygambit/action.pxi

@@ -72,7 +72,12 @@ class Action:
 
     @property
     def label(self) -> str:
-        """Get or set the text label of the action."""
+        """Get or set the text label of the action.
+
+        .. versionchanged:: 16.7.0
+            An invalid label now raises ``ValueError``: a label may contain only printable ASCII
+            characters and spaces, not begin/end with a space, nor have two consecutive spaces.
+        """
         return self.action.deref().GetLabel().decode("ascii")
 
     @label.setter

src/pygambit/gambit.pxd

@@ -106,7 +106,7 @@ cdef extern from "games/game.h":
         int GetId() except +
         c_GamePlayer GetPlayer() except +
         string GetLabel() except +
-        void SetLabel(string) except +
+        void SetLabel(string) except +ValueError
         c_GameAction GetAction(c_GameInfoset) except +
 
     cdef cppclass c_GameSequenceRep "GameSequenceRep":
@@ -120,7 +120,7 @@ cdef extern from "games/game.h":
         bint Precedes(c_GameNode) except +
 
         string GetLabel() except +
-        void SetLabel(string) except +
+        void SetLabel(string) except +ValueError
 
     cdef cppclass c_GameInfosetRep "GameInfosetRep":
         cppclass Actions:
@@ -148,7 +148,7 @@ cdef extern from "games/game.h":
         c_GamePlayer GetPlayer() except +
 
         string GetLabel() except +
-        void SetLabel(string) except +
+        void SetLabel(string) except +ValueError
 
         c_GameAction GetAction(int) except +IndexError
         Actions GetActions() except +
@@ -197,7 +197,7 @@ cdef extern from "games/game.h":
         int IsChance() except +
 
         string GetLabel() except +
-        void SetLabel(string) except +
+        void SetLabel(string) except +ValueError
 
         c_GameStrategy GetStrategy(int) except +IndexError
         Strategies GetStrategies() except +
@@ -212,7 +212,7 @@ cdef extern from "games/game.h":
         int GetNumber() except +
 
         string GetLabel() except +
-        void SetLabel(string) except +
+        void SetLabel(string) except +ValueError
 
         T GetPayoff[T](c_GamePlayer) except +IndexError
         void SetPayoff(c_GamePlayer, c_Number) except +IndexError
@@ -232,7 +232,7 @@ cdef extern from "games/game.h":
         int GetNumber() except +
 
         string GetLabel() except +
-        void SetLabel(string) except +
+        void SetLabel(string) except +ValueError
 
         c_GameInfoset GetInfoset() except +
         c_GamePlayer GetPlayer() except +
@@ -333,7 +333,7 @@ cdef extern from "games/game.h":
         Nodes GetNodes() except +
 
         c_GameStrategy GetStrategy(int) except +IndexError
-        c_GameStrategy NewStrategy(c_GamePlayer, string) except +
+        c_GameStrategy NewStrategy(c_GamePlayer, string) except +ValueError
         void DeleteStrategy(c_GameStrategy) except +
         int MixedProfileLength() except +
 

src/pygambit/game.pxi

@@ -2210,9 +2210,9 @@ class Game:
             )
         resolved_player = cython.cast(Player,
                                       self._resolve_player(player, "add_strategy"))
+        label_bytes = (str(label) if label is not None else "").encode("ascii")
         return Strategy.wrap(
-            self.game.deref().NewStrategy(resolved_player.player,
-                                          (str(label) if label is not None else "").encode())
+            self.game.deref().NewStrategy(resolved_player.player, label_bytes)
         )
 
     def delete_strategy(self, strategy: Strategy | str) -> None:

src/pygambit/infoset.pxi

@@ -165,7 +165,12 @@ class Infoset:
 
     @property
     def label(self) -> str:
-        """Get or set the text label of the information set."""
+        """Get or set the text label of the information set.
+
+        .. versionchanged:: 16.7.0
+            An invalid label now raises ``ValueError``: a label may contain only printable ASCII
+            characters and spaces, not begin/end with a space, nor have two consecutive spaces.
+        """
         return self.infoset.deref().GetLabel().decode("ascii")
 
     @label.setter

src/pygambit/node.pxi

@@ -135,7 +135,12 @@ class Node:
 
     @property
     def label(self) -> str:
-        """The text label associated with the node."""
+        """The text label associated with the node.
+
+        .. versionchanged:: 16.7.0
+            An invalid label now raises ``ValueError``: a label may contain only printable ASCII
+            characters and spaces, not begin/end with a space, nor have two consecutive spaces.
+        """
         return self.node.deref().GetLabel().decode("ascii")
 
     @label.setter

src/pygambit/outcome.pxi

@@ -63,7 +63,12 @@ class Outcome:
 
     @property
     def label(self) -> str:
-        """The text label associated with this outcome."""
+        """The text label associated with this outcome.
+
+        .. versionchanged:: 16.7.0
+            An invalid label now raises ``ValueError``: a label may contain only printable ASCII
+            characters and spaces, not begin/end with a space, nor have two consecutive spaces.
+        """
         return self.outcome.deref().GetLabel().decode("ascii")
 
     @label.setter

src/pygambit/player.pxi

@@ -247,7 +247,12 @@ class Player:
 
     @property
     def label(self) -> str:
-        """Gets or sets the text label of the player."""
+        """Gets or sets the text label of the player.
+
+        .. versionchanged:: 16.7.0
+            An invalid label now raises ``ValueError``: a label may contain only printable ASCII
+            characters and spaces, not begin/end with a space, nor have two consecutive spaces.
+        """
         return self.player.deref().GetLabel().decode("ascii")
 
     @label.setter