From 9fee1ab752ce3b8bae410a8098547bbea3886fed Mon Sep 17 00:00:00 2001 From: drdkad Date: Thu, 20 Aug 2026 08:38:00 +0100 Subject: [PATCH 1/6] Add ignore set to CheckPlayerLabel --- src/games/game.h | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/games/game.h b/src/games/game.h index 0bbe161c96..85bbe62848 100644 --- a/src/games/game.h +++ b/src/games/game.h @@ -936,7 +936,9 @@ class GameRep : public std::enable_shared_from_this { void IncrementVersion() { m_version++; } void IndexStrategies() const; /// Validate that p_label is a nonempty, valid, unique label for a player of this game, - void CheckPlayerLabel(const std::string &p_label) const; + /// ignoring the labels of the players in p_ignore. + void CheckPlayerLabel(const std::string &p_label, + const std::set &p_ignore = {}) const; /// Validate that p_label is a nonempty, valid, unique label for an outcome of this game. void CheckOutcomeLabel(const std::string &p_label) const; //@} @@ -1357,6 +1359,8 @@ class GameRep : public std::enable_shared_from_this { auto GetPlayersWithChance() const { return prepend_value(GetChance(), GetPlayers()); } /// Creates a new player in the game, with no moves virtual GamePlayer NewPlayer(const std::string &p_label) = 0; + /// Reassign player labels. Keys of p_labels are current labels; values are their replacements. + void RelabelPlayers(const std::map &p_labels); //@} /// @name Dimensions of the game @@ -1591,7 +1595,8 @@ inline void GameInfosetRep::SetLabel(const std::string &p_label) m_player->CheckInfosetLabel(p_label, {this}); m_label = p_label; } -inline void GameRep::CheckPlayerLabel(const std::string &p_label) const +inline void GameRep::CheckPlayerLabel(const std::string &p_label, + const std::set &p_ignore) const { if (p_label.empty()) { throw ValueException("Player label must not be empty"); @@ -1601,7 +1606,7 @@ inline void GameRep::CheckPlayerLabel(const std::string &p_label) const throw ValueException("Player label must not be the reserved chance player label"); } for (const auto &player : m_players) { - if (player->GetLabel() == p_label) { + if (p_ignore.count(player.get()) == 0 && player->GetLabel() == p_label) { throw ValueException("Player label must be unique within the game"); } } From b46b5f2a646957aa8cc2669851fa003e6c0c9bbc Mon Sep 17 00:00:00 2001 From: drdkad Date: Thu, 20 Aug 2026 08:38:15 +0100 Subject: [PATCH 2/6] Add GameRep::RelabelPlayers --- src/games/game.cc | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/games/game.cc b/src/games/game.cc index 8ea3a1c422..9d72e81c01 100644 --- a/src/games/game.cc +++ b/src/games/game.cc @@ -233,6 +233,48 @@ void GameRep::WriteNfgFile(std::ostream &p_file) const }; } +//------------------------------------------------------------------------ +// GameRep: Players +//------------------------------------------------------------------------ + +void GameRep::RelabelPlayers(const std::map &p_labels) +{ + // Resolve each key to exactly one (personal) player of the game. + std::map assignment; + std::set relabeled; + for (const auto &[old_label, new_label] : p_labels) { + GamePlayerRep *match = nullptr; + for (const auto &player : m_players) { + if (player->GetLabel() == old_label) { + if (match) { + throw ValueException("Player label '" + old_label + "' is ambiguous in this game"); + } + match = player.get(); + } + } + if (!match) { + if (IsTree() && old_label == GetChance()->GetLabel()) { + throw ValueException("The chance player's label cannot be changed"); + } + throw ValueException("No player with label '" + old_label + "' in this game"); + } + assignment[match] = new_label; + relabeled.insert(match); + } + // Replacement labels must be legal, unique against untouched players, and pairwise distinct + std::set targets; + for (const auto &[player, new_label] : assignment) { + CheckPlayerLabel(new_label, relabeled); + if (!targets.insert(new_label).second) { + throw ValueException("Player label '" + new_label + + "' would be duplicated by the relabelling"); + } + } + for (const auto &[player, new_label] : assignment) { + player->m_label = new_label; + } +} + //======================================================================== // MixedStrategyProfileRep //======================================================================== From d7ed7849ef78e701bf01aca633bdfee8b71141f5 Mon Sep 17 00:00:00 2001 From: drdkad Date: Thu, 20 Aug 2026 08:39:11 +0100 Subject: [PATCH 3/6] Add Game.relabel_players; remove Player.label setter --- src/pygambit/gambit.pxd | 1 + src/pygambit/game.pxi | 74 +++++++++++++++++++++++++++++++++++++++-- src/pygambit/player.pxi | 9 +++-- 3 files changed, 77 insertions(+), 7 deletions(-) diff --git a/src/pygambit/gambit.pxd b/src/pygambit/gambit.pxd index 4bcda2bb7a..fba2538dd5 100644 --- a/src/pygambit/gambit.pxd +++ b/src/pygambit/gambit.pxd @@ -319,6 +319,7 @@ cdef extern from "games/game.h": Players GetPlayers() except + c_GamePlayer GetChance() except + c_GamePlayer NewPlayer(string) except +ValueError + void RelabelPlayers(stdmap[string, string]) except +ValueError int NumOutcomes() except + c_GameOutcome GetOutcome(int) except +IndexError diff --git a/src/pygambit/game.pxi b/src/pygambit/game.pxi index 3c4306f467..32de907a5e 100644 --- a/src/pygambit/game.pxi +++ b/src/pygambit/game.pxi @@ -707,8 +707,9 @@ class Game: arrays = list(payoffs.values()) shape = arrays[0].shape g = Game.new_table(shape) - for (player, label) in zip(g.players, payoffs, strict=True): - player.label = label + g.relabel_players( + {player.label: label for player, label in zip(g.players, payoffs, strict=True)} + ) for profile in itertools.product(*(range(s) for s in shape)): for array, player in zip(arrays, g.players, strict=True): g[profile][player] = array[profile] @@ -2798,6 +2799,75 @@ class Game: return self.game.deref().RelabelStrategies(resolved_player.player, c_labels) + def relabel_players(self, + labels: typing.Mapping[str, str], + strict: bool = True) -> None: + """Simultaneously reassign the labels of the game's players. + + `labels` maps current player labels to their replacements. The reassignment + is simultaneous, so labels can be swapped directly. Players are not re-ordered: each + relabelled player keeps its position. + + The chance player is not part of the operation: its label is reserved, and a + key of `labels` equal to it raises ``ValueError`` even when `strict` is `False`. + + .. versionadded:: 17.0.0 + + Parameters + ---------- + labels : Mapping[str, str] + A mapping from current player labels to replacement labels. + Entries whose key equals their value are ignored. + strict : bool, default True + If `True`, every key of `labels` must be the label of a player of the game, + and unknown keys raise ``KeyError``. If `False`, unknown keys are ignored. + + Raises + ------ + KeyError + When `strict` is `True`, if a key of `labels` matches no player of the game. + TypeError + If `labels` is not a mapping, or any key or value is not a string. + ValueError + If a key of `labels` matches more than one player; if a key of `labels` + is the label of the chance player; or if any replacement label is empty, + is not a valid label, would result in a duplicate label, or (in an + extensive game) is the reserved label of the chance player. + + See Also + -------- + relabel_actions : Change the labels of actions at an information set. + relabel_strategies : Change the labels of a player's strategies. + """ + if not hasattr(labels, "items"): + raise TypeError( + f"relabel_players(): labels must be a mapping, " + f"not {labels.__class__.__name__}" + ) + current = [player.label for player in self.players] + chance_label = self.players.chance.label if self.is_tree else None + c_labels = stdmap[string, string]() + for old, new in labels.items(): + if not isinstance(old, str) or not isinstance(new, str): + raise TypeError("relabel_players(): labels must map str to str") + if old == chance_label: + raise ValueError("relabel_players(): the chance player's label is reserved") + matches = current.count(old) + if matches > 1: + raise ValueError( + f"relabel_players(): label '{old}' is ambiguous in this game" + ) + if matches == 0: + if strict: + raise KeyError(f"relabel_players(): no player with label '{old}'") + continue + if new == old: + continue + c_labels[old.encode("utf-8")] = new.encode("utf-8") + if c_labels.empty(): + return + self.game.deref().RelabelPlayers(c_labels) + @dataclasses.dataclass class NodeCoordinates: diff --git a/src/pygambit/player.pxi b/src/pygambit/player.pxi index 7236ebe0dc..2916db8ed7 100644 --- a/src/pygambit/player.pxi +++ b/src/pygambit/player.pxi @@ -248,20 +248,19 @@ class Player: @property def label(self) -> str: - """Gets or sets the text label of the player. + """The text label of the player. .. versionchanged:: 17.0.0 A label may now be any well-formed UTF-8 text, not just ASCII; it must still contain no control characters, and must not begin/end with whitespace or have two consecutive whitespace characters. "Whitespace" means any Unicode space separator (e.g. U+00A0 NO-BREAK SPACE), not just the ASCII space. + + The label is now read-only, and must be nonempty and unique among the game's + players; use `Game.relabel_players` to change it. """ return self.player.deref().GetLabel().decode("utf-8") - @label.setter - def label(self, value: str) -> None: - self.player.deref().SetLabel(value.encode("utf-8")) - @property def number(self) -> int: """Returns the number of the player in its game. From ca44f7de72bc2db85b4c22a89d7359a2c3338452 Mon Sep 17 00:00:00 2001 From: drdkad Date: Thu, 20 Aug 2026 08:39:30 +0100 Subject: [PATCH 4/6] update tests / tutorials --- doc/tutorials/01_quickstart.ipynb | 9 ++++- tests/test_io.py | 6 +-- tests/test_outcomes.py | 3 +- tests/test_players.py | 66 ++++++++++++++++++++++++++----- 4 files changed, 68 insertions(+), 16 deletions(-) diff --git a/doc/tutorials/01_quickstart.ipynb b/doc/tutorials/01_quickstart.ipynb index c1ad89a233..b1fba454f6 100644 --- a/doc/tutorials/01_quickstart.ipynb +++ b/doc/tutorials/01_quickstart.ipynb @@ -76,7 +76,14 @@ "id": "9d8203e8", "metadata": {}, "outputs": [], - "source": "tom, jerry = g.players\ntom.label = \"Tom\"\njerry.label = \"Jerry\"\n\nfor player in g.players:\n cooperate, defect = player.strategies\n g.relabel_strategies(player, {cooperate.label: \"Cooperate\", defect.label: \"Defect\"})" + "source": [ + "tom, jerry = g.players\n", + "g.relabel_players({tom.label: \"Tom\", jerry.label: \"Jerry\"})\n", + "\n", + "for player in g.players:\n", + " cooperate, defect = player.strategies\n", + " g.relabel_strategies(player, {cooperate.label: \"Cooperate\", defect.label: \"Defect\"})" + ] }, { "cell_type": "markdown", diff --git a/tests/test_io.py b/tests/test_io.py index 984814e3e1..7a720a470a 100644 --- a/tests/test_io.py +++ b/tests/test_io.py @@ -160,8 +160,7 @@ def test_write_efg_as_nfg(): def test_write_html(): game = gbt.Game.new_table([2, 2]) alice, bob = game.players - alice.label = "Alice" - bob.label = "Bob" + game.relabel_players({alice.label: "Alice", bob.label: "Bob"}) serialized_game = game.to_html() assert isinstance(serialized_game, str) assert "Alice" in serialized_game @@ -171,8 +170,7 @@ def test_write_html(): def test_write_latex(): game = gbt.Game.new_table([2, 2], title="Game title") alice, bob = game.players - alice.label = "Alice" - bob.label = "Bob" + game.relabel_players({alice.label: "Alice", bob.label: "Bob"}) serialized_game = game.to_latex() assert "\\begin{game}" in serialized_game assert "[\\textbf{Alice}][\\textbf{Bob}]" in serialized_game diff --git a/tests/test_outcomes.py b/tests/test_outcomes.py index 65e5f7fa51..b3ebcf6313 100644 --- a/tests/test_outcomes.py +++ b/tests/test_outcomes.py @@ -84,8 +84,7 @@ def test_outcome_index_invalid_type(game: gbt.Game): def test_outcome_payoff_by_player_label(): game = gbt.Game.new_table([2, 2]) pl1, pl2 = list(game.players) - pl1.label = "joe" - pl2.label = "dan" + game.relabel_players({pl1.label: "joe", pl2.label: "dan"}) out1, out2, *_ = list(game.outcomes) out1["joe"] = 1 out1["dan"] = 2 diff --git a/tests/test_players.py b/tests/test_players.py index 0b58047238..a0eefb34df 100644 --- a/tests/test_players.py +++ b/tests/test_players.py @@ -14,7 +14,7 @@ def test_player_count(): def test_player_label(label): game = gbt.Game.new_table([2, 2]) player = next(iter(game.players)) - player.label = label + game.relabel_players({player.label: label}) assert player.label == label @@ -23,7 +23,7 @@ def test_player_label_invalid_raises_valueerror(label): game = gbt.Game.new_table([2, 2]) player = next(iter(game.players)) with pytest.raises(ValueError): - player.label = label + game.relabel_players({player.label: label}) @pytest.mark.parametrize("label", games.UNICODE_LABELS) @@ -31,7 +31,7 @@ def test_player_label_unicode_accepted(label): """Non-ASCII UTF-8 labels are accepted as of #862 (17.0).""" game = gbt.Game.new_table([2, 2]) player = next(iter(game.players)) - player.label = label + game.relabel_players({player.label: label}) assert player.label == label @@ -77,7 +77,7 @@ def test_chance_player_label_cannot_be_changed(): """The chance player's label is reserved ("Chance") and cannot be changed.""" game = gbt.Game.new_tree() with pytest.raises(ValueError): - game.players.chance.label = "Nature" + game.relabel_players({game.players.chance.label: "Nature"}) def test_regular_player_cannot_be_relabeled_to_chance(): @@ -85,14 +85,13 @@ def test_regular_player_cannot_be_relabeled_to_chance(): game.add_player("Alice") player = next(iter(game.players)) with pytest.raises(ValueError): - player.label = "Chance" + game.relabel_players({player.label: "Chance"}) def test_player_index_by_string(): game = gbt.Game.new_table([2, 2]) pl1, pl2 = game.players - pl1.label = "Alphonse" - pl2.label = "Gaston" + game.relabel_players({pl1.label: "Alphonse", pl2.label: "Gaston"}) assert game.players["Alphonse"].label == "Alphonse" assert game.players["Gaston"].label == "Gaston" @@ -113,14 +112,63 @@ def test_set_empty_player_raises_valueerror(): game = games.create_stripped_down_poker_efg() player = next(iter(game.players)) with pytest.raises(ValueError): - player.label = "" + game.relabel_players({player.label: ""}) def test_set_duplicate_player_raises_valueerror(): game = games.create_stripped_down_poker_efg() pl1, pl2, *_ = game.players with pytest.raises(ValueError): - pl1.label = pl2.label + game.relabel_players({pl1.label: pl2.label}) + + +def test_relabel_players_swap(): + game = gbt.Game.new_table([2, 2]) + a, b = (player.label for player in game.players) + game.relabel_players({a: b, b: a}) + assert [player.label for player in game.players] == [b, a] + + +def test_relabel_players_swap_tree(): + game = gbt.Game.new_tree(["Alice", "Bob"]) + game.relabel_players({"Alice": "Bob", "Bob": "Alice"}) + assert [player.label for player in game.players] == ["Bob", "Alice"] + + +def test_relabel_players_duplicate_raises_valueerror(): + game = gbt.Game.new_table([2, 2, 2]) + a, b, _ = (player.label for player in game.players) + with pytest.raises(ValueError): + game.relabel_players({a: b}) + with pytest.raises(ValueError): + game.relabel_players({a: "X", b: "X"}) + + +@pytest.mark.parametrize("bad", ["", " x"]) +def test_relabel_players_bad_label_raises_and_leaves_game_unchanged(bad: str): + game = gbt.Game.new_table([2, 2]) + a, b = (player.label for player in game.players) + with pytest.raises(ValueError): + game.relabel_players({a: "X", b: bad}) + assert [player.label for player in game.players] == [a, b] + + +def test_relabel_players_unknown_label_strictness(): + game = gbt.Game.new_table([2, 2]) + a = next(iter(game.players)).label + with pytest.raises(KeyError): + game.relabel_players({"no-such-player": "X"}) + game.relabel_players({"no-such-player": "X", a: "Y"}, strict=False) + assert next(iter(game.players)).label == "Y" + + +def test_relabel_players_chance_key_raises_even_when_not_strict(): + """The chance player's label is reserved; a key equal to it is an error.""" + game = gbt.Game.new_tree(["Alice"]) + with pytest.raises(ValueError): + game.relabel_players({"Chance": "Nature"}) + with pytest.raises(ValueError): + game.relabel_players({"Chance": "Nature"}, strict=False) def test_strategic_game_add_player(): From eeaca771b798fd47f8ed62221713bef014fd1a12 Mon Sep 17 00:00:00 2001 From: drdkad Date: Thu, 20 Aug 2026 08:39:50 +0100 Subject: [PATCH 5/6] update api.rst and changelog --- ChangeLog | 3 +++ doc/pygambit.api.rst | 1 + 2 files changed, 4 insertions(+) diff --git a/ChangeLog b/ChangeLog index fe2989fded..b146e0d594 100644 --- a/ChangeLog +++ b/ChangeLog @@ -5,6 +5,7 @@ - `gambit-ipa` now supports verbose output via the `-V` flag, reporting on intermediate progress. (#501) - Added `-R` option to `gambit-liap`, `gambit-simpdiv`, `gambit-ipa`, and `gambit-gnm` to reproducibly seed random starting point generation. (#448) +- Added `Game.relabel_players`, which simultaneously reassigns the labels of the game's players. (#1058) ### Fixed - `Game.add_player`, on a game in strategic form, no longer leaves the new player's sole @@ -24,6 +25,8 @@ ### Removed - Assigning to `Strategy.label` has been removed; use `Game.relabel_strategies`, which enforces nonempty, unique labels. (#1047) +- Assigning to `Player.label` has been removed; use `Game.relabel_players`, which enforces + nonempty, unique labels. (#1058) ## [17.0.0-alpha.1] - 2026-08-13 diff --git a/doc/pygambit.api.rst b/doc/pygambit.api.rst index 31ba32ca9a..7735953718 100644 --- a/doc/pygambit.api.rst +++ b/doc/pygambit.api.rst @@ -85,6 +85,7 @@ Transforming game components :toctree: api/ Game.add_player + Game.relabel_players Game.add_outcome Game.delete_outcome Game.set_outcome From d46d256b7d059c3c0ff45d4c81e3db73183192fc Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Thu, 20 Aug 2026 14:32:58 +0100 Subject: [PATCH 6/6] Retire GamePlayerRep::SetLabel; route all player relabeling through RelabelPlayers Mirrors the treatment GameActionRep::SetLabel and GameStrategyRep::SetLabel already got (#1031, #1047): with Game.relabel_players now the sole public way to rename a player, the underlying per-object setter is no longer needed. The GUI's Game Properties dialog previously renamed players one at a time via DoSetPlayerLabel, which could fail to swap two players' labels: renaming the first collided with the second's still-current label, even though the dialog's own validation (checking the proposed final state) saw nothing wrong. DoSetPlayerLabel is replaced with DoRelabelPlayers, which builds a single old-label -> new-label map from all changed rows and applies it in one RelabelPlayers call, the same batching DoSetStrategies already uses for strategies. NewTableDocument's default "Player N" labeling is batched the same way. With SetLabel gone, the two remaining internal callers in file.cc are updated: BuildNfg's RelabelWithoutCollision scratch-label trick for parsed player labels is replaced by a direct RelabelPlayers call (RelabelPlayers's simultaneous resolution makes the scratch pass unnecessary, so RelabelWithoutCollision is deleted with no callers left), and NormalizeGameLabels's generic per-element setter is given a RelabelPlayers-based lambda for the player branch, leaving the outcome branch untouched. Co-Authored-By: Claude Sonnet 5 --- src/games/file.cc | 30 ++++++++++-------------------- src/games/game.h | 12 ------------ src/gui/dlgameprop.cc | 4 ++-- src/gui/gamedoc.cc | 4 ++-- src/gui/gamedoc.h | 9 ++++++--- src/gui/gameframe.cc | 12 +++++++++++- src/pygambit/gambit.pxd | 1 - 7 files changed, 31 insertions(+), 41 deletions(-) diff --git a/src/games/file.cc b/src/games/file.cc index 750ee23624..b0d48c61c4 100644 --- a/src/games/file.cc +++ b/src/games/file.cc @@ -375,24 +375,6 @@ template void NormalizeLabelStrings(Container &p_labels) [](std::string &s, const std::string &v) { s = v; }); } -template -void RelabelWithoutCollision(Container &&p_container, const std::vector &p_labels) -{ - const std::set forbidden(p_labels.begin(), p_labels.end()); - size_t scratchIndex = 0; - for (auto &&element : p_container) { - std::string candidate; - do { - candidate = "_gambit_reader_scratch_" + std::to_string(scratchIndex++); - } while (forbidden.contains(candidate)); - element->SetLabel(candidate); - } - size_t index = 0; - for (auto &&element : p_container) { - element->SetLabel(p_labels[index++]); - } -} - void ReadPlayers(GameFileLexer &p_state, TableFileGame &p_data) { p_state.ExpectNextToken(TOKEN_LBRACE, "'{'"); @@ -538,7 +520,12 @@ Game BuildNfg(GameFileLexer &p_parser, TableFileGame &p_data) playerLabels.push_back(p_data.GetPlayer(player->GetNumber())); } NormalizeLabelStrings(playerLabels); - RelabelWithoutCollision(nfg->GetPlayers(), playerLabels); + std::map playerRelabels; + size_t playerIndex = 0; + for (auto player : nfg->GetPlayers()) { + playerRelabels[player->GetLabel()] = playerLabels[playerIndex++]; + } + nfg->RelabelPlayers(playerRelabels); for (auto player : nfg->GetPlayers()) { std::vector strategyLabels; @@ -907,7 +894,10 @@ void NormalizeGameLabels(const Game &p_game) { const auto get_label = [](const auto &e) { return e->GetLabel(); }; const auto set_label = [](const auto &e, const std::string &s) { e->SetLabel(s); }; - NormalizeLabels(p_game->GetPlayers(), get_label, set_label); + const auto relabel_player = [&p_game](const auto &e, const std::string &s) { + p_game->RelabelPlayers({{e->GetLabel(), s}}); + }; + NormalizeLabels(p_game->GetPlayers(), get_label, relabel_player); NormalizeLabels(p_game->GetOutcomes(), get_label, set_label); // Action labels are not normalized here: for tree games, ParseNode/ParsePersonalNode // already normalize each infoset's actions individually, at creation, from the raw diff --git a/src/games/game.h b/src/games/game.h index eaf64bcbdf..a0931bac3d 100644 --- a/src/games/game.h +++ b/src/games/game.h @@ -625,7 +625,6 @@ class GamePlayerRep : public std::enable_shared_from_this { Game GetGame() const; const std::string &GetLabel() const { return m_label; } - void SetLabel(const std::string &p_label); bool IsChance() const { return (m_number == 0); } @@ -1624,17 +1623,6 @@ inline void GameRep::CheckOutcomeLabel(const std::string &p_label) const inline bool GameInfosetRep::IsChanceInfoset() const { return m_player->IsChance(); } inline Game GamePlayerRep::GetGame() const { return m_game->shared_from_this(); } -inline void GamePlayerRep::SetLabel(const std::string &p_label) -{ - if (IsChance()) { - throw ValueException("The chance player's label cannot be changed"); - } - if (p_label == m_label) { - return; - } - GetGame()->CheckPlayerLabel(p_label); - m_label = p_label; -} inline GameStrategy GamePlayerRep::GetStrategy(int st) const { m_game->EnsureStrategies(); diff --git a/src/gui/dlgameprop.cc b/src/gui/dlgameprop.cc index 7d11bf1107..8deaa1ef9c 100644 --- a/src/gui/dlgameprop.cc +++ b/src/gui/dlgameprop.cc @@ -136,8 +136,8 @@ PlayerLabelPanel::PlayerLabelPanel(wxWindow *p_parent, GameDocument *p_doc, wxALIGN_CENTER_VERTICAL | wxALIGN_RIGHT); if (player->IsChance()) { - // Chance's label is reserved and can't be changed (GamePlayerRep::SetLabel throws on - // IsChance()) -- just display it. + // Chance's label is reserved and can't be changed (GameRep::RelabelPlayers rejects it + // as a key) -- just display it. gridSizer->Add(new wxStaticText(this, wxID_STATIC, wxString::FromUTF8(player->GetLabel())), 1, wxALIGN_CENTER_VERTICAL); } diff --git a/src/gui/gamedoc.cc b/src/gui/gamedoc.cc index 2a5c377a5f..53065bb2fb 100644 --- a/src/gui/gamedoc.cc +++ b/src/gui/gamedoc.cc @@ -414,9 +414,9 @@ GamePlayer GameDocument::DoNewPlayer() return player; } -void GameDocument::DoSetPlayerLabel(GamePlayer p_player, const wxString &p_label) +void GameDocument::DoRelabelPlayers(const std::map &p_labels) { - p_player->SetLabel(p_label.ToStdString(wxConvUTF8)); + m_game->RelabelPlayers(p_labels); NotifyChanged(GameModificationType::GameLabels); } diff --git a/src/gui/gamedoc.h b/src/gui/gamedoc.h index 2a004a1b5c..d501ad072e 100644 --- a/src/gui/gamedoc.h +++ b/src/gui/gamedoc.h @@ -293,7 +293,8 @@ class GameDocument { void DoSave(const wxString &p_filename, GameSaveFormat p_format); void DoSetTitle(const wxString &p_title, const wxString &p_comment); GamePlayer DoNewPlayer(); - void DoSetPlayerLabel(GamePlayer p_player, const wxString &p_label); + /// Reassign player labels in a single operation; see `Game::RelabelPlayers`. + void DoRelabelPlayers(const std::map &p_labels); /// Declare the strategies of `p_player` in a single operation, covering any combination /// of adding, deleting, reordering, and relabeling strategies. /// @@ -357,10 +358,12 @@ inline GameDocument *NewTableDocument(const std::vector &p_dim) { const Game nfg = NewTable(p_dim); nfg->SetTitle("Untitled Strategic Game"); + std::map labels; int pl = 1; - for (auto player : nfg->GetPlayers()) { - player->SetLabel("Player " + std::to_string(pl++)); + for (const auto &player : nfg->GetPlayers()) { + labels[player->GetLabel()] = "Player " + std::to_string(pl++); } + nfg->RelabelPlayers(labels); return new GameDocument(nfg); } diff --git a/src/gui/gameframe.cc b/src/gui/gameframe.cc index f65902e4f4..0ca2531f4c 100644 --- a/src/gui/gameframe.cc +++ b/src/gui/gameframe.cc @@ -882,15 +882,25 @@ void GameFrame::OnEditGame(wxCommandEvent &) m_doc->DoSetTitle(dialog.GetTitle(), dialog.GetDescription()); TreeRenderConfig style = m_doc->GetStyle(); + std::map labels; for (int i = 0; i < dialog.NumRows(); i++) { const GamePlayer player = dialog.GetPlayer(i); if (player->IsChance()) { style.SetChanceColor(dialog.GetPlayerColor(i)); continue; // chance's label is reserved and can't be changed } - m_doc->DoSetPlayerLabel(player, dialog.GetPlayerLabel(i)); + const std::string newLabel = dialog.GetPlayerLabel(i).ToStdString(wxConvUTF8); + if (newLabel != player->GetLabel()) { + labels[player->GetLabel()] = newLabel; + } style.SetPlayerColor(player->GetNumber(), dialog.GetPlayerColor(i)); } + // Relabeling all players in one call, rather than one at a time, lets two players' + // labels be swapped directly without tripping the duplicate-label check on an + // intermediate state that a per-player rename would pass through. + if (!labels.empty()) { + m_doc->DoRelabelPlayers(labels); + } m_doc->SetStyle(style); } catch (std::exception &ex) { diff --git a/src/pygambit/gambit.pxd b/src/pygambit/gambit.pxd index 1fb97a6844..785008659b 100644 --- a/src/pygambit/gambit.pxd +++ b/src/pygambit/gambit.pxd @@ -196,7 +196,6 @@ cdef extern from "games/game.h": int IsChance() except + string GetLabel() except + - void SetLabel(string) except +ValueError c_GameStrategy GetStrategy(int) except +IndexError Strategies GetStrategies() except +