From 0d96f69a47438ec76c3ad8cbbd40270feca63c39 Mon Sep 17 00:00:00 2001 From: drdkad Date: Mon, 24 Aug 2026 12:45:02 +0100 Subject: [PATCH 01/15] Represent absent outcomes by a per-game null outcome sentinel. --- src/games/game.cc | 6 ++++-- src/games/game.h | 30 ++++++++++++++++++++++++++---- src/games/gametable.cc | 36 ++++++++++++++++++------------------ src/games/gametree.cc | 14 +++++++++----- 4 files changed, 57 insertions(+), 29 deletions(-) diff --git a/src/games/game.cc b/src/games/game.cc index 9d72e81c01..67ec8c400b 100644 --- a/src/games/game.cc +++ b/src/games/game.cc @@ -39,8 +39,9 @@ namespace Gambit { // class GameOutcomeRep //======================================================================== -GameOutcomeRep::GameOutcomeRep(GameRep *p_game, int p_number, const std::string &p_label) - : m_game(p_game), m_number(p_number), m_label(p_label) +GameOutcomeRep::GameOutcomeRep(GameRep *p_game, int p_number, const std::string &p_label, + bool p_isNull /* = false */) + : m_isNull(p_isNull), m_game(p_game), m_number(p_number), m_label(p_label) { CheckLabel(p_label); for (const auto &player : m_game->m_players) { @@ -170,6 +171,7 @@ GameRep::~GameRep() for (auto outcome : m_outcomes) { outcome->Invalidate(); } + m_nullOutcome->Invalidate(); } void GameRep::IndexStrategies() const diff --git a/src/games/game.h b/src/games/game.h index fe5efa78b1..f103827d81 100644 --- a/src/games/game.h +++ b/src/games/game.h @@ -331,6 +331,7 @@ class GameOutcomeRep : public std::enable_shared_from_this { friend class GameTableRep; bool m_valid{true}; + bool m_isNull; GameRep *m_game; int m_number; std::string m_label; @@ -340,12 +341,14 @@ class GameOutcomeRep : public std::enable_shared_from_this { /// @name Lifecycle //@{ /// Creates a new outcome object, with payoffs set to zero - GameOutcomeRep(GameRep *p_game, int p_number, const std::string &p_label); + GameOutcomeRep(GameRep *p_game, int p_number, const std::string &p_label, bool p_isNull = false); ~GameOutcomeRep() = default; //@} bool IsValid() const { return m_valid; } void Invalidate() { m_valid = false; } + /// Returns whether this is the game's null outcome. + bool IsNull() const { return m_isNull; } /// @name Data access //@{ @@ -674,7 +677,7 @@ class GameNodeRep : public std::enable_shared_from_this { std::string m_label; GameInfosetRep *m_infoset{nullptr}; GameNodeRep *m_parent; - GameOutcomeRep *m_outcome{nullptr}; + GameOutcomeRep *m_outcome; std::vector> m_children; void DeleteOutcome(GameOutcomeRep *outc); @@ -733,7 +736,10 @@ class GameNodeRep : public std::enable_shared_from_this { GameNode GetNextSibling() const; GameNode GetPriorSibling() const; - GameOutcome GetOutcome() const { return (m_outcome) ? m_outcome->shared_from_this() : nullptr; } + GameOutcome GetOutcome() const + { + return (m_outcome->IsNull()) ? nullptr : m_outcome->shared_from_this(); + } bool IsSuccessorOf(GameNode from) const; bool IsSubgameRoot() const; @@ -923,11 +929,13 @@ class GameRep : public std::enable_shared_from_this { protected: std::vector> m_players; std::vector> m_outcomes; + /// The game's null outcome, shared by every node or contingency with no outcome attached. + std::shared_ptr m_nullOutcome; mutable CartesianProductSpace m_pureStrategies; std::string m_title, m_comment; unsigned int m_version{0}; - GameRep() = default; + GameRep() { m_nullOutcome = std::make_shared(this, 0, "", true); } /// @name Managing the representation //@{ @@ -1494,6 +1502,9 @@ class GameRep : public std::enable_shared_from_this { inline Game GameOutcomeRep::GetGame() const { return m_game->shared_from_this(); } inline void GameOutcomeRep::SetLabel(const std::string &p_label) { + if (m_isNull) { + throw ValueException("The null outcome's label cannot be changed"); + } if (p_label == m_label) { return; } @@ -1503,6 +1514,10 @@ inline void GameOutcomeRep::SetLabel(const std::string &p_label) template const T &GameOutcomeRep::GetPayoff(const GamePlayer &p_player) const { + if (m_isNull) { + static const Number kZeroPayoff; + return static_cast(kZeroPayoff); + } try { return static_cast(m_payoffs.at(p_player.get())); } @@ -1513,6 +1528,10 @@ template const T &GameOutcomeRep::GetPayoff(const GamePlayer &p_player template <> inline const Number &GameOutcomeRep::GetPayoff(const GamePlayer &p_player) const { + if (m_isNull) { + static const Number kZeroPayoff; + return kZeroPayoff; + } try { return m_payoffs.at(p_player.get()); } @@ -1523,6 +1542,9 @@ template <> inline const Number &GameOutcomeRep::GetPayoff(const GamePlayer &p_p inline void GameOutcomeRep::SetPayoff(const GamePlayer &p_player, const Number &p_value) { + if (m_isNull) { + throw UndefinedException("Payoffs cannot be set on the null outcome"); + } if (p_player->GetGame() != GetGame()) { throw MismatchException(); } diff --git a/src/games/gametable.cc b/src/games/gametable.cc index c749b01f7a..7d869daceb 100644 --- a/src/games/gametable.cc +++ b/src/games/gametable.cc @@ -58,20 +58,20 @@ Game NewTable(const std::vector &p_dim, bool p_sparseOutcomes /*= false*/) GameOutcome TablePureStrategyProfileRep::GetOutcome() const { - if (const auto outcome = dynamic_cast(*m_game).m_results.at(m_index)) { - return outcome->shared_from_this(); - } - return nullptr; + const auto outcome = dynamic_cast(*m_game).m_results.at(m_index); + return outcome->IsNull() ? nullptr : outcome->shared_from_this(); } void TablePureStrategyProfileRep::SetOutcome(GameOutcome p_outcome) { - dynamic_cast(*m_game).m_results[m_index] = p_outcome.get(); + dynamic_cast(*m_game).m_results[m_index] = + p_outcome ? p_outcome.get() : m_game->m_nullOutcome.get(); } Rational TablePureStrategyProfileRep::GetPayoff(const GamePlayer &p_player) const { - if (const auto outcome = dynamic_cast(*m_game).m_results.at(m_index)) { + if (const auto outcome = dynamic_cast(*m_game).m_results.at(m_index); + !outcome->IsNull()) { return outcome->GetPayoff(p_player); } return Rational(0); @@ -86,7 +86,8 @@ Rational TablePureStrategyProfileRep::GetStrategyValue(const GameStrategy &p_str const long digit_old = (m_index / stride) % m_radices[index]; const long digit_new = p_strategy->GetNumber() - 1; const long new_index = m_index + (digit_new - digit_old) * stride; - if (const auto outcome = dynamic_cast(*m_game).m_results[new_index]) { + if (const auto outcome = dynamic_cast(*m_game).m_results[new_index]; + !outcome->IsNull()) { return outcome->GetPayoff(player); } return Rational(0); @@ -297,7 +298,7 @@ template T TableMixedStrategyProfileRep::GetPayoff(int pl) const const auto player = game->GetPlayer(pl); T value{0}; for (auto [index, prob] : ProductDistribution(this->m_probs, this->m_offsets)) { - if (const auto outcome = g.m_results[index]) { + if (const auto outcome = g.m_results[index]; !outcome->IsNull()) { value += prob * outcome->template GetPayoff(player); } } @@ -314,7 +315,7 @@ T TableMixedStrategyProfileRep::GetPayoffDeriv(int pl, const GameStrategy &st T value{0}; for (auto [index, prob] : ProductDistribution(this->m_probs, this->m_offsets, strategy->GetPlayer()->GetNumber())) { - if (const auto outcome = g.m_results[base_index + index]) { + if (const auto outcome = g.m_results[base_index + index]; !outcome->IsNull()) { value += prob * outcome->template GetPayoff(player); } } @@ -332,7 +333,7 @@ bool TableMixedStrategyProfileRep::GetPayoffDerivs(int pl, Vector &p_deriv for (auto [index, prob] : ProductDistribution(this->m_probs, this->m_offsets, pl)) { auto deriv_it = p_derivs.begin(); for (const auto base_index : segment) { - if (const auto outcome = g.m_results[base_index + index]) { + if (const auto outcome = g.m_results[base_index + index]; !outcome->IsNull()) { *deriv_it += prob * outcome->template GetPayoff(player); ++deriv_it; } @@ -356,7 +357,7 @@ T TableMixedStrategyProfileRep::GetPayoffDeriv(int pl, const GameStrategy &st for (auto [index, prob] : ProductDistribution(this->m_probs, this->m_offsets, strategy1->GetPlayer()->GetNumber(), strategy2->GetPlayer()->GetNumber())) { - if (const auto outcome = g.m_results[base_index + index]) { + if (const auto outcome = g.m_results[base_index + index]; !outcome->IsNull()) { value += prob * outcome->template GetPayoff(player); } } @@ -385,7 +386,7 @@ GameTableRep::GameTableRep(const std::vector &dim, bool p_sparseOutcomes /* IndexStrategies(); if (p_sparseOutcomes) { - std::fill(m_results.begin(), m_results.end(), nullptr); + std::fill(m_results.begin(), m_results.end(), m_nullOutcome.get()); } else { m_outcomes = std::vector>(m_results.size()); @@ -500,8 +501,7 @@ void GameTableRep::WriteNfgFile(std::ostream &p_file) const void GameTableRep::DeleteOutcome(const GameOutcome &p_outcome) { IncrementVersion(); - std::replace(m_results.begin(), m_results.end(), p_outcome.get(), - static_cast(nullptr)); + std::replace(m_results.begin(), m_results.end(), p_outcome.get(), m_nullOutcome.get()); m_outcomes.erase( std::find(m_outcomes.begin(), m_outcomes.end(), std::shared_ptr(p_outcome))); p_outcome->Invalidate(); @@ -692,9 +692,9 @@ void GameTableRep::SetPlayers(const std::vector &p_labels) new_strides[j] = new_size; new_size *= m_players[j]->m_strategies.size(); } - std::vector newResults(new_size, nullptr); + std::vector newResults(new_size, m_nullOutcome.get()); for (long old_index = 0; old_index < old_size; ++old_index) { - if (m_results[old_index] == nullptr) { + if (m_results[old_index]->IsNull()) { continue; } long new_index = 0; @@ -763,9 +763,9 @@ void GameTableRep::RebuildTable(const std::vector &old_radices, long p_pla new_size *= m_players[i]->m_strategies.size(); } - std::vector newResults(new_size, nullptr); + std::vector newResults(new_size, m_nullOutcome.get()); for (long old_index = 0; old_index < old_size; ++old_index) { - if (m_results[old_index] == nullptr) { + if (m_results[old_index]->IsNull()) { continue; } long new_index = 0; diff --git a/src/games/gametree.cc b/src/games/gametree.cc index 4e01fc69a6..ea95951b1e 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -360,7 +360,10 @@ GameSubgameRep::InfosetCollection GameSubgameRep::GetSubgameDifference() const // class GameNodeRep //======================================================================== -GameNodeRep::GameNodeRep(GameRep *e, GameNodeRep *p) : m_game(e), m_parent(p) {} +GameNodeRep::GameNodeRep(GameRep *e, GameNodeRep *p) + : m_game(e), m_parent(p), m_outcome(e->m_nullOutcome.get()) +{ +} GameNodeRep::~GameNodeRep() { @@ -413,7 +416,7 @@ void GameNodeRep::DeleteOutcome(GameOutcomeRep *outc) { m_game->IncrementVersion(); if (outc == m_outcome) { - m_outcome = nullptr; + m_outcome = m_game->m_nullOutcome.get(); } for (auto child : m_children) { child->DeleteOutcome(outc); @@ -428,7 +431,8 @@ void GameTreeRep::SetOutcome(const GameNode &p_node, const GameOutcome &p_outcom if (p_outcome && p_outcome->m_game != this) { throw MismatchException(); } - if (const auto newOutcome = p_outcome.get_shared().get(); newOutcome != p_node->m_outcome) { + if (const auto newOutcome = p_outcome ? p_outcome.get_shared().get() : m_nullOutcome.get(); + newOutcome != p_node->m_outcome) { p_node->m_outcome = newOutcome; IncrementVersion(); } @@ -511,7 +515,7 @@ void GameTreeRep::DeleteTree(GameNode p_node) RemoveMember(node->m_infoset, node); node->m_infoset = nullptr; } - node->m_outcome = nullptr; + node->m_outcome = m_nullOutcome.get(); node->m_label = ""; ClearComputedValues(); @@ -578,7 +582,7 @@ void GameTreeRep::MoveTree(GameNode p_dest, GameNode p_src) dest->shared_from_this())); std::swap(src->m_parent, dest->m_parent); dest->m_label = ""; - dest->m_outcome = nullptr; + dest->m_outcome = m_nullOutcome.get(); ClearComputedValues(); InvalidateTreeOrdering(); From 225b3f4f0108be24abafe9283c53c789c784f927 Mon Sep 17 00:00:00 2001 From: drdkad Date: Mon, 24 Aug 2026 21:47:28 +0100 Subject: [PATCH 02/15] Resolve `Node.outcome` to the null outcome rather than `None` --- src/games/behavmixed.cc | 8 ++--- src/games/behavpure.cc | 6 ++-- src/games/game.h | 5 +-- src/games/gametree.cc | 15 ++++----- src/games/seqpure.cc | 4 +-- src/pygambit/gambit.pxd | 1 + src/pygambit/node.pxi | 58 ++++++++++++---------------------- src/pygambit/outcome.pxi | 33 +++++++++++++++++-- src/solvers/logit/logbehav.imp | 8 ++--- tests/test_node.py | 41 ++++++++++++++++++++++++ 10 files changed, 110 insertions(+), 69 deletions(-) diff --git a/src/games/behavmixed.cc b/src/games/behavmixed.cc index c5e2deb7ee..ef03f8bfa2 100644 --- a/src/games/behavmixed.cc +++ b/src/games/behavmixed.cc @@ -552,11 +552,9 @@ template void MixedBehaviorProfile::ComputeNodeValues() const for (const auto &player : game->GetPlayers()) { vals[player] = static_cast(0); } - if (node->GetOutcome()) { - const GameOutcome &outcome = node->GetOutcome(); - for (const auto &player : game->GetPlayers()) { - vals[player] += outcome->GetPayoff(player); - } + const GameOutcome &outcome = node->GetOutcome(); + for (const auto &player : game->GetPlayers()) { + vals[player] += outcome->GetPayoff(player); } for (auto [action, child] : node->GetActions()) { const T p = GetActionProb(action); diff --git a/src/games/behavpure.cc b/src/games/behavpure.cc index e013dc0d42..72ce677f80 100644 --- a/src/games/behavpure.cc +++ b/src/games/behavpure.cc @@ -50,9 +50,7 @@ T PureBehaviorProfile::GetPayoff(const GameNode &p_node, const GamePlayer &p_pla { T payoff(0); - if (p_node->GetOutcome()) { - payoff += p_node->GetOutcome()->GetPayoff(p_player); - } + payoff += p_node->GetOutcome()->GetPayoff(p_player); if (!p_node->IsTerminal()) { if (p_node->GetInfoset()->IsChanceInfoset()) { @@ -78,7 +76,7 @@ GameOutcome PureBehaviorProfile::GetOutcome(const GameNode &p_start) const GameOutcome outcome; GameNode node = p_start; while (true) { - if (node->GetOutcome()) { + if (!node->GetOutcome()->IsNull()) { if (outcome) { // More than one outcome-bearing node along this path; the combined // payoff cannot be attributed to a single outcome's preserved text. diff --git a/src/games/game.h b/src/games/game.h index f103827d81..a7ad310724 100644 --- a/src/games/game.h +++ b/src/games/game.h @@ -736,10 +736,7 @@ class GameNodeRep : public std::enable_shared_from_this { GameNode GetNextSibling() const; GameNode GetPriorSibling() const; - GameOutcome GetOutcome() const - { - return (m_outcome->IsNull()) ? nullptr : m_outcome->shared_from_this(); - } + GameOutcome GetOutcome() const { return m_outcome->shared_from_this(); } bool IsSuccessorOf(GameNode from) const; bool IsSubgameRoot() const; diff --git a/src/games/gametree.cc b/src/games/gametree.cc index ea95951b1e..bd311c0b5c 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -975,11 +975,10 @@ bool GameTreeRep::IsConstSum() const } } - if (const auto outcome = p_node->GetOutcome()) { - sum += sum_function(m_game->m_players, [&](const auto &p_player) { - return outcome->GetPayoff(p_player); - }); - } + const auto outcome = p_node->GetOutcome(); + sum += sum_function(m_game->m_players, [&](const auto &p_player) { + return outcome->GetPayoff(p_player); + }); m_subtreeSums[p_node] = sum; return DFSCallbackResult::Continue; } @@ -1018,9 +1017,7 @@ Rational GameTreeRep::AggregateSubtreePayoff(const GamePlayer &p_player, m_subtreeValues.erase(child); } } - if (const auto outcome = p_node->GetOutcome()) { - value += outcome->GetPayoff(m_player); - } + value += p_node->GetOutcome()->GetPayoff(m_player); m_subtreeValues[p_node] = value; m_result = value; // We write the root node value last, so will be correct on termination return DFSCallbackResult::Continue; @@ -1665,7 +1662,7 @@ void WriteEfgFile(std::ostream &f, const GameNode &n) } f << ' '; } - if (n->GetOutcome()) { + if (!n->GetOutcome()->IsNull()) { f << n->GetOutcome()->GetNumber() << " " << QuoteString(n->GetOutcome()->GetLabel()) << ' ' << FormatList( n->GetGame()->GetPlayers(), diff --git a/src/games/seqpure.cc b/src/games/seqpure.cc index 83fb6b7e89..090f2cabc5 100644 --- a/src/games/seqpure.cc +++ b/src/games/seqpure.cc @@ -149,9 +149,7 @@ Rational PureSequenceProfile::GetPayoff(const GamePlayer &p_player) const frames.pop(); const GameNode &n = frame.node; - if (n->GetOutcome()) { - frame.cumulative += n->GetOutcome()->GetPayoff(p_player); - } + frame.cumulative += n->GetOutcome()->GetPayoff(p_player); if (n->IsTerminal()) { // Credit the cumulative payoff along this path only if every diff --git a/src/pygambit/gambit.pxd b/src/pygambit/gambit.pxd index fe51bae0e8..82a3d19cb1 100644 --- a/src/pygambit/gambit.pxd +++ b/src/pygambit/gambit.pxd @@ -208,6 +208,7 @@ cdef extern from "games/game.h": cdef cppclass c_GameOutcomeRep "GameOutcomeRep": c_Game GetGame() except + int GetNumber() except + + bool IsNull() except + string GetLabel() except + void SetLabel(string) except +ValueError diff --git a/src/pygambit/node.pxi b/src/pygambit/node.pxi index fc6e1b19af..957a9a5ffa 100644 --- a/src/pygambit/node.pxi +++ b/src/pygambit/node.pxi @@ -182,6 +182,11 @@ class NodeOutcome: so the value reflects the current state of the game even after the game is mutated. .. versionadded:: 16.7.0 + + .. versionchanged:: 17.0.0 + A node with no outcome attached resolves to the game's null outcome: the view is + falsy, its ``label`` is ``None``, its payoffs read as zero, and it compares unequal + to every outcome — including another null and itself — and to ``None``. """ node = cython.declare(c_GameNode) @@ -197,65 +202,38 @@ class NodeOutcome: @cython.cfunc def _resolve(self) -> Outcome: - if self.node.deref().GetOutcome() == cython.cast(c_GameOutcome, NULL): - return None return Outcome.wrap(self.node.deref().GetOutcome()) def __getattr__(self, name): if name.startswith("_"): raise AttributeError(f"'NodeOutcome' object has no attribute '{name}'") - resolved = self._resolve() - if resolved is None: - raise AttributeError( - f"node has no outcome attached; cannot access '{name}'" - ) - return getattr(resolved, name) + return getattr(self._resolve(), name) def __getitem__(self, player): - resolved = self._resolve() - if resolved is None: - raise KeyError("node has no outcome attached") - return resolved[player] + return self._resolve()[player] def __setitem__(self, player, value): - resolved = self._resolve() - if resolved is None: - raise KeyError("node has no outcome attached") - resolved[player] = value + self._resolve()[player] = value @property def label(self): - resolved = self._resolve() - if resolved is None: - raise AttributeError("node has no outcome attached; cannot access 'label'") - return resolved.label + return self._resolve().label @label.setter def label(self, value): - resolved = self._resolve() - if resolved is None: - raise AttributeError("node has no outcome attached; cannot set 'label'") - resolved.label = value + self._resolve().label = value def __repr__(self) -> str: - resolved = self._resolve() - return repr(resolved) if resolved is not None else "None" + return repr(self._resolve()) def __eq__(self, other: typing.Any) -> bool: - mine = self._resolve() - if isinstance(other, NodeOutcome): - other = cython.cast(NodeOutcome, other)._resolve() - if mine is None or other is None: - return mine is None and other is None - return mine == other + return self._resolve() == other def __bool__(self) -> bool: - return self._resolve() is not None + return not self.node.deref().GetOutcome().deref().IsNull() def __hash__(self) -> int: - # Hash by the resolved outcome (transitional). - resolved = self._resolve() - return hash(resolved) if resolved is not None else 0 + return hash(self._resolve()) @cython.cclass @@ -517,11 +495,17 @@ class Node: Returns a lazy, node-anchored view resolved on each access, so the value reflects the current state of the game even if the game is mutated after this property is read. - When no outcome is attached, the view is falsy and equals ``None``. + When no outcome is attached, the view resolves to the game's null outcome: + its ``label`` is ``None``, its payoffs read as zero, and it compares unequal + to every outcome, including another null. .. versionchanged:: 16.7.0 Now returns a lazily-evaluated, node-anchored view rather than capturing the outcome at the time of access. + + .. versionchanged:: 17.0.0 + Resolves to the null outcome rather than ``None`` when no outcome is attached; + two null outcomes compare unequal. """ return NodeOutcome.wrap(self.node) diff --git a/src/pygambit/outcome.pxi b/src/pygambit/outcome.pxi index 6d9e0c362b..914f8d1a71 100644 --- a/src/pygambit/outcome.pxi +++ b/src/pygambit/outcome.pxi @@ -42,6 +42,8 @@ class Outcome: return obj def __repr__(self) -> str: + if self.outcome.deref().IsNull(): + return f"Outcome(game={self.game}, label=None)" if self.label: return f"Outcome(game={self.game}, label='{self.label}')" else: @@ -50,20 +52,31 @@ class Outcome: def __eq__(self, other: typing.Any): if not isinstance(other, Outcome): return NotImplemented + if (self.outcome.deref().IsNull() + or cython.cast(Outcome, other).outcome.deref().IsNull()): + # Null outcomes are not equal to anything, including themselves (cf. nan). + return False return self.outcome.deref() == cython.cast(Outcome, other).outcome.deref() def __hash__(self) -> int: return cython.cast(cython.long, self.outcome.deref()) + def __bool__(self) -> bool: + """``True`` for a real outcome; the null outcome is falsy.""" + return not self.outcome.deref().IsNull() + @property def game(self) -> Game: """Returns the game with which this outcome is associated.""" return Game.wrap(self.outcome.deref().GetGame()) @property - def label(self) -> str: + def label(self) -> str | None: """The text label associated with this outcome. + The null outcome's label is ``None``; testing ``outcome.label is None`` is the + idiomatic nullity check. + .. versionchanged:: 16.7.0 An outcome label must be nonempty and unique within the game; an empty or duplicate label now raises ``ValueError``. @@ -73,7 +86,10 @@ class Outcome: 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 null outcome resolves with label ``None``. """ + if self.outcome.deref().IsNull(): + return None return self.outcome.deref().GetLabel().decode("utf-8") @label.setter @@ -81,10 +97,17 @@ class Outcome: self.outcome.deref().SetLabel(value.encode("utf-8")) @property - def number(self) -> int: + def number(self) -> int | None: """Returns the number of the outcome in the game. Outcomes are numbered starting with 0. + + The null outcome is not a member of the game's outcomes, so it has no number. + + .. versionchanged:: 17.0.0 + The null outcome resolves here with number ``None``. """ + if self.outcome.deref().IsNull(): + return None return self.outcome.deref().GetNumber() - 1 def __getitem__( @@ -92,6 +115,8 @@ class Outcome: ) -> decimal.Decimal | Rational: """The payoff to `player` at the outcome. + The null outcome reports a zero payoff to every player of its game. + Raises ------ MismatchError @@ -124,7 +149,11 @@ class Outcome: If `player` is a ``Player`` from a different game than the outcome. ValueError If `value` cannot be interpreted as a number. + UndefinedOperationError + If this is the null outcome; payoffs cannot be set on it. """ + if self.outcome.deref().IsNull(): + raise UndefinedOperationError("Payoffs cannot be set on the null outcome") resolved_player = cython.cast(Player, self.game._resolve_player(player, "Outcome.__setitem__")) self.outcome.deref().SetPayoff(resolved_player.player, _to_number(value)) diff --git a/src/solvers/logit/logbehav.imp b/src/solvers/logit/logbehav.imp index aa13d878df..0cad1825cc 100644 --- a/src/solvers/logit/logbehav.imp +++ b/src/solvers/logit/logbehav.imp @@ -169,11 +169,9 @@ T LogBehavProfile::DiffNodeValue(const GameNode &p_node, const GamePlayer &p_ template void LogBehavProfile::ComputeSolutionDataPass2(const GameNode &node) const { - if (node->GetOutcome()) { - const GameOutcome outcome = node->GetOutcome(); - for (auto player : m_game->GetPlayers()) { - m_nodeValues[node][player] += outcome->GetPayoff(player); - } + const GameOutcome outcome = node->GetOutcome(); + for (auto player : m_game->GetPlayers()) { + m_nodeValues[node][player] += outcome->GetPayoff(player); } const GameInfoset infoset = node->GetInfoset(); diff --git a/tests/test_node.py b/tests/test_node.py index 4618f06cc4..8dc84a3610 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -76,6 +76,45 @@ def test_outcome_equality_is_symmetric(): assert outcome == proxy +def test_null_outcome_label_is_none(): + """The blessed nullity idiom: a node with no outcome has `outcome.label is None`.""" + game = games.read_from_file("basic_extensive_game.efg") + assert game.root.outcome.label is None + + +def test_null_outcome_compares_unequal_to_itself(): + """Null outcomes are unequal to everything, including another view of the same + node's outcome; equality must not short-circuit on node identity.""" + game = games.read_from_file("basic_extensive_game.efg") + assert (game.root.outcome == game.root.outcome) is False + + +def test_null_outcome_reads_zero_payoffs(): + """Reading a payoff through an unset node reports zero to every player of the game.""" + game = games.read_from_file("basic_extensive_game.efg") + for player in game.players: + assert game.root.outcome[player] == 0 + + +def test_null_outcome_payoff_write_raises(): + game = games.read_from_file("basic_extensive_game.efg") + with pytest.raises(gbt.UndefinedOperationError): + game.root.outcome[game.players["Player 1"]] = 1 + + +def test_null_outcome_label_write_raises(): + game = games.read_from_file("basic_extensive_game.efg") + with pytest.raises(ValueError): + game.root.outcome.label = "Outcome 4" + + +def test_null_outcome_number_is_none(): + """The null outcome is not a member of the game's outcomes, so it has no number. + -1 would be a valid index and would silently resolve to the last real outcome.""" + game = games.read_from_file("basic_extensive_game.efg") + assert game.root.outcome.number 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") @@ -686,6 +725,8 @@ def _subtrees_equal( if n1 == recursion_stop_node: return n2.is_terminal if n1.is_terminal and n2.is_terminal: + if not n1.outcome and not n2.outcome: + return True return n1.outcome == n2.outcome if n1.is_terminal is not n2.is_terminal: return False From a083575946bd09ed5fe6c5ab6b09eb397dc45fcb Mon Sep 17 00:00:00 2001 From: drdkad Date: Wed, 26 Aug 2026 09:23:57 +0100 Subject: [PATCH 03/15] Implement Game.make_outcome to create and attach outcomes --- doc/tutorials/01_quickstart.ipynb | 30 +++--- src/games/file.cc | 63 +++++++++---- src/games/game.h | 20 +++- src/games/gametable.cc | 110 ++++++++++++++++------ src/games/gametable.h | 2 + src/games/gametree.cc | 63 +++++++++++++ src/games/gametree.h | 2 + src/pygambit/gambit.pxd | 6 +- src/pygambit/game.pxi | 146 ++++++++++++++++++++++++++---- src/pygambit/outcome.pxi | 5 +- tests/test_game.py | 5 + tests/test_outcomes.py | 14 +-- tests/test_players.py | 10 +- 13 files changed, 381 insertions(+), 95 deletions(-) diff --git a/doc/tutorials/01_quickstart.ipynb b/doc/tutorials/01_quickstart.ipynb index 37ad9a9b79..9b83fa2856 100644 --- a/doc/tutorials/01_quickstart.ipynb +++ b/doc/tutorials/01_quickstart.ipynb @@ -104,21 +104,11 @@ "metadata": {}, "outputs": [], "source": [ - "# Both cooperate\n", - "g[\"Cooperate\", \"Cooperate\"][\"Tom\"] = -1\n", - "g[\"Cooperate\", \"Cooperate\"][\"Jerry\"] = -1\n", - "\n", - "# Tom cooperates, Jerry defects\n", - "g[\"Cooperate\", \"Defect\"][\"Tom\"] = -3\n", - "g[\"Cooperate\", \"Defect\"][\"Jerry\"] = 0\n", - "\n", - "# Tom defects, Jerry cooperates\n", - "g[\"Defect\", \"Cooperate\"][\"Tom\"] = 0\n", - "g[\"Defect\", \"Cooperate\"][\"Jerry\"] = -3\n", - "\n", - "# Both defect\n", - "g[\"Defect\", \"Defect\"][\"Tom\"] = -2\n", - "g[\"Defect\", \"Defect\"][\"Jerry\"] = -2" + "# Each contingency gets an outcome, created and attached in one step\n", + "g.make_outcome((\"Cooperate\", \"Cooperate\"), {\"Tom\": -1, \"Jerry\": -1}, \"Both cooperate\")\n", + "g.make_outcome((\"Cooperate\", \"Defect\"), {\"Tom\": -3, \"Jerry\": 0}, \"Tom cooperates, Jerry defects\")\n", + "g.make_outcome((\"Defect\", \"Cooperate\"), {\"Tom\": 0, \"Jerry\": -3}, \"Tom defects, Jerry cooperates\")\n", + "g.make_outcome((\"Defect\", \"Defect\"), {\"Tom\": -2, \"Jerry\": -2}, \"Both defect\")" ] }, { @@ -291,7 +281,15 @@ "id": "980bf6b1", "metadata": {}, "outputs": [], - "source": "payoffs = msp.payoffs\nfor player in g.players:\n print(f\"{player.label} plays the equilibrium strategy:\")\n print(f\"Probability of cooperating: {msp[player.label]['Cooperate']}\")\n print(f\"Probability of defecting: {msp[player.label]['Defect']}\")\n print(f\"Payoff: {payoffs[player.label]}\")\n print()" + "source": [ + "payoffs = msp.payoffs\n", + "for player in g.players:\n", + " print(f\"{player.label} plays the equilibrium strategy:\")\n", + " print(f\"Probability of cooperating: {msp[player.label]['Cooperate']}\")\n", + " print(f\"Probability of defecting: {msp[player.label]['Defect']}\")\n", + " print(f\"Payoff: {payoffs[player.label]}\")\n", + " print()" + ] }, { "cell_type": "markdown", diff --git a/src/games/file.cc b/src/games/file.cc index 4f5a53292b..b95a2bd9a6 100644 --- a/src/games/file.cc +++ b/src/games/file.cc @@ -441,19 +441,22 @@ void ParseNfgHeader(GameFileLexer &p_state, TableFileGame &p_data) } } -void ReadOutcomeList(GameFileLexer &p_parser, Game &p_nfg) +struct NfgOutcomeRecords { + std::vector m_labels; + std::vector> m_payoffs; +}; + +NfgOutcomeRecords ReadOutcomeList(GameFileLexer &p_parser, const Game &p_nfg) { auto players = p_nfg->GetPlayers(); p_parser.GetNextToken(); - // Buffer raw labels + payoffs so labels can be normalized (unique, nonempty) - // before the outcome objects are created (NewOutcome now rejects empty/duplicate). - std::vector labels; - std::vector> payoff_lists; + // Buffer raw labels + payoffs; which records become outcomes depends on the index table + NfgOutcomeRecords records; while (p_parser.GetCurrentToken() == TOKEN_LBRACE) { p_parser.ExpectNextToken(TOKEN_TEXT, "outcome name"); - labels.push_back(p_parser.GetLastText()); + records.m_labels.push_back(p_parser.GetLastText()); p_parser.GetNextToken(); std::vector payoffs; @@ -462,34 +465,54 @@ void ReadOutcomeList(GameFileLexer &p_parser, Game &p_nfg) payoffs.emplace_back(p_parser.GetLastText()); p_parser.AcceptNextToken(TOKEN_COMMA); } - payoff_lists.push_back(payoffs); + records.m_payoffs.push_back(payoffs); p_parser.ExpectCurrentToken(TOKEN_RBRACE, "'}'"); p_parser.GetNextToken(); } p_parser.ExpectCurrentToken(TOKEN_RBRACE, "'}'"); p_parser.GetNextToken(); + return records; +} +void ParseOutcomeBody(GameFileLexer &p_parser, Game &p_nfg) +{ + const auto records = ReadOutcomeList(p_parser, p_nfg); + // First pass: the outcome id at each contingency, in iteration order. + std::vector ids; + for ([[maybe_unused]] const auto &profile : StrategyContingencies(p_nfg)) { + p_parser.ExpectCurrentToken(TOKEN_NUMBER, "outcome index"); + ids.push_back(std::stoi(p_parser.GetLastText())); + p_parser.GetNextToken(); + } + const std::set referenced(ids.begin(), ids.end()); + std::vector labels; + for (size_t i = 0; i < records.m_labels.size(); ++i) { + if (referenced.contains(static_cast(i) + 1)) { + labels.push_back(records.m_labels[i]); + } + } NormalizeLabelStrings(labels); - for (size_t i = 0; i < labels.size(); ++i) { - auto outcome = p_nfg->NewOutcome(labels[i]); - auto player_it = players.begin(); - for (const auto &payoff : payoff_lists[i]) { + std::map created; + auto label_it = labels.begin(); + for (size_t i = 0; i < records.m_labels.size(); ++i) { + if (!referenced.contains(static_cast(i) + 1)) { + continue; + } + auto outcome = p_nfg->NewOutcome(*label_it++); + auto player_it = p_nfg->GetPlayers().begin(); + for (const auto &payoff : records.m_payoffs[i]) { outcome->SetPayoff(*player_it, payoff); ++player_it; } + created.emplace(static_cast(i) + 1, outcome); } -} - -void ParseOutcomeBody(GameFileLexer &p_parser, Game &p_nfg) -{ - ReadOutcomeList(p_parser, p_nfg); + // Second pass: attach. + auto id_it = ids.begin(); for (const auto &profile : StrategyContingencies(p_nfg)) { - p_parser.ExpectCurrentToken(TOKEN_NUMBER, "outcome index"); - if (const int outcomeId = std::stoi(p_parser.GetLastText())) { - profile->SetOutcome(p_nfg->GetOutcome(outcomeId)); + if (const int outcomeId = *id_it++) { + profile->SetOutcome(created.at(outcomeId)); } - p_parser.GetNextToken(); } } diff --git a/src/games/game.h b/src/games/game.h index a7ad310724..f54fad7221 100644 --- a/src/games/game.h +++ b/src/games/game.h @@ -944,7 +944,8 @@ class GameRep : public std::enable_shared_from_this { 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; + void CheckOutcomeLabel(const std::string &p_label, + const std::set &p_ignore = {}) const; //@} /// Hooks for derived classes to update lazily-computed orderings if required @@ -1432,6 +1433,18 @@ class GameRep : public std::enable_shared_from_this { } /// Creates a new outcome in the game virtual GameOutcome NewOutcome(const std::string &p_label) { throw UndefinedException(); } + /// Creates an outcome with the given payoffs and label for the specified nodes. + virtual GameOutcome MakeOutcome(const std::vector &, const std::vector &, + const std::string &) + { + throw UndefinedException(); + } + /// Creates an outcome with the given payoffs and label for the specified contingencies. + virtual GameOutcome MakeOutcome(const std::vector> &, + const std::vector &, const std::string &) + { + throw UndefinedException(); + } /// Deletes the specified outcome from the game virtual void DeleteOutcome(const GameOutcome &) { throw UndefinedException(); } //@} @@ -1627,14 +1640,15 @@ inline void GameRep::CheckPlayerLabel(const std::string &p_label, } } } -inline void GameRep::CheckOutcomeLabel(const std::string &p_label) const +inline void GameRep::CheckOutcomeLabel(const std::string &p_label, + const std::set &p_ignore) const { if (p_label.empty()) { throw ValueException("Outcome label must not be empty"); } CheckLabel(p_label); for (const auto &outcome : m_outcomes) { - if (outcome->GetLabel() == p_label) { + if (outcome->GetLabel() == p_label && !p_ignore.contains(outcome.get())) { throw ValueException("Outcome label must be unique within the game"); } } diff --git a/src/games/gametable.cc b/src/games/gametable.cc index 7d869daceb..c556294f0d 100644 --- a/src/games/gametable.cc +++ b/src/games/gametable.cc @@ -58,8 +58,7 @@ Game NewTable(const std::vector &p_dim, bool p_sparseOutcomes /*= false*/) GameOutcome TablePureStrategyProfileRep::GetOutcome() const { - const auto outcome = dynamic_cast(*m_game).m_results.at(m_index); - return outcome->IsNull() ? nullptr : outcome->shared_from_this(); + return dynamic_cast(*m_game).m_results.at(m_index)->shared_from_this(); } void TablePureStrategyProfileRep::SetOutcome(GameOutcome p_outcome) @@ -70,11 +69,8 @@ void TablePureStrategyProfileRep::SetOutcome(GameOutcome p_outcome) Rational TablePureStrategyProfileRep::GetPayoff(const GamePlayer &p_player) const { - if (const auto outcome = dynamic_cast(*m_game).m_results.at(m_index); - !outcome->IsNull()) { - return outcome->GetPayoff(p_player); - } - return Rational(0); + return dynamic_cast(*m_game).m_results.at(m_index)->GetPayoff( + p_player); } Rational TablePureStrategyProfileRep::GetStrategyValue(const GameStrategy &p_strategy) const @@ -86,11 +82,7 @@ Rational TablePureStrategyProfileRep::GetStrategyValue(const GameStrategy &p_str const long digit_old = (m_index / stride) % m_radices[index]; const long digit_new = p_strategy->GetNumber() - 1; const long new_index = m_index + (digit_new - digit_old) * stride; - if (const auto outcome = dynamic_cast(*m_game).m_results[new_index]; - !outcome->IsNull()) { - return outcome->GetPayoff(player); - } - return Rational(0); + return dynamic_cast(*m_game).m_results[new_index]->GetPayoff(player); } PureStrategyProfile GameTableRep::NewPureStrategyProfile() const @@ -298,9 +290,7 @@ template T TableMixedStrategyProfileRep::GetPayoff(int pl) const const auto player = game->GetPlayer(pl); T value{0}; for (auto [index, prob] : ProductDistribution(this->m_probs, this->m_offsets)) { - if (const auto outcome = g.m_results[index]; !outcome->IsNull()) { - value += prob * outcome->template GetPayoff(player); - } + value += prob * g.m_results[index]->template GetPayoff(player); } return value; } @@ -315,9 +305,7 @@ T TableMixedStrategyProfileRep::GetPayoffDeriv(int pl, const GameStrategy &st T value{0}; for (auto [index, prob] : ProductDistribution(this->m_probs, this->m_offsets, strategy->GetPlayer()->GetNumber())) { - if (const auto outcome = g.m_results[base_index + index]; !outcome->IsNull()) { - value += prob * outcome->template GetPayoff(player); - } + value += prob * g.m_results[base_index + index]->template GetPayoff(player); } return value; } @@ -333,10 +321,8 @@ bool TableMixedStrategyProfileRep::GetPayoffDerivs(int pl, Vector &p_deriv for (auto [index, prob] : ProductDistribution(this->m_probs, this->m_offsets, pl)) { auto deriv_it = p_derivs.begin(); for (const auto base_index : segment) { - if (const auto outcome = g.m_results[base_index + index]; !outcome->IsNull()) { - *deriv_it += prob * outcome->template GetPayoff(player); - ++deriv_it; - } + *deriv_it += prob * g.m_results[base_index + index]->template GetPayoff(player); + ++deriv_it; } } return true; @@ -357,9 +343,7 @@ T TableMixedStrategyProfileRep::GetPayoffDeriv(int pl, const GameStrategy &st for (auto [index, prob] : ProductDistribution(this->m_probs, this->m_offsets, strategy1->GetPlayer()->GetNumber(), strategy2->GetPlayer()->GetNumber())) { - if (const auto outcome = g.m_results[base_index + index]; !outcome->IsNull()) { - value += prob * outcome->template GetPayoff(player); - } + value += prob * g.m_results[base_index + index]->template GetPayoff(player); } return value; } @@ -484,8 +468,7 @@ void GameTableRep::WriteNfgFile(std::ostream &p_file) const for (auto iter : StrategyContingencies( StrategySupportProfile(std::const_pointer_cast(shared_from_this())))) { - const auto outcome = iter->GetOutcome(); - p_file << ((outcome) ? outcome->m_number : 0) << ' '; + p_file << iter->GetOutcome()->m_number << ' '; } p_file << std::endl; } @@ -498,8 +481,81 @@ void GameTableRep::WriteNfgFile(std::ostream &p_file) const // GameTableRep: Outcomes //------------------------------------------------------------------------ +GameOutcome +GameTableRep::MakeOutcome(const std::vector> &p_contingencies, + const std::vector &p_payoffs, const std::string &p_label) +{ + if (p_contingencies.empty()) { + throw ValueException("At least one contingency must be specified"); + } + if (p_payoffs.size() != m_players.size()) { + throw ValueException("A payoff must be specified for each player"); + } + const auto &strides = m_pureStrategies.m_strides; + // `covered` collects the candidates for absorption. + std::set selected; + std::set covered; + for (const auto &contingency : p_contingencies) { + if (contingency.size() != m_players.size()) { + throw ValueException("Each contingency must give one strategy per player"); + } + long index = 0; + for (const auto &[pl, strategy] : enumerate(contingency)) { + if (strategy->m_player != m_players[pl].get()) { + if (strategy->m_player->m_game != this) { + throw MismatchException(); + } + throw ValueException("Each contingency must give one strategy per player"); + } + index += (strategy->m_number - 1) * strides[pl]; + } + if (!selected.insert(index).second) { + throw ValueException("Each contingency may be referenced only once"); + } + if (!m_results[index]->IsNull()) { + covered.insert(m_results[index]); + } + } + std::set absorbed; + if (!covered.empty()) { + // A candidate survives absorption if some cell outside the selection still references it. + absorbed = covered; + for (size_t index = 0; index < m_results.size() && !absorbed.empty(); index++) { + if (!selected.contains(static_cast(index))) { + absorbed.erase(m_results[index]); + } + } + } + CheckOutcomeLabel(p_label, absorbed); + + IncrementVersion(); + auto outcome = std::make_shared(this, m_outcomes.size() + 1, p_label); + m_outcomes.push_back(outcome); + for (const auto &[pl, player] : enumerate(m_players)) { + outcome->SetPayoff(player, p_payoffs[pl]); + } + for (const auto index : selected) { + m_results[index] = outcome.get(); + } + if (!absorbed.empty()) { + for (const auto *dead : absorbed) { + auto member = std::find_if(m_outcomes.begin(), m_outcomes.end(), + [dead](const auto &c) { return c.get() == dead; }); + (*member)->Invalidate(); + m_outcomes.erase(member); + } + std::for_each( + m_outcomes.begin(), m_outcomes.end(), + [outc = 1](const std::shared_ptr &c) mutable { c->m_number = outc++; }); + } + return outcome; +} + void GameTableRep::DeleteOutcome(const GameOutcome &p_outcome) { + if (p_outcome->IsNull()) { + throw UndefinedException("The null outcome cannot be deleted"); + } IncrementVersion(); std::replace(m_results.begin(), m_results.end(), p_outcome.get(), m_nullOutcome.get()); m_outcomes.erase( diff --git a/src/games/gametable.h b/src/games/gametable.h index 01e88f8682..6aa768415f 100644 --- a/src/games/gametable.h +++ b/src/games/gametable.h @@ -52,6 +52,8 @@ class GameTableRep : public GameExplicitRep { /// Construct a new table game with the given dimension /// If p_sparseOutcomes = true, outcomes for all contingencies are left null explicit GameTableRep(const std::vector &p_dim, bool p_sparseOutcomes = false); + GameOutcome MakeOutcome(const std::vector> &, + const std::vector &, const std::string &) override; Game Copy() const override; //@} diff --git a/src/games/gametree.cc b/src/games/gametree.cc index bd311c0b5c..873751b274 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -1833,8 +1833,71 @@ std::vector GameTreeRep::GetPlays(GameAction action) const return plays; } +GameOutcome GameTreeRep::MakeOutcome(const std::vector &p_nodes, + const std::vector &p_payoffs, + const std::string &p_label) +{ + if (p_nodes.empty()) { + throw ValueException("At least one node must be specified"); + } + if (p_payoffs.size() != m_players.size()) { + throw ValueException("A payoff must be specified for each player"); + } + std::set selected; + std::set covered; + for (const auto &node : p_nodes) { + if (node->m_game != this) { + throw MismatchException(); + } + if (!selected.insert(node.get()).second) { + throw ValueException("Each node may be referenced only once"); + } + if (!node->m_outcome->IsNull()) { + covered.insert(node->m_outcome); + } + } + std::set absorbed; + if (!covered.empty()) { + absorbed = covered; + for (const auto &node : GetNodes()) { + if (absorbed.empty()) { + break; + } + if (!selected.contains(node.get())) { + absorbed.erase(node->m_outcome); + } + } + } + CheckOutcomeLabel(p_label, absorbed); + + IncrementVersion(); + auto outcome = std::make_shared(this, m_outcomes.size() + 1, p_label); + m_outcomes.push_back(outcome); + for (const auto &[pl, player] : enumerate(m_players)) { + outcome->SetPayoff(player, p_payoffs[pl]); + } + for (auto *node : selected) { + node->m_outcome = outcome.get(); + } + if (!absorbed.empty()) { + for (const auto *dead : absorbed) { + auto member = std::find_if(m_outcomes.begin(), m_outcomes.end(), + [dead](const auto &c) { return c.get() == dead; }); + (*member)->Invalidate(); + m_outcomes.erase(member); + } + std::for_each( + m_outcomes.begin(), m_outcomes.end(), + [outc = 1](const std::shared_ptr &c) mutable { c->m_number = outc++; }); + } + return outcome; +} + void GameTreeRep::DeleteOutcome(const GameOutcome &p_outcome) { + if (p_outcome->IsNull()) { + throw UndefinedException("The null outcome cannot be deleted"); + } IncrementVersion(); m_root->DeleteOutcome(p_outcome.get()); p_outcome->Invalidate(); diff --git a/src/games/gametree.h b/src/games/gametree.h index 7e3906a77d..4aa1fa9f66 100644 --- a/src/games/gametree.h +++ b/src/games/gametree.h @@ -177,6 +177,8 @@ class GameTreeRep final : public GameExplicitRep { void MoveTree(GameNode dest, GameNode src) override; void DeleteParent(GameNode) override; void DeleteTree(GameNode) override; + GameOutcome MakeOutcome(const std::vector &, const std::vector &, + const std::string &) override; GameInfoset MakeInfoset(const std::vector &, const GamePlayer &, const std::string &) override; void Reveal(GameInfoset, GamePlayer) override; diff --git a/src/pygambit/gambit.pxd b/src/pygambit/gambit.pxd index 82a3d19cb1..e8291ab3ff 100644 --- a/src/pygambit/gambit.pxd +++ b/src/pygambit/gambit.pxd @@ -368,6 +368,10 @@ cdef extern from "games/game.h": void DeleteParent(c_GameNode) except + void DeleteTree(c_GameNode) except + c_GameInfoset MakeInfoset(stdvector[c_GameNode], c_GamePlayer, string) except +ValueError + c_GameOutcome MakeOutcome(stdvector[c_GameNode], stdvector[c_Number], + string) except +ValueError + c_GameOutcome MakeOutcome(stdvector[stdvector[c_GameStrategy]], stdvector[c_Number], + string) except +ValueError void Reveal(c_GameInfoset, c_GamePlayer) except + void RelabelActions(c_GameInfoset, stdmap[string, string]) except +ValueError void SetMoveActions(c_GameInfoset, stdvector[string]) except +ValueError @@ -384,7 +388,7 @@ cdef extern from "games/game.h": stdvector[c_GameSubgame] GetSubgames() except + c_Game NewTree(stdvector[string]) except +ValueError - c_Game NewTable(stdvector[int]) except + + c_Game NewTable(stdvector[int], bool) except + cdef extern from "games/stratpure.h": diff --git a/src/pygambit/game.pxi b/src/pygambit/game.pxi index 4ea1b9f880..1d8c1f176f 100644 --- a/src/pygambit/game.pxi +++ b/src/pygambit/game.pxi @@ -590,7 +590,7 @@ class Game: Game The newly-created strategic game. """ - g = Game.wrap(NewTable(list(dim))) + g = Game.wrap(NewTable(list(dim), True)) g.title = title return g @@ -631,7 +631,7 @@ class Game: if len(set(a.shape for a in arrays)) > 1: raise ValueError("All specified arrays must have the same shape") shape = arrays[0].shape - g = Game.new_table(shape) + g = Game.wrap(NewTable(list(shape), False)) 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] @@ -707,7 +707,7 @@ class Game: raise ValueError("All specified arrays must have the same shape") arrays = list(payoffs.values()) shape = arrays[0].shape - g = Game.new_table(shape) + g = Game.wrap(NewTable(list(shape), False)) g.relabel_players( {player.label: label for player, label in zip(g.players, payoffs, strict=True)} ) @@ -1029,12 +1029,7 @@ class Game: if self.is_tree or self.game.deref().IsAgg(): return DerivedGameOutcome.wrap(self.game, psp) - else: - outcome = Outcome.wrap(deref(deref(psp).deref()).GetOutcome()) - if outcome.outcome != cython.cast(c_GameOutcome, NULL): - return outcome - else: - return None + return Outcome.wrap(deref(deref(psp).deref()).GetOutcome()) def __getitem__(self, contingency): """Returns the `Outcome` associated with a profile of pure strategies. @@ -1053,38 +1048,63 @@ class Game: IndexError If an integer index is out of range for the corresponding player, or a label or ``Strategy`` does not belong to that player. + MismatchError + If a ``Strategy`` belongs to a different game. .. note:: Unlike the game's object collections, strategies within a contingency can be referenced by integer index, as a contingency is a coordinate in the players' strategy spaces; labels and ``Strategy`` objects are also accepted. """ + return self._get_contingency( + *tuple(self._resolve_contingency(contingency, "Game.__getitem__")) + ) + + def _resolve_contingency(self, contingency: typing.Any, funcname: str, + argname: str = "contingency") -> list: + """Resolve a pure-strategy contingency to a list of per-player strategy indices. + + Each element may be a ``Strategy``, its label, or its index within the + corresponding player's strategies. + """ players = list(self.players) try: if len(contingency) != len(players): - raise KeyError("Number of strategies is not equal to the number of players") + raise KeyError( + f"{funcname}(): number of strategies in {argname} is not equal to " + f"the number of players" + ) except TypeError: - raise TypeError("contingency must be a tuple-like object") from None + raise TypeError(f"{funcname}(): {argname} must be a tuple-like object") from None cont = [0 for _ in players] for (pl, st) in enumerate(contingency): player = players[pl] if isinstance(st, int): if st < 0 or st >= len(player.strategies): - raise IndexError(f"Provided strategy index {st} out of range for player {pl}") + raise IndexError( + f"{funcname}(): strategy index {st} out of range for player {pl}" + ) cont[pl] = st elif isinstance(st, str): try: cont[pl] = [s.label for s in player.strategies].index(st) except ValueError: - raise IndexError(f"Provided strategy label '{st}' not defined") + raise IndexError(f"{funcname}(): player {pl} has no strategy labelled '{st}'") elif isinstance(st, Strategy): + if st.game != self: + raise MismatchError(f"{funcname}(): {argname} must be composed of " + f"strategies from the same game") try: cont[pl] = list(player.strategies).index(st) except ValueError: - raise IndexError(f"Provided strategy '{st}' not available to player") + raise IndexError( + f"{funcname}(): strategy '{st}' not available to player {pl}" + ) else: - raise TypeError("Must use a tuple of ints, strategy labels, or strategies") - return self._get_contingency(*tuple(cont)) + raise TypeError( + f"{funcname}(): {argname} must contain ints, strategy labels, or strategies" + ) + return cont def _fill_strategy_profile(self, profile: MixedStrategyProfile, @@ -2654,6 +2674,100 @@ class Game: c_labels.push_back(label.encode("utf-8")) self.game.deref().SetPlayers(c_labels) + def make_outcome(self, + nodes, + payoffs: typing.Mapping, + label: str) -> Outcome: + """Create an outcome with `payoffs` and `label` and attach it at `nodes`. + + For an extensive game, `nodes` is a ``Node`` or an iterable of nodes. For a + strategic game, `nodes` is a pure-strategy contingency — a sequence giving one + strategy per player, each a ``Strategy`` or a strategy label — or an iterable + of such contingencies. + + Any outcome all of whose references are among `nodes` is absorbed by the + operation: it is removed from the game, and `label` may reuse its label. + + .. versionadded:: 17.0.0 + + Parameters + ---------- + nodes : Node, contingency, or iterable of these + Where to attach the new outcome. Nonempty; each node or contingency may + be referenced only once. + payoffs : Mapping + A complete mapping from the game's players (or their labels) to payoffs. + Every player must be present; zeroes must be given explicitly. + label : str + The label of the new outcome; must be nonempty and, after the operation, + unique within the game. + + Returns + ------- + Outcome + A reference to the newly-created outcome. + + Raises + ------ + MismatchError + If any node, strategy, or player is from a different game. + ValueError + If `nodes` is empty or contains a repeat; if `payoffs` is not a complete + mapping over exactly the game's players; or if `label` is empty or is held + by an outcome that is not absorbed by the operation. + UndefinedOperationError + If the game is in action-graph representation, where outcomes are not + represented explicitly. + """ + if self.game.deref().IsAgg(): + raise UndefinedOperationError( + "make_outcome(): operation not defined for games in action-graph representation" + ) + if not hasattr(payoffs, "items"): + raise TypeError( + f"make_outcome(): payoffs must be a mapping, not {payoffs.__class__.__name__}" + ) + resolved_payoffs = {} + for player, value in payoffs.items(): + resolved = cython.cast(Player, self._resolve_player(player, "make_outcome", "payoffs")) + if resolved in resolved_payoffs: + raise ValueError("make_outcome(): each player may appear only once in payoffs") + resolved_payoffs[resolved] = value + if set(resolved_payoffs) != set(self.players): + raise ValueError( + "make_outcome(): payoffs must be specified for each player of the game" + ) + c_payoffs = stdvector[c_Number]() + for player in self.players: + c_payoffs.push_back(_to_number(resolved_payoffs[player])) + if self.is_tree: + resolved_nodes = self._resolve_nodes(nodes, "make_outcome") + c_nodes = stdvector[c_GameNode]() + for n in resolved_nodes: + c_nodes.push_back(cython.cast(Node, n).node) + return Outcome.wrap( + self.game.deref().MakeOutcome(c_nodes, c_payoffs, label.encode("utf-8")) + ) + try: + entries = list(nodes) + except TypeError: + raise TypeError( + "make_outcome(): nodes must be a contingency or an iterable of contingencies" + ) from None + if entries and all(isinstance(e, (Strategy, str, int)) for e in entries): + entries = [entries] + players = list(self.players) + c_contingencies = stdvector[stdvector[c_GameStrategy]]() + for entry in entries: + c_one = stdvector[c_GameStrategy]() + for pl, index in enumerate(self._resolve_contingency(entry, "make_outcome", "nodes")): + strategy = list(players[pl].strategies)[index] + c_one.push_back(cython.cast(Strategy, strategy).strategy) + c_contingencies.push_back(c_one) + return Outcome.wrap( + self.game.deref().MakeOutcome(c_contingencies, c_payoffs, label.encode("utf-8")) + ) + def add_outcome(self, label: str, payoffs: list | None = None) -> Outcome: diff --git a/src/pygambit/outcome.pxi b/src/pygambit/outcome.pxi index 914f8d1a71..b3a56c1f74 100644 --- a/src/pygambit/outcome.pxi +++ b/src/pygambit/outcome.pxi @@ -153,7 +153,10 @@ class Outcome: If this is the null outcome; payoffs cannot be set on it. """ if self.outcome.deref().IsNull(): - raise UndefinedOperationError("Payoffs cannot be set on the null outcome") + raise UndefinedOperationError( + "Payoffs cannot be set on the null outcome; " + "use Game.make_outcome to create and attach an outcome" + ) resolved_player = cython.cast(Player, self.game._resolve_player(player, "Outcome.__setitem__")) self.outcome.deref().SetPayoff(resolved_player.player, _to_number(value)) diff --git a/tests/test_game.py b/tests/test_game.py index 6c01fc42bd..f65aee245d 100644 --- a/tests/test_game.py +++ b/tests/test_game.py @@ -99,6 +99,7 @@ def test_from_dict(): def test_game_get_outcome_by_index(): game = gbt.Game.new_table([2, 2]) + game.make_outcome((0, 0), {"1": 0, "2": 0}, "top left") assert game[0, 0] == next(iter(game.outcomes)) @@ -107,6 +108,7 @@ def test_game_get_outcome_by_label(): pl1, pl2 = game.players game.relabel_strategies(pl1, {next(iter(pl1.strategies)).label: "defect"}) game.relabel_strategies(pl2, {next(iter(pl2.strategies)).label: "cooperate"}) + game.make_outcome(("defect", "cooperate"), {"1": 0, "2": 0}, "corner") assert game["defect", "cooperate"] == next(iter(game.outcomes)) @@ -146,6 +148,9 @@ def test_game_get_outcome_unmatched_label(): def test_game_get_outcome_with_strategies(): game = gbt.Game.new_table([2, 2]) pl1, pl2 = game.players + game.make_outcome( + (next(iter(pl1.strategies)), next(iter(pl2.strategies))), {"1": 0, "2": 0}, "corner" + ) assert ( game[next(iter(pl1.strategies)), next(iter(pl2.strategies))] == next(iter(game.outcomes)) diff --git a/tests/test_outcomes.py b/tests/test_outcomes.py index b3ebcf6313..e0d3b67e47 100644 --- a/tests/test_outcomes.py +++ b/tests/test_outcomes.py @@ -15,7 +15,7 @@ def test_outcome_add(game: gbt.Game): @pytest.mark.parametrize( - "game", [gbt.Game.new_table([2, 2])] + "game", [gbt.Game.from_arrays([[0, 0], [0, 0]], [[0, 0], [0, 0]])] ) def test_outcome_delete(game: gbt.Game): outcome_count = len(game.outcomes) @@ -25,7 +25,7 @@ def test_outcome_delete(game: gbt.Game): @pytest.mark.parametrize("label", games.VALID_LABELS) def test_outcome_label(label: str): - game = gbt.Game.new_table([2, 2]) + game = gbt.Game.from_arrays([[0, 0], [0, 0]], [[0, 0], [0, 0]]) outcome = next(iter(game.outcomes)) outcome.label = label assert outcome.label == label @@ -33,7 +33,7 @@ def test_outcome_label(label: str): @pytest.mark.parametrize("label", games.INVALID_LABELS) def test_outcome_label_invalid_raises_valueerror(label: str): - game = gbt.Game.new_table([2, 2]) + game = gbt.Game.from_arrays([[0, 0], [0, 0]], [[0, 0], [0, 0]]) outcome = next(iter(game.outcomes)) with pytest.raises(ValueError): outcome.label = label @@ -42,7 +42,7 @@ def test_outcome_label_invalid_raises_valueerror(label: str): @pytest.mark.parametrize("label", games.UNICODE_LABELS) def test_outcome_label_unicode_accepted(label: str): """Non-ASCII UTF-8 labels are accepted as of #862 (17.0).""" - game = gbt.Game.new_table([2, 2]) + game = gbt.Game.from_arrays([[0, 0], [0, 0]], [[0, 0], [0, 0]]) outcome = next(iter(game.outcomes)) outcome.label = label assert outcome.label == label @@ -50,7 +50,7 @@ def test_outcome_label_unicode_accepted(label: str): @pytest.mark.parametrize( "game,label", - [(gbt.Game.new_table([2, 2]), "outcome label")] + [(gbt.Game.from_arrays([[0, 0], [0, 0]], [[0, 0], [0, 0]]), "outcome label")] ) def test_outcome_index_label(game: gbt.Game, label: str): outcome = next(iter(game.outcomes)) @@ -60,7 +60,7 @@ def test_outcome_index_label(game: gbt.Game, label: str): @pytest.mark.parametrize( - "game", [gbt.Game.new_table([2, 2])] + "game", [gbt.Game.from_arrays([[0, 0], [0, 0]], [[0, 0], [0, 0]])] ) def test_outcome_index_unmatched_label(game: gbt.Game): with pytest.raises(KeyError): @@ -82,7 +82,7 @@ def test_outcome_index_invalid_type(game: gbt.Game): def test_outcome_payoff_by_player_label(): - game = gbt.Game.new_table([2, 2]) + game = gbt.Game.from_arrays([[0, 0], [0, 0]], [[0, 0], [0, 0]]) pl1, pl2 = list(game.players) game.relabel_players({pl1.label: "joe", pl2.label: "dan"}) out1, out2, *_ = list(game.outcomes) diff --git a/tests/test_players.py b/tests/test_players.py index 4339f04fc0..9851da8728 100644 --- a/tests/test_players.py +++ b/tests/test_players.py @@ -212,11 +212,13 @@ def _tag_contingencies(game: gbt.Game) -> None: can be recomputed from its strategies' labels after the game is mutated. """ players = list(game.players) - for contingency in game.contingencies: - outcome = game[contingency] + for n, contingency in enumerate(game.contingencies, start=1): strategies = [list(p.strategies)[i] for p, i in zip(players, contingency, strict=True)] - for pl_index, (player, strategy) in enumerate(zip(players, strategies, strict=True)): - outcome[player] = int(f"{pl_index}{strategy.label}") + payoffs = { + player: int(f"{pl_index}{strategy.label}") + for pl_index, (player, strategy) in enumerate(zip(players, strategies, strict=True)) + } + game.make_outcome(tuple(strategies), payoffs, f"c{n}") def test_strategic_game_set_strategies_drop_preserves_other_payoffs(): From c1023ed2b5fe8fb397ccc2d770534bec77601e4e Mon Sep 17 00:00:00 2001 From: drdkad Date: Wed, 26 Aug 2026 10:27:12 +0100 Subject: [PATCH 04/15] Update Changelog and api.rst --- ChangeLog | 8 ++++++++ doc/pygambit.api.rst | 1 + 2 files changed, 9 insertions(+) diff --git a/ChangeLog b/ChangeLog index 19c2093797..cb149a56cf 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,10 +4,18 @@ - Added `BehaviorSupportProfile` to `pygambit`. - Added callback functions to Nash solvers, allowing calling code to take actions when an equilibrium is found or another solver event is emitted. +- Added `Game.make_outcome`, which creates an outcome with the given payoffs and attaches it to a + set of nodes (extensive) or contingencies (strategic) in a single operation. (#1061) ### Changed - Revised `StrategySupportProfile` to expose a slimmer set of operations, and implement `StrategySupport` as an immutable snapshot. +- Absent outcomes are now represented by a per-game null outcome rather than by a null reference. + Two null outcomes always compare unequal (cf. missing values in `pandas`). Null outcomes do not appear when + iterating over a game's outcomes, and assigning a payoff to one raises `UndefinedOperationError`. (#1061) +- `Game.new_table` no longer creates an outcome for every contingency; a new strategic game has no outcomes, + and every contingency is initially null. Outcomes are created as they are needed; + games built by `Game.from_arrays` and `Game.from_dict` are unaffected. (#1061) ## [17.0.0-alpha.2] - 2026-08-21 diff --git a/doc/pygambit.api.rst b/doc/pygambit.api.rst index 8c827754e3..af59b7f245 100644 --- a/doc/pygambit.api.rst +++ b/doc/pygambit.api.rst @@ -71,6 +71,7 @@ Transforming game information structure :toctree: api/ Game.make_infoset + Game.make_outcome Game.make_event Game.relabel_actions Game.set_move_actions From c0bc3949c9da52f608b7d7f00f940d07119ccad4 Mon Sep 17 00:00:00 2001 From: drdkad Date: Wed, 26 Aug 2026 14:09:51 +0100 Subject: [PATCH 05/15] add make_outcome tests --- tests/test_outcomes.py | 56 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/test_outcomes.py b/tests/test_outcomes.py index e0d3b67e47..40b2e819d4 100644 --- a/tests/test_outcomes.py +++ b/tests/test_outcomes.py @@ -14,6 +14,62 @@ def test_outcome_add(game: gbt.Game): assert len(game.outcomes) == outcome_count + 1 +def test_make_outcome_attaches_to_all_given_nodes(): + game = gbt.Game.new_tree(["Alice", "Bob"]) + game.append_move(game.root, "Alice", ["U", "M", "D"]) + up, middle, down = game.root.children + outcome = game.make_outcome([up, middle], {"Alice": 1, "Bob": -1}, "shared") + assert up.outcome == outcome + assert middle.outcome == outcome + assert not down.outcome + assert outcome["Alice"] == 1 + assert outcome["Bob"] == -1 + + +def test_make_outcome_attaches_at_contingencies(): + game = gbt.Game.new_table([2, 2]) + outcome = game.make_outcome([(0, 0), (1, 1)], {"1": 2, "2": -2}, "diagonal") + assert game[0, 0] == outcome + assert game[1, 1] == outcome + assert not game[0, 1] + assert outcome["1"] == 2 + + +def test_make_outcome_absorbs_fully_covered_outcome_and_reuses_label(): + game = gbt.Game.new_tree(["Alice"]) + game.append_move(game.root, "Alice", ["U", "D"]) + up, down = game.root.children + game.make_outcome(up, {"Alice": 1}, "w") + game.make_outcome([up, down], {"Alice": 2}, "w") + assert [(o.label, o["Alice"]) for o in game.outcomes] == [("w", 2)] + + +def test_make_outcome_label_of_partially_covered_outcome_refused(): + game = gbt.Game.new_tree(["Alice"]) + game.append_move(game.root, "Alice", ["U", "M", "D"]) + up, middle, down = game.root.children + game.make_outcome([up, middle], {"Alice": 1}, "w") + with pytest.raises(ValueError): + game.make_outcome(down, {"Alice": 2}, "w") + assert len(game.outcomes) == 1 + + +def test_make_outcome_incomplete_payoffs_raises(): + game = gbt.Game.new_tree(["Alice", "Bob"]) + game.append_move(game.root, "Alice", ["U", "D"]) + with pytest.raises(ValueError): + game.make_outcome(next(iter(game.root.children)), {"Alice": 1}, "w") + + +def test_make_outcome_payoffs_naming_player_twice_raises(): + game = gbt.Game.new_tree(["Alice", "Bob"]) + game.append_move(game.root, "Alice", ["U", "D"]) + alice = game.players["Alice"] + with pytest.raises(ValueError): + game.make_outcome(next(iter(game.root.children)), + {"Alice": 1, alice: 2, "Bob": 0}, "w") + + @pytest.mark.parametrize( "game", [gbt.Game.from_arrays([[0, 0], [0, 0]], [[0, 0], [0, 0]])] ) From bb3fcffb702a8ddee4fbe684aaa18db6ff0a1a3b Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Wed, 26 Aug 2026 16:24:59 +0100 Subject: [PATCH 06/15] Make sure we never return a null pointer in PureBehaviorProfile::GetOutcome(). --- src/games/behavpure.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/games/behavpure.cc b/src/games/behavpure.cc index 72ce677f80..ac71898b0c 100644 --- a/src/games/behavpure.cc +++ b/src/games/behavpure.cc @@ -85,7 +85,7 @@ GameOutcome PureBehaviorProfile::GetOutcome(const GameNode &p_start) const outcome = node->GetOutcome(); } if (node->IsTerminal()) { - return outcome; + return outcome ? outcome : node->GetOutcome(); } if (node->GetInfoset()->IsChanceInfoset()) { throw UndefinedException(); From a329e740a92e0b03cb61e10757078919fbad34f2 Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Wed, 26 Aug 2026 16:38:04 +0100 Subject: [PATCH 07/15] Eliminate duplication in outcome housekeeping --- src/games/game.h | 15 +++++++++++++++ src/games/gametable.cc | 17 ++--------------- src/games/gametree.cc | 17 ++--------------- 3 files changed, 19 insertions(+), 30 deletions(-) diff --git a/src/games/game.h b/src/games/game.h index f54fad7221..0492876088 100644 --- a/src/games/game.h +++ b/src/games/game.h @@ -326,6 +326,7 @@ inline void CheckText(const std::string &p_text) /// This class represents an outcome in a game. An outcome /// specifies a vector of payoffs to players. class GameOutcomeRep : public std::enable_shared_from_this { + friend class GameRep; friend class GameExplicitRep; friend class GameTreeRep; friend class GameTableRep; @@ -946,6 +947,8 @@ class GameRep : public std::enable_shared_from_this { /// Validate that p_label is a nonempty, valid, unique label for an outcome of this game. void CheckOutcomeLabel(const std::string &p_label, const std::set &p_ignore = {}) const; + /// Invalidate and remove the given outcomes from the game, renumbering the survivors. + void EraseOutcomes(const std::set &p_outcomes); //@} /// Hooks for derived classes to update lazily-computed orderings if required @@ -1653,6 +1656,18 @@ inline void GameRep::CheckOutcomeLabel(const std::string &p_label, } } } +inline void GameRep::EraseOutcomes(const std::set &p_outcomes) +{ + for (const auto *outcome : p_outcomes) { + auto member = std::find_if(m_outcomes.begin(), m_outcomes.end(), + [outcome](const auto &c) { return c.get() == outcome; }); + (*member)->Invalidate(); + m_outcomes.erase(member); + } + std::for_each( + m_outcomes.begin(), m_outcomes.end(), + [outc = 1](const std::shared_ptr &c) mutable { c->m_number = outc++; }); +} inline bool GameInfosetRep::IsChanceInfoset() const { return m_player->IsChance(); } inline Game GamePlayerRep::GetGame() const { return m_game->shared_from_this(); } diff --git a/src/games/gametable.cc b/src/games/gametable.cc index c556294f0d..9a935fb366 100644 --- a/src/games/gametable.cc +++ b/src/games/gametable.cc @@ -538,15 +538,7 @@ GameTableRep::MakeOutcome(const std::vector> &p_contin m_results[index] = outcome.get(); } if (!absorbed.empty()) { - for (const auto *dead : absorbed) { - auto member = std::find_if(m_outcomes.begin(), m_outcomes.end(), - [dead](const auto &c) { return c.get() == dead; }); - (*member)->Invalidate(); - m_outcomes.erase(member); - } - std::for_each( - m_outcomes.begin(), m_outcomes.end(), - [outc = 1](const std::shared_ptr &c) mutable { c->m_number = outc++; }); + EraseOutcomes(absorbed); } return outcome; } @@ -558,12 +550,7 @@ void GameTableRep::DeleteOutcome(const GameOutcome &p_outcome) } IncrementVersion(); std::replace(m_results.begin(), m_results.end(), p_outcome.get(), m_nullOutcome.get()); - m_outcomes.erase( - std::find(m_outcomes.begin(), m_outcomes.end(), std::shared_ptr(p_outcome))); - p_outcome->Invalidate(); - std::for_each( - m_outcomes.begin(), m_outcomes.end(), - [outc = 1](const std::shared_ptr &c) mutable { c->m_number = outc++; }); + EraseOutcomes({p_outcome.get()}); } //------------------------------------------------------------------------ diff --git a/src/games/gametree.cc b/src/games/gametree.cc index 873751b274..5f579fcd16 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -1880,15 +1880,7 @@ GameOutcome GameTreeRep::MakeOutcome(const std::vector &p_nodes, node->m_outcome = outcome.get(); } if (!absorbed.empty()) { - for (const auto *dead : absorbed) { - auto member = std::find_if(m_outcomes.begin(), m_outcomes.end(), - [dead](const auto &c) { return c.get() == dead; }); - (*member)->Invalidate(); - m_outcomes.erase(member); - } - std::for_each( - m_outcomes.begin(), m_outcomes.end(), - [outc = 1](const std::shared_ptr &c) mutable { c->m_number = outc++; }); + EraseOutcomes(absorbed); } return outcome; } @@ -1900,12 +1892,7 @@ void GameTreeRep::DeleteOutcome(const GameOutcome &p_outcome) } IncrementVersion(); m_root->DeleteOutcome(p_outcome.get()); - p_outcome->Invalidate(); - m_outcomes.erase( - std::find(m_outcomes.begin(), m_outcomes.end(), std::shared_ptr(p_outcome))); - std::for_each( - m_outcomes.begin(), m_outcomes.end(), - [outc = 1](const std::shared_ptr &c) mutable { c->m_number = outc++; }); + EraseOutcomes({p_outcome.get()}); } //------------------------------------------------------------------------ From 41ced25ee0f262340f9ec14f9f8fd8a5a472a439 Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Wed, 26 Aug 2026 16:41:02 +0100 Subject: [PATCH 08/15] Check for outcome->IsNull() and not just null-pointer. --- src/gui/efgdragdrop.cc | 6 +++--- src/gui/efgnodemenu.cc | 2 +- src/gui/efgtooltip.cc | 4 ++-- src/gui/gamedoc.cc | 8 ++++---- src/gui/nfgtable.cc | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/gui/efgdragdrop.cc b/src/gui/efgdragdrop.cc index 0288a8abc3..a1191104ac 100644 --- a/src/gui/efgdragdrop.cc +++ b/src/gui/efgdragdrop.cc @@ -86,7 +86,7 @@ bool TreeDropTarget::OnDropOutcome(const GameNode &p_node, const wxString &p_tex p_text.Right(p_text.Length() - 1).ToLong(&n); const GameNode srcNode = GetNode(m_model->GetGame()->GetRoot(), n); - if (!srcNode || srcNode == p_node || !srcNode->GetOutcome()) { + if (!srcNode || srcNode == p_node || srcNode->GetOutcome()->IsNull()) { return false; } @@ -296,7 +296,7 @@ bool EfgDisplay::ShowTreeDropMenu(const GameNode &p_targetNode, const GameNode & bool EfgDisplay::CanDropOutcomeOn(const GameNode &p_target, const GameNode &p_source) const { - return p_target && p_source && p_target != p_source && p_source->GetOutcome(); + return p_target && p_source && p_target != p_source && !p_source->GetOutcome()->IsNull(); } bool EfgDisplay::ShowOutcomeDropMenu(const GameNode &p_targetNode, const GameNode &p_sourceNode, @@ -372,7 +372,7 @@ void EfgDisplay::OnMouseMotion(wxMouseEvent &p_event) return; } - if ((hit.region == Outcome || hit.region == Payoff) && hit.node->GetOutcome()) { + if ((hit.region == Outcome || hit.region == Payoff) && !hit.node->GetOutcome()->IsNull()) { wxString label; label << "O" << hit.node->GetNumber(); wxTextDataObject textData(label); diff --git a/src/gui/efgnodemenu.cc b/src/gui/efgnodemenu.cc index 62ed030feb..c3d4308688 100644 --- a/src/gui/efgnodemenu.cc +++ b/src/gui/efgnodemenu.cc @@ -204,7 +204,7 @@ void EfgDisplay::UpdateNodeMenu(const GameNode &p_node) !m_doc->GetGame()->IsAbsentMinded(p_node->GetInfoset())); m_nodeMenu->Enable(GBT_MENU_EDIT_DELETE_TREE, p_node && !p_node->IsTerminal()); m_nodeMenu->Enable(GBT_MENU_EDIT_DELETE_PARENT, p_node && p_node->GetParent()); - m_nodeMenu->Enable(GBT_MENU_EDIT_REMOVE_OUTCOME, p_node && p_node->GetOutcome()); + m_nodeMenu->Enable(GBT_MENU_EDIT_REMOVE_OUTCOME, p_node && !p_node->GetOutcome()->IsNull()); m_nodeMenu->Enable(GBT_MENU_EDIT_NODE, static_cast(p_node)); m_nodeMenu->Enable(GBT_MENU_EDIT_MOVE, p_node && p_node->GetInfoset()); diff --git a/src/gui/efgtooltip.cc b/src/gui/efgtooltip.cc index 557ac64a3a..ac6c33957a 100644 --- a/src/gui/efgtooltip.cc +++ b/src/gui/efgtooltip.cc @@ -270,7 +270,7 @@ void OutcomeEditorPopup::LoadValues() { const GameOutcome outcome = m_node ? m_node->GetOutcome() : nullptr; - if (!outcome) { + if (!outcome || outcome->IsNull()) { // GameOutcome::NewOutcome() (called from Commit(), via DoSetOutcomeData) rejects an empty // or duplicate label outright -- pre-filling a fresh, unique one here means the dialog never // opens already showing that as an error the user has to notice and fix before they can @@ -330,7 +330,7 @@ void OutcomeEditorPopup::BeginEdit(const GameNode &p_node, int p_initialPlayer) // Like AppendMovePopup's "Append move"/"Insert move" title switch: which verb applies depends // on state BeginEdit() only just received, not anything fixed at construction. - SetTitle(m_node->GetOutcome() ? _("Edit outcome") : _("New outcome")); + SetTitle(!m_node->GetOutcome()->IsNull() ? _("Edit outcome") : _("New outcome")); LoadValues(); UpdateValidation(); // also does the Fit()/PositionPopup() a plain node change still needs diff --git a/src/gui/gamedoc.cc b/src/gui/gamedoc.cc index a23e259c84..ccb3990ca4 100644 --- a/src/gui/gamedoc.cc +++ b/src/gui/gamedoc.cc @@ -735,9 +735,9 @@ void GameDocument::DoSetOutcomeData(const GameNode &p_node, const wxString &p_la const std::string label = p_label.ToStdString(wxConvUTF8); GameOutcome outcome = p_node->GetOutcome(); - bool changed = !outcome; + bool changed = outcome->IsNull(); - if (outcome) { + if (!outcome->IsNull()) { changed = outcome->GetLabel() != label; if (!changed) { @@ -755,7 +755,7 @@ void GameDocument::DoSetOutcomeData(const GameNode &p_node, const wxString &p_la return; } - if (!outcome) { + if (outcome->IsNull()) { outcome = m_game->NewOutcome(p_label.ToStdString(wxConvUTF8)); m_game->SetOutcome(p_node, outcome); } @@ -772,7 +772,7 @@ void GameDocument::DoSetOutcomeData(const GameNode &p_node, const wxString &p_la void GameDocument::DoRemoveOutcome(GameNode p_node) { - if (!p_node || !p_node->GetOutcome()) { + if (!p_node || p_node->GetOutcome()->IsNull()) { return; } m_game->SetOutcome(p_node, nullptr); diff --git a/src/gui/nfgtable.cc b/src/gui/nfgtable.cc index f8747eb098..411e280c95 100644 --- a/src/gui/nfgtable.cc +++ b/src/gui/nfgtable.cc @@ -511,7 +511,7 @@ void TableWidget::SetPayoffCellValue(int row, int col, const wxString &value) { PureStrategyProfile profile = GetPayoffProfile(row, col); GameOutcome outcome = profile->GetOutcome(); - if (!outcome) { + if (outcome->IsNull()) { m_doc->DoNewOutcome(profile); profile = GetPayoffProfile(row, col); outcome = profile->GetOutcome(); From 6c736517fb193ae7751deca294198ca4b26c0a0f Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Wed, 26 Aug 2026 17:55:19 +0100 Subject: [PATCH 09/15] Added discussion of meaning of multi-outcome highlighting --- doc/gui.nfg.building.rst | 18 +++++++++++++++ src/gui/nfgpayoffgrid.cc | 49 +++++++++++++++++++++++++++++++++++++++- src/gui/nfgtable.cc | 15 ++++++++++++ src/gui/nfgtable.h | 11 +++++++++ 4 files changed, 92 insertions(+), 1 deletion(-) diff --git a/doc/gui.nfg.building.rst b/doc/gui.nfg.building.rst index 9739e9a459..3c442c93af 100644 --- a/doc/gui.nfg.building.rst +++ b/doc/gui.nfg.building.rst @@ -133,3 +133,21 @@ inputting the payoff for the first (row) player, pressing the :kbd:`Tab` key, inputting the payoff for the second (column) player, pressing the :kbd:`Tab` key, and so forth, until all the payoff entries in the table have been filled. + +Shared outcomes +^^^^^^^^^^^^^^^^ + +The payoffs shown at a contingency are stored as an *outcome*, and the +same outcome can be attached to more than one contingency -- most +commonly because several strategy profiles induce the same terminal +node in the reduced strategic game of an extensive game, but a +strategic game can also be built or loaded with outcomes shared this +way directly. Editing a payoff at any one of these contingencies +changes it at every other contingency sharing that outcome, since they +are, in fact, the same outcome. + +To see this grouping before editing, hover the mouse pointer over any +payoff cell: every other cell sharing its outcome is shown with a +shaded background for as long as the pointer remains over one of them. +The highlight does not change while a payoff is being edited, even if +the pointer moves elsewhere in the meantime. diff --git a/src/gui/nfgpayoffgrid.cc b/src/gui/nfgpayoffgrid.cc index 55c2d6fb03..253dac0dcf 100644 --- a/src/gui/nfgpayoffgrid.cc +++ b/src/gui/nfgpayoffgrid.cc @@ -34,6 +34,9 @@ namespace Gambit::GUI { +/// Background for every payoff cell sharing the outcome currently under the mouse. +static const wxColour kHoverOutcomeBg(220, 240, 234); + //========================================================================= // class PayoffCellRenderer //========================================================================= @@ -179,7 +182,8 @@ wxGridCellAttr *PayoffTable::GetAttr(int row, int col, wxGridCellAttr::wxAttrKin // header cell, not empty space. attr->SetOverflow(false); attr->SetTextColour(m_table->GetPlayerColor(m_table->GetPayoffPlayerForColumn(col))); - attr->SetBackgroundColour(*wxWHITE); + attr->SetBackgroundColour(m_table->IsPayoffCellHighlighted(row, col) ? kHoverOutcomeBg + : *wxWHITE); m_renderer->IncRef(); attr->SetRenderer(m_renderer); m_editor->IncRef(); @@ -250,6 +254,9 @@ class PayoffGrid final : public TableGridBase { void OnCharHook(wxKeyEvent &); void HandleTabTraversal(wxKeyEvent &); void MoveEditorByTab(bool p_backwards); + /// Highlight every cell sharing the outcome under the mouse + void OnMotion(wxMouseEvent &); + void OnLeaveWindow(wxMouseEvent &); //@} public: @@ -265,6 +272,46 @@ PayoffGrid::PayoffGrid(TableWidget *p_parent) Bind(wxEVT_KEY_DOWN, &PayoffGrid::OnKeyDown, this); Bind(wxEVT_CHAR_HOOK, &PayoffGrid::OnCharHook, this); + GetGridWindow()->Bind(wxEVT_MOTION, &PayoffGrid::OnMotion, this); + GetGridWindow()->Bind(wxEVT_LEAVE_WINDOW, &PayoffGrid::OnLeaveWindow, this); +} + +void PayoffGrid::OnMotion(wxMouseEvent &p_event) +{ + // While a cell is being edited, the highlight is pinned to whatever outcome it showed + // when editing began -- the mouse may wander (e.g. to check another part of the table) + // without that being taken as "now hovering something else". + if (IsCellEditControlShown()) { + p_event.Skip(); + return; + } + const wxGridCellCoords coords = XYToCell(p_event.GetPosition()); + if (coords.GetRow() >= 0 && coords.GetCol() >= 0) { + m_table->SetHoverOutcome( + m_table->GetPayoffProfile(coords.GetRow(), coords.GetCol())->GetOutcome()); + } + else { + m_table->SetHoverOutcome(GameOutcome()); + } + p_event.Skip(); +} + +void PayoffGrid::OnLeaveWindow(wxMouseEvent &p_event) +{ + if (IsCellEditControlShown()) { + p_event.Skip(); + return; + } + // A cell editor is a real child window created directly over the current cell (by + // EnableCellEditControl(), including the deferred one MoveEditorByTab() creates for + // the next cell); on at least macOS/Cocoa, that can itself generate a leave-window + // event on the grid window underneath even though the pointer never moved. Only + // clear the highlight if the pointer has genuinely left the grid's screen area. + const wxRect windowRect(GetGridWindow()->GetScreenPosition(), GetGridWindow()->GetSize()); + if (!windowRect.Contains(wxGetMousePosition())) { + m_table->SetHoverOutcome(GameOutcome()); + } + p_event.Skip(); } void PayoffGrid::MoveEditorByTab(bool p_backwards) diff --git a/src/gui/nfgtable.cc b/src/gui/nfgtable.cc index 411e280c95..4eb47fe4bc 100644 --- a/src/gui/nfgtable.cc +++ b/src/gui/nfgtable.cc @@ -546,4 +546,19 @@ GameStrategy TableWidget::GetStrategyByPlayerAndIndex(int player, int strategy) auto strategies = GetSupport().GetStrategies(GetSupport().GetGame()->GetPlayer(player)); return *std::next(strategies.begin(), strategy - 1); } + +void TableWidget::SetHoverOutcome(const GameOutcome &p_outcome) +{ + const GameOutcome outcome = (p_outcome && !p_outcome->IsNull()) ? p_outcome : GameOutcome(); + if (m_hoverOutcome == outcome) { + return; + } + m_hoverOutcome = outcome; + m_payoffGrid->Refresh(); +} + +bool TableWidget::IsPayoffCellHighlighted(int row, int col) const +{ + return m_hoverOutcome && GetPayoffProfile(row, col)->GetOutcome() == m_hoverOutcome; +} } // namespace Gambit::GUI diff --git a/src/gui/nfgtable.h b/src/gui/nfgtable.h index 28a6ff7f08..ec489ab9bc 100644 --- a/src/gui/nfgtable.h +++ b/src/gui/nfgtable.h @@ -289,6 +289,10 @@ class TableWidget final : public wxPanel { std::shared_ptr m_layout; + /// The outcome currently under the mouse in the payoff grid, or the null handle if + /// none (including when the cell under the mouse has the game's null outcome). + GameOutcome m_hoverOutcome; + /// @name Event handlers //@{ /// Called when the payoff grid is scrolled; keeps row/col header grids in sync @@ -449,6 +453,13 @@ class TableWidget final : public wxPanel { int GetPayoffColumnsPerContingency() const; bool IsPayoffStrategyDominated(int row, int col, bool strict) const; + /// Sets the outcome to highlight in the payoff grid (every cell sharing it), e.g. as + /// the mouse moves over the grid; pass the null handle (or the game's null outcome) to + /// clear the highlight. Repaints the payoff grid if the highlighted outcome changes. + void SetHoverOutcome(const GameOutcome &p_outcome); + /// Whether the given cell's outcome is the one currently set by SetHoverOutcome. + bool IsPayoffCellHighlighted(int row, int col) const; + //@} bool IsReadOnly() const; From 3b018ea26e4ece66436cc005bb96b67919a2b450 Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Wed, 26 Aug 2026 19:13:34 +0100 Subject: [PATCH 10/15] Add mechanism for merging/removing outcomes in strategic form --- doc/gui.nfg.building.rst | 33 ++++ src/gui/gamedoc.cc | 26 +++ src/gui/gamedoc.h | 3 + src/gui/nfgpayoffgrid.cc | 348 +++++++++++++++++++++++++++++++++++++++ src/gui/nfgtable.h | 12 +- 5 files changed, 416 insertions(+), 6 deletions(-) diff --git a/doc/gui.nfg.building.rst b/doc/gui.nfg.building.rst index 3c442c93af..e096c57f72 100644 --- a/doc/gui.nfg.building.rst +++ b/doc/gui.nfg.building.rst @@ -151,3 +151,36 @@ payoff cell: every other cell sharing its outcome is shown with a shaded background for as long as the pointer remains over one of them. The highlight does not change while a payoff is being edited, even if the pointer moves elsewhere in the meantime. + +Merging and splitting outcomes +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Which contingencies share an outcome can itself be changed from the +table. Click and drag, or shift-click, across several payoff cells to +select them; the selection automatically expands to cover every +payoff column of any contingency it touches, since an outcome is +shared at the level of a whole contingency, not a single payoff cell. +Right-clicking the selection -- or, with nothing selected, +right-clicking a single cell -- opens a menu with two commands: + ++ :guilabel:`Set outcome...` (labelled :guilabel:`Merge into one + outcome...` when more than one contingency is selected) opens a + dialog to give a label and a payoff for each player, then attaches + the resulting outcome to every selected contingency. Selecting + several contingencies this way merges them into one shared outcome; + selecting a single contingency that is currently part of a larger + shared outcome splits it off into its own outcome, leaving the rest + of the group unchanged. If the selection already matches one + outcome's full membership exactly, this instead edits that outcome's + own label and payoffs in place, rather than creating a new one. + The dialog starts from a label and payoffs to edit -- the shared + outcome's own, if editing one in place, or the first selected + contingency's current payoffs with a freshly generated label + otherwise -- and disables :guilabel:`OK` until the label is + nonempty and unique among the game's outcomes and every payoff is a + valid number. ++ :guilabel:`Remove outcome` clears every selected contingency back to + having no outcome. It is disabled if none of the selection currently + has one. + +Clicking any cell afterward clears the selection. diff --git a/src/gui/gamedoc.cc b/src/gui/gamedoc.cc index ccb3990ca4..a9b0cee8d8 100644 --- a/src/gui/gamedoc.cc +++ b/src/gui/gamedoc.cc @@ -779,6 +779,32 @@ void GameDocument::DoRemoveOutcome(GameNode p_node) NotifyChanged(GameModificationType::GamePayoffs); } +void GameDocument::DoRemoveOutcome(const PureStrategyProfile &p_profile) +{ + if (p_profile->GetOutcome()->IsNull()) { + return; + } + p_profile->SetOutcome(nullptr); + NotifyChanged(GameModificationType::GamePayoffs); +} + +void GameDocument::DoMakeOutcome(const std::vector &p_profiles, + const std::vector &p_payoffs, const std::string &p_label) +{ + std::vector> contingencies; + contingencies.reserve(p_profiles.size()); + for (const auto &profile : p_profiles) { + std::vector strategies; + strategies.reserve(m_game->NumPlayers()); + for (const auto &player : m_game->GetPlayers()) { + strategies.push_back(profile->GetStrategy(player)); + } + contingencies.push_back(strategies); + } + m_game->MakeOutcome(contingencies, p_payoffs, p_label); + NotifyChanged(GameModificationType::GamePayoffs); +} + void GameDocument::DoSetPayoff(GameOutcome p_outcome, int p_player, const wxString &p_value) { p_outcome->SetPayoff(m_game->GetPlayer(p_player), Number(p_value.ToStdString())); diff --git a/src/gui/gamedoc.h b/src/gui/gamedoc.h index 896e2cf57d..d38e0fe17a 100644 --- a/src/gui/gamedoc.h +++ b/src/gui/gamedoc.h @@ -393,6 +393,9 @@ class GameDocument { void DoSetOutcomeData(const GameNode &p_node, const wxString &p_label, const std::vector &p_payoffs); void DoRemoveOutcome(GameNode p_node); + void DoRemoveOutcome(const PureStrategyProfile &p_profile); + void DoMakeOutcome(const std::vector &p_profiles, + const std::vector &p_payoffs, const std::string &p_label); void DoSetPayoff(GameOutcome p_outcome, int p_player, const wxString &p_value); void DoAnalysisOutputChanged(); diff --git a/src/gui/nfgpayoffgrid.cc b/src/gui/nfgpayoffgrid.cc index 253dac0dcf..a5ee2bb06b 100644 --- a/src/gui/nfgpayoffgrid.cc +++ b/src/gui/nfgpayoffgrid.cc @@ -20,6 +20,9 @@ // Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. // +#include +#include + #include #ifndef WX_PRECOMP #include @@ -29,6 +32,8 @@ #include "renratio.h" // special renderer/editor for rational numbers +#include "dlexcept.h" +#include "editlabel.h" #include "gamedoc.h" #include "nfgtable.h" @@ -36,6 +41,9 @@ namespace Gambit::GUI { /// Background for every payoff cell sharing the outcome currently under the mouse. static const wxColour kHoverOutcomeBg(220, 240, 234); +/// Background for the cells currently selected for a "Set outcome" action -- distinct +/// from the hover highlight above, and from every player's payoff colour. +static const wxColour kSelectionBg(224, 224, 224); //========================================================================= // class PayoffCellRenderer @@ -239,6 +247,177 @@ void PayoffTable::OnUpdate() } } +//========================================================================= +// class SetOutcomeDialog +//========================================================================= + +namespace { +const wxColour kInvalidFieldBg(255, 220, 220); +} // namespace + +//! +//! Prompts for a label and one payoff per player, to create the outcome attached to a +//! set of contingencies -- whether that's merging several into one (a multi-contingency +//! selection), splitting one off from a shared outcome (a single contingency out of a +//! larger group), or just editing a shared outcome's own label/payoffs in place (a +//! selection that already exactly matches one outcome's membership). +//! +class SetOutcomeDialog final : public wxDialog { + struct Row { + GamePlayer player; + wxTextCtrl *payoffCtrl; + }; + + const Game m_game; + const std::set m_ignoreForLabel; + LabelTextCtrl *m_labelCtrl; + std::vector m_payoffRows; + wxStaticText *m_errorText; + wxButton *m_okButton; + wxColour m_defaultBg; + + //! + //! Re-validates every field, recolouring each to flag whether it is currently valid, + //! and returns a description of the first problem found (or an empty string if none). + //! + wxString ValidateFields() + { + const wxString label = m_labelCtrl->GetNormalizedValue(); + const bool labelEmpty = label.empty(); + bool labelDuplicate = false; + if (!labelEmpty) { + for (const auto &outcome : m_game->GetOutcomes()) { + if (!m_ignoreForLabel.contains(outcome.get()) && + wxString::FromUTF8(outcome->GetLabel()) == label) { + labelDuplicate = true; + break; + } + } + } + m_labelCtrl->SetBackgroundColour((labelEmpty || labelDuplicate) ? kInvalidFieldBg + : m_defaultBg); + m_labelCtrl->Refresh(); + + wxString message; + if (labelEmpty) { + message = _("Outcome label cannot be empty."); + } + else if (labelDuplicate) { + message = _("Outcome label must be unique."); + } + + for (const auto &row : m_payoffRows) { + bool invalid = false; + try { + lexical_cast(row.payoffCtrl->GetValue().ToStdString()); + } + catch (const std::exception &) { + invalid = true; + } + row.payoffCtrl->SetBackgroundColour(invalid ? kInvalidFieldBg : m_defaultBg); + row.payoffCtrl->Refresh(); + if (invalid && message.empty()) { + message = wxString::Format(_("Payoff for %s is not a valid number."), + wxString::FromUTF8(row.player->GetLabel())); + } + } + return message; + } + + void UpdateValidation() + { + const wxString message = ValidateFields(); + m_errorText->SetLabel(message); + m_errorText->Show(!message.empty()); + m_okButton->Enable(message.empty()); + Layout(); + Fit(); + } + + void OnOK(wxCommandEvent &p_event) + { + if (!ValidateFields().empty()) { + wxBell(); + UpdateValidation(); + return; + } + p_event.Skip(); + } + +public: + SetOutcomeDialog(wxWindow *p_parent, const Game &p_game, const wxString &p_title, + const wxString &p_label, const std::vector &p_payoffValues, + const std::set &p_ignoreForLabel) + : wxDialog(p_parent, wxID_ANY, p_title), m_game(p_game), m_ignoreForLabel(p_ignoreForLabel) + { + auto *topSizer = new wxBoxSizer(wxVERTICAL); + + auto *labelSizer = new wxBoxSizer(wxHORIZONTAL); + labelSizer->Add(new wxStaticText(this, wxID_ANY, _("Label")), 0, + wxALL | wxALIGN_CENTRE_VERTICAL, 5); + m_labelCtrl = new LabelTextCtrl(this, wxID_ANY, p_label); + labelSizer->Add(m_labelCtrl, 1, wxALL | wxEXPAND, 5); + topSizer->Add(labelSizer, 0, wxEXPAND | wxALL, 5); + + auto *payoffSizer = new wxFlexGridSizer(2, FromDIP(5), FromDIP(10)); + payoffSizer->AddGrowableCol(1, 1); + const auto players = p_game->GetPlayers(); + size_t index = 0; + for (const auto &player : players) { + wxString playerLabel = wxString::FromUTF8(player->GetLabel()); + if (playerLabel.empty()) { + playerLabel = wxString::Format(_("Player %d"), player->GetNumber()); + } + payoffSizer->Add(new wxStaticText(this, wxID_ANY, playerLabel), 0, wxALIGN_CENTRE_VERTICAL); + auto *ctrl = new wxTextCtrl(this, wxID_ANY, p_payoffValues.at(index)); + ctrl->Bind(wxEVT_TEXT, [this](wxCommandEvent &p_textEvent) { + UpdateValidation(); + p_textEvent.Skip(); + }); + payoffSizer->Add(ctrl, 1, wxEXPAND); + m_payoffRows.push_back({player, ctrl}); + ++index; + } + topSizer->Add(payoffSizer, 0, wxEXPAND | wxALL, 5); + + m_defaultBg = m_labelCtrl->GetBackgroundColour(); + + m_errorText = new wxStaticText(this, wxID_ANY, wxEmptyString); + m_errorText->SetForegroundColour(*wxRED); + m_errorText->Wrap(FromDIP(260)); + m_errorText->Hide(); + topSizer->Add(m_errorText, 0, wxEXPAND | wxALL, 5); + + auto *buttonSizer = CreateStdDialogButtonSizer(wxOK | wxCANCEL); + topSizer->Add(buttonSizer, 0, wxEXPAND | wxALL, 5); + + SetSizer(topSizer); + Fit(); + CentreOnParent(); + + m_labelCtrl->Bind(wxEVT_TEXT, [this](wxCommandEvent &p_textEvent) { + UpdateValidation(); + p_textEvent.Skip(); + }); + m_okButton = static_cast(FindWindow(wxID_OK)); + Bind(wxEVT_BUTTON, &SetOutcomeDialog::OnOK, this, wxID_OK); + + UpdateValidation(); + } + + wxString GetOutcomeLabel() const { return m_labelCtrl->GetNormalizedValue(); } + + std::vector GetPayoffs() const + { + std::vector payoffs; + payoffs.reserve(m_payoffRows.size()); + for (const auto &row : m_payoffRows) { + payoffs.emplace_back(row.payoffCtrl->GetValue().ToStdString()); + } + return payoffs; + } +}; + //========================================================================= // class PayoffGrid //========================================================================= @@ -246,6 +425,9 @@ void PayoffTable::OnUpdate() class PayoffGrid final : public TableGridBase { TableWidget *m_table; PayoffTable *m_gridTable; + // Guards OnRangeSelecting's own SelectBlock() call against re-entering itself, since + // that call fires another RANGE_SELECTING event in turn. + bool m_snappingSelection = false; /// @name Overriding wxGrid behavior for event-handling //@{ @@ -257,8 +439,28 @@ class PayoffGrid final : public TableGridBase { /// Highlight every cell sharing the outcome under the mouse void OnMotion(wxMouseEvent &); void OnLeaveWindow(wxMouseEvent &); + /// Allows range selection (unlike the header grids), snapped live as the drag/shift-click + /// proceeds to cover every payoff column of each contingency it touches -- so the visible + /// selection always matches what a "Set outcome" action on it would cover, selecting + /// being otherwise cell-grained, with no notion of a contingency. + void OnRangeSelecting(wxGridRangeSelectEvent &) override; + /// The base class's own click handling drives the cursor and cell editor, and swallows + /// the event outright (never reaching wxGrid's own default handling, which is what + /// would otherwise clear a range selection on a plain click) -- so a plain click here + /// must clear it itself, or nothing ever would. + void OnCellLeftClick(wxGridEvent &) override; + /// Offer to merge the selected contingencies into one outcome, split one off from a + /// shared outcome, or clear their outcomes entirely + void OnCellRightClick(wxGridEvent &); //@} + /// The distinct contingencies covered by the current selection, deduplicated across + /// the several payoff columns each contingency spans -- or just the given cell, if + /// nothing is currently selected (e.g. a right-click with no preceding drag/shift-click). + std::vector GetSelectedContingencies(int p_fallbackRow, + int p_fallbackCol) const; + void ShowSetOutcomeDialog(const std::vector &p_profiles); + public: explicit PayoffGrid(TableWidget *p_parent); @@ -269,13 +471,159 @@ PayoffGrid::PayoffGrid(TableWidget *p_parent) : TableGridBase(p_parent, wxID_ANY), m_table(p_parent), m_gridTable(new PayoffTable(p_parent)) { SetTable(m_gridTable, true); + SetSelectionBackground(kSelectionBg); + SetSelectionForeground(*wxBLACK); Bind(wxEVT_KEY_DOWN, &PayoffGrid::OnKeyDown, this); Bind(wxEVT_CHAR_HOOK, &PayoffGrid::OnCharHook, this); + Bind(wxEVT_GRID_CELL_RIGHT_CLICK, &PayoffGrid::OnCellRightClick, this); GetGridWindow()->Bind(wxEVT_MOTION, &PayoffGrid::OnMotion, this); GetGridWindow()->Bind(wxEVT_LEAVE_WINDOW, &PayoffGrid::OnLeaveWindow, this); } +void PayoffGrid::OnRangeSelecting(wxGridRangeSelectEvent &p_event) +{ + if (m_snappingSelection || !p_event.Selecting()) { + p_event.Skip(); + return; + } + + const int perContingency = m_table->GetPayoffColumnsPerContingency(); + const int leftCol = (p_event.GetLeftCol() / perContingency) * perContingency; + const int rightCol = ((p_event.GetRightCol() / perContingency) + 1) * perContingency - 1; + + if (leftCol == p_event.GetLeftCol() && rightCol == p_event.GetRightCol()) { + p_event.Skip(); + return; + } + + // The raw range wxGrid is about to apply doesn't align to contingency boundaries -- + // veto it, and apply the snapped range ourselves instead. wxGrid re-derives the next + // range from its own drag anchor and the pointer's current cell, not from whatever was + // last actually selected, so this doesn't disturb the drag once it's under way. + p_event.Veto(); + m_snappingSelection = true; + SelectBlock(p_event.GetTopRow(), leftCol, p_event.GetBottomRow(), rightCol, true); + m_snappingSelection = false; +} + +void PayoffGrid::OnCellLeftClick(wxGridEvent &p_event) +{ + ClearSelection(); + TableGridBase::OnCellLeftClick(p_event); +} + +std::vector PayoffGrid::GetSelectedContingencies(int p_fallbackRow, + int p_fallbackCol) const +{ + const int perContingency = m_table->GetPayoffColumnsPerContingency(); + std::set> blocks; // (row, contingency's column-block index) + + const wxGridCellCoordsArray topLeft = GetSelectionBlockTopLeft(); + const wxGridCellCoordsArray bottomRight = GetSelectionBlockBottomRight(); + for (size_t i = 0; i < topLeft.size(); ++i) { + for (int row = topLeft[i].GetRow(); row <= bottomRight[i].GetRow(); ++row) { + for (int col = topLeft[i].GetCol(); col <= bottomRight[i].GetCol(); ++col) { + blocks.emplace(row, col / perContingency); + } + } + } + for (const auto &coords : GetSelectedCells()) { + blocks.emplace(coords.GetRow(), coords.GetCol() / perContingency); + } + if (blocks.empty()) { + blocks.emplace(p_fallbackRow, p_fallbackCol / perContingency); + } + + std::vector profiles; + profiles.reserve(blocks.size()); + for (const auto &[row, colBlock] : blocks) { + profiles.push_back(m_table->GetPayoffProfile(row, colBlock * perContingency)); + } + return profiles; +} + +void PayoffGrid::ShowSetOutcomeDialog(const std::vector &p_profiles) +{ + const Game game = m_table->GetDocument()->GetGame(); + + // Does the selection already exactly match one existing outcome's membership? If so, + // this is really an in-place edit of that outcome, not a merge or split. + GameOutcome shared = p_profiles.front()->GetOutcome(); + for (const auto &profile : p_profiles) { + if (profile->GetOutcome() != shared) { + shared = GameOutcome(); + break; + } + } + const bool editingInPlace = shared && !shared->IsNull(); + + wxString label; + std::vector payoffValues; + std::set ignoreForLabel; + if (editingInPlace) { + label = wxString::FromUTF8(shared->GetLabel()); + ignoreForLabel.insert(shared.get()); + for (const auto &player : game->GetPlayers()) { + payoffValues.push_back(wxString::FromUTF8(shared->GetPayoff(player))); + } + } + else { + label = wxString::FromUTF8(GenerateOutcomeLabel(game)); + const GameOutcome seed = p_profiles.front()->GetOutcome(); + for (const auto &player : game->GetPlayers()) { + payoffValues.push_back(wxString::FromUTF8(seed->GetPayoff(player))); + } + } + + SetOutcomeDialog dialog(this, game, + p_profiles.size() > 1 ? _("Merge into one outcome") : _("Set outcome"), + label, payoffValues, ignoreForLabel); + if (dialog.ShowModal() != wxID_OK) { + return; + } + m_table->GetDocument()->DoMakeOutcome(p_profiles, dialog.GetPayoffs(), + dialog.GetOutcomeLabel().ToStdString()); + ClearSelection(); +} + +void PayoffGrid::OnCellRightClick(wxGridEvent &p_event) +{ + const std::vector profiles = + GetSelectedContingencies(p_event.GetRow(), p_event.GetCol()); + + const bool anyHasOutcome = + std::any_of(profiles.begin(), profiles.end(), [](const PureStrategyProfile &p_profile) { + return !p_profile->GetOutcome()->IsNull(); + }); + + const int setOutcomeId = wxWindow::NewControlId(); + const int removeOutcomeId = wxWindow::NewControlId(); + + wxMenu menu; + menu.Append(setOutcomeId, + profiles.size() > 1 ? _("Merge into one outcome...") : _("Set outcome...")); + menu.Append(removeOutcomeId, _("Remove outcome")); + menu.Enable(removeOutcomeId, anyHasOutcome); + + const int selection = GetPopupMenuSelectionFromUser(menu, p_event.GetPosition()); + + try { + if (selection == setOutcomeId) { + ShowSetOutcomeDialog(profiles); + } + else if (selection == removeOutcomeId) { + for (const auto &profile : profiles) { + m_table->GetDocument()->DoRemoveOutcome(profile); + } + ClearSelection(); + } + } + catch (std::exception &ex) { + ExceptionDialog(this, ex.what()).ShowModal(); + } +} + void PayoffGrid::OnMotion(wxMouseEvent &p_event) { // While a cell is being edited, the highlight is pinned to whatever outcome it showed diff --git a/src/gui/nfgtable.h b/src/gui/nfgtable.h index ec489ab9bc..984450fc27 100644 --- a/src/gui/nfgtable.h +++ b/src/gui/nfgtable.h @@ -43,13 +43,7 @@ class NfgPanel; //! file-local to one of them, since it's a base class used across all three .cc files. //! class TableGridBase : public wxGrid { - //! - //! @name Suppressing the built-in selection highlight & cursor rectangle - //! - //@{ - void OnRangeSelecting(wxGridRangeSelectEvent &p_event) { p_event.Veto(); } void OnSelectCell(wxGridEvent &p_event) { p_event.Skip(); } - //@} protected: /// Shows the editor on one click; overridden by the header grids to edit strategies @@ -61,6 +55,12 @@ class TableGridBase : public wxGrid { p_event.Skip(false); } + /// Suppresses the built-in selection highlight and cursor rectangle by default, since + /// most of the grids in this display have no use for a range selection at all. The + /// payoff grid overrides this to allow selecting several contingencies at once, to act + /// on together (e.g. merging them into one outcome). + virtual void OnRangeSelecting(wxGridRangeSelectEvent &p_event) { p_event.Veto(); } + public: explicit TableGridBase(wxWindow *p_parent, wxWindowID p_id = wxID_ANY) : wxGrid(p_parent, p_id) { From 21cae9add3aa76b676901af15577f980f0494a41 Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Wed, 26 Aug 2026 19:30:51 +0100 Subject: [PATCH 11/15] Add edit outcome in strategic games --- doc/gui.nfg.building.rst | 12 ++++++++- src/gui/gamedoc.cc | 14 +++++++++++ src/gui/gamedoc.h | 2 ++ src/gui/nfgpayoffgrid.cc | 53 +++++++++++++++++++++++++++++++++------- 4 files changed, 71 insertions(+), 10 deletions(-) diff --git a/doc/gui.nfg.building.rst b/doc/gui.nfg.building.rst index e096c57f72..791cd604cb 100644 --- a/doc/gui.nfg.building.rst +++ b/doc/gui.nfg.building.rst @@ -161,7 +161,7 @@ select them; the selection automatically expands to cover every payoff column of any contingency it touches, since an outcome is shared at the level of a whole contingency, not a single payoff cell. Right-clicking the selection -- or, with nothing selected, -right-clicking a single cell -- opens a menu with two commands: +right-clicking a single cell -- opens a menu with three commands: + :guilabel:`Set outcome...` (labelled :guilabel:`Merge into one outcome...` when more than one contingency is selected) opens a @@ -179,6 +179,16 @@ right-clicking a single cell -- opens a menu with two commands: otherwise -- and disables :guilabel:`OK` until the label is nonempty and unique among the game's outcomes and every payoff is a valid number. ++ :guilabel:`Edit outcome...` is the direct route to that same + in-place case: it is enabled whenever the selection matches one + existing outcome's full membership exactly (a single contingency is + enough, if that contingency's outcome has no other members), and + opens the same label-and-payoffs dialog pre-filled with that + outcome's own values, changing them in place without touching which + contingencies share it. Editing a payoff here is no different from + typing directly into a payoff cell -- it is offered as a second way + to reach the same result, alongside also being able to change the + label, matching how outcomes are edited in an extensive game. + :guilabel:`Remove outcome` clears every selected contingency back to having no outcome. It is disabled if none of the selection currently has one. diff --git a/src/gui/gamedoc.cc b/src/gui/gamedoc.cc index a9b0cee8d8..a4dac734e9 100644 --- a/src/gui/gamedoc.cc +++ b/src/gui/gamedoc.cc @@ -805,6 +805,20 @@ void GameDocument::DoMakeOutcome(const std::vector &p_profi NotifyChanged(GameModificationType::GamePayoffs); } +void GameDocument::DoSetOutcomeData(GameOutcome p_outcome, const wxString &p_label, + const std::vector &p_payoffs) +{ + if (p_payoffs.size() != m_game->NumPlayers()) { + throw std::invalid_argument("Incorrect number of payoff values"); + } + p_outcome->SetLabel(p_label.ToStdString(wxConvUTF8)); + for (size_t index = 0; index < p_payoffs.size(); ++index) { + p_outcome->SetPayoff(m_game->GetPlayer(static_cast(index) + 1), + Number(p_payoffs[index].ToStdString())); + } + NotifyChanged(GameModificationType::GamePayoffs); +} + void GameDocument::DoSetPayoff(GameOutcome p_outcome, int p_player, const wxString &p_value) { p_outcome->SetPayoff(m_game->GetPlayer(p_player), Number(p_value.ToStdString())); diff --git a/src/gui/gamedoc.h b/src/gui/gamedoc.h index d38e0fe17a..c3ac14349d 100644 --- a/src/gui/gamedoc.h +++ b/src/gui/gamedoc.h @@ -392,6 +392,8 @@ class GameDocument { void DoSetOutcome(GameNode p_node, GameOutcome p_outcome); void DoSetOutcomeData(const GameNode &p_node, const wxString &p_label, const std::vector &p_payoffs); + void DoSetOutcomeData(GameOutcome p_outcome, const wxString &p_label, + const std::vector &p_payoffs); void DoRemoveOutcome(GameNode p_node); void DoRemoveOutcome(const PureStrategyProfile &p_profile); void DoMakeOutcome(const std::vector &p_profiles, diff --git a/src/gui/nfgpayoffgrid.cc b/src/gui/nfgpayoffgrid.cc index a5ee2bb06b..3a4b3f4934 100644 --- a/src/gui/nfgpayoffgrid.cc +++ b/src/gui/nfgpayoffgrid.cc @@ -459,7 +459,11 @@ class PayoffGrid final : public TableGridBase { /// nothing is currently selected (e.g. a right-click with no preceding drag/shift-click). std::vector GetSelectedContingencies(int p_fallbackRow, int p_fallbackCol) const; + /// The one outcome shared by every given contingency, if they all have the same + /// non-null outcome -- the null handle otherwise (mixed outcomes, or none at all). + static GameOutcome GetSharedOutcome(const std::vector &p_profiles); void ShowSetOutcomeDialog(const std::vector &p_profiles); + void ShowEditOutcomeDialog(const GameOutcome &p_outcome); public: explicit PayoffGrid(TableWidget *p_parent); @@ -543,25 +547,29 @@ std::vector PayoffGrid::GetSelectedContingencies(int p_fall return profiles; } -void PayoffGrid::ShowSetOutcomeDialog(const std::vector &p_profiles) +GameOutcome PayoffGrid::GetSharedOutcome(const std::vector &p_profiles) { - const Game game = m_table->GetDocument()->GetGame(); - - // Does the selection already exactly match one existing outcome's membership? If so, - // this is really an in-place edit of that outcome, not a merge or split. GameOutcome shared = p_profiles.front()->GetOutcome(); for (const auto &profile : p_profiles) { if (profile->GetOutcome() != shared) { - shared = GameOutcome(); - break; + return GameOutcome(); } } - const bool editingInPlace = shared && !shared->IsNull(); + return (shared && !shared->IsNull()) ? shared : GameOutcome(); +} + +void PayoffGrid::ShowSetOutcomeDialog(const std::vector &p_profiles) +{ + const Game game = m_table->GetDocument()->GetGame(); + + // Does the selection already exactly match one existing outcome's membership? If so, + // this is really an in-place edit of that outcome, not a merge or split. + const GameOutcome shared = GetSharedOutcome(p_profiles); wxString label; std::vector payoffValues; std::set ignoreForLabel; - if (editingInPlace) { + if (shared) { label = wxString::FromUTF8(shared->GetLabel()); ignoreForLabel.insert(shared.get()); for (const auto &player : game->GetPlayers()) { @@ -587,6 +595,26 @@ void PayoffGrid::ShowSetOutcomeDialog(const std::vector &p_ ClearSelection(); } +void PayoffGrid::ShowEditOutcomeDialog(const GameOutcome &p_outcome) +{ + const Game game = m_table->GetDocument()->GetGame(); + std::vector payoffValues; + for (const auto &player : game->GetPlayers()) { + payoffValues.push_back(wxString::FromUTF8(p_outcome->GetPayoff(player))); + } + SetOutcomeDialog dialog(this, game, _("Edit outcome"), wxString::FromUTF8(p_outcome->GetLabel()), + payoffValues, {p_outcome.get()}); + if (dialog.ShowModal() != wxID_OK) { + return; + } + std::vector payoffs; + for (const auto &value : dialog.GetPayoffs()) { + payoffs.push_back(wxString::FromUTF8(lexical_cast(value))); + } + m_table->GetDocument()->DoSetOutcomeData(p_outcome, dialog.GetOutcomeLabel(), payoffs); + ClearSelection(); +} + void PayoffGrid::OnCellRightClick(wxGridEvent &p_event) { const std::vector profiles = @@ -596,13 +624,17 @@ void PayoffGrid::OnCellRightClick(wxGridEvent &p_event) std::any_of(profiles.begin(), profiles.end(), [](const PureStrategyProfile &p_profile) { return !p_profile->GetOutcome()->IsNull(); }); + const GameOutcome shared = GetSharedOutcome(profiles); const int setOutcomeId = wxWindow::NewControlId(); + const int editOutcomeId = wxWindow::NewControlId(); const int removeOutcomeId = wxWindow::NewControlId(); wxMenu menu; menu.Append(setOutcomeId, profiles.size() > 1 ? _("Merge into one outcome...") : _("Set outcome...")); + menu.Append(editOutcomeId, _("Edit outcome...")); + menu.Enable(editOutcomeId, static_cast(shared)); menu.Append(removeOutcomeId, _("Remove outcome")); menu.Enable(removeOutcomeId, anyHasOutcome); @@ -612,6 +644,9 @@ void PayoffGrid::OnCellRightClick(wxGridEvent &p_event) if (selection == setOutcomeId) { ShowSetOutcomeDialog(profiles); } + else if (selection == editOutcomeId) { + ShowEditOutcomeDialog(shared); + } else if (selection == removeOutcomeId) { for (const auto &profile : profiles) { m_table->GetDocument()->DoRemoveOutcome(profile); From 79f470c6a38c54dd143949f59babd9c3b2039d5e Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Wed, 26 Aug 2026 20:05:06 +0100 Subject: [PATCH 12/15] wx version compatibility --- src/gui/nfgpayoffgrid.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/gui/nfgpayoffgrid.cc b/src/gui/nfgpayoffgrid.cc index 3a4b3f4934..3224577e89 100644 --- a/src/gui/nfgpayoffgrid.cc +++ b/src/gui/nfgpayoffgrid.cc @@ -532,8 +532,9 @@ std::vector PayoffGrid::GetSelectedContingencies(int p_fall } } } - for (const auto &coords : GetSelectedCells()) { - blocks.emplace(coords.GetRow(), coords.GetCol() / perContingency); + const wxGridCellCoordsArray selectedCells = GetSelectedCells(); + for (size_t i = 0; i < selectedCells.size(); ++i) { + blocks.emplace(selectedCells[i].GetRow(), selectedCells[i].GetCol() / perContingency); } if (blocks.empty()) { blocks.emplace(p_fallbackRow, p_fallbackCol / perContingency); From 4010b9c0c942d581feb34fe998d24b22b2dcfe16 Mon Sep 17 00:00:00 2001 From: drdkad Date: Thu, 27 Aug 2026 10:47:48 +0100 Subject: [PATCH 13/15] Allow extending the payoff-grid selection with a modified click --- src/gui/nfgpayoffgrid.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/gui/nfgpayoffgrid.cc b/src/gui/nfgpayoffgrid.cc index 3224577e89..2fddca6ea4 100644 --- a/src/gui/nfgpayoffgrid.cc +++ b/src/gui/nfgpayoffgrid.cc @@ -513,6 +513,14 @@ void PayoffGrid::OnRangeSelecting(wxGridRangeSelectEvent &p_event) void PayoffGrid::OnCellLeftClick(wxGridEvent &p_event) { + if (p_event.ControlDown() || p_event.MetaDown()) { + const int per = m_table->GetPayoffColumnsPerContingency(); + const int left = (p_event.GetCol() / per) * per; + m_snappingSelection = true; + SelectBlock(p_event.GetRow(), left, p_event.GetRow(), left + per - 1, true); + m_snappingSelection = false; + return; + } ClearSelection(); TableGridBase::OnCellLeftClick(p_event); } From ef22cb5999fab7058c6b1dd52beab7a02d149046 Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Fri, 28 Aug 2026 10:19:24 +0100 Subject: [PATCH 14/15] Extend command/meta-click to be a toggle not just add cell. --- src/gui/nfgpayoffgrid.cc | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/gui/nfgpayoffgrid.cc b/src/gui/nfgpayoffgrid.cc index 2fddca6ea4..ea3e70fd5c 100644 --- a/src/gui/nfgpayoffgrid.cc +++ b/src/gui/nfgpayoffgrid.cc @@ -515,9 +515,21 @@ void PayoffGrid::OnCellLeftClick(wxGridEvent &p_event) { if (p_event.ControlDown() || p_event.MetaDown()) { const int per = m_table->GetPayoffColumnsPerContingency(); + const int row = p_event.GetRow(); const int left = (p_event.GetCol() / per) * per; + + // Selection is always block-aligned to whole contingencies (every payoff column of + // one, or none of them), so the first column of the block stands in for the whole + // block: toggle the block off if it's there, on otherwise. m_snappingSelection = true; - SelectBlock(p_event.GetRow(), left, p_event.GetRow(), left + per - 1, true); + if (IsInSelection(row, left)) { + for (int col = left; col < left + per; ++col) { + DeselectCell(row, col); + } + } + else { + SelectBlock(row, left, row, left + per - 1, true); + } m_snappingSelection = false; return; } From dff90dbd316f430c745542beb6d378e8b064552c Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Fri, 28 Aug 2026 13:29:14 +0100 Subject: [PATCH 15/15] Update CLI tests for new make_outcome --- tests/cli/conftest.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/cli/conftest.py b/tests/cli/conftest.py index 7c2cfa5b43..4351378df1 100644 --- a/tests/cli/conftest.py +++ b/tests/cli/conftest.py @@ -22,9 +22,9 @@ def _table_game(payoffs: dict, title: str) -> gbt.Game: s2a, s2b = p2.strategies strategies = {"a": s1a, "b": s1b, "A": s2a, "B": s2b} for (row, col), (v1, v2) in payoffs.items(): - outcome = game[strategies[row], strategies[col]] - outcome[p1] = v1 - outcome[p2] = v2 + game.make_outcome( + (strategies[row], strategies[col]), {p1: v1, p2: v2}, f"{row}{col}" + ) return game