From 21c3394e24e472972f79ca4248f4d9a290cb8fee Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 11:40:35 +0000 Subject: [PATCH 1/2] Fix confetti firing for non-unanimous votes (#16) Confetti celebrated whenever the non-pending votes that had arrived so far all matched. Because votes and the "show votes" signal are delivered independently over SignalR, a client could process the reveal before a differing vote (e.g. someone voting 1) arrived, see an apparently unanimous set, and fire confetti for a round that was not unanimous. Only celebrate once the round is fully settled: every player must have cast the same numeric vote. A pending vote (not voted yet, or still in transit) now blocks the celebration, so a late differing vote can no longer trigger a false positive. The check is re-evaluated on every render while votes are shown, so a unanimous result that settles just after the reveal still celebrates. Extracted the logic into a testable GameState.IsUnanimous extension and added unit tests covering the reported scenario. --- .../GameStateExtensionsTests.cs | 69 +++++++++++++++++++ PointingParty.Client/Components/GameUi.razor | 42 +++++------ PointingParty.Client/GameStateExtensions.cs | 21 ++++++ 3 files changed, 109 insertions(+), 23 deletions(-) diff --git a/PointingParty.Client.Tests/GameStateExtensionsTests.cs b/PointingParty.Client.Tests/GameStateExtensionsTests.cs index 1671d9b..195cb43 100644 --- a/PointingParty.Client.Tests/GameStateExtensionsTests.cs +++ b/PointingParty.Client.Tests/GameStateExtensionsTests.cs @@ -56,4 +56,73 @@ public void Returns_Default_When_Nobody_Voted(VoteStatus vote) Assert.Equal(default, state.AverageVote()); } + + [Fact] + public void IsUnanimous_When_Everyone_Voted_The_Same_Number() + { + var state = StateWith( + ("a", 2), + ("b", 2), + ("c", 2)); + + Assert.True(state.IsUnanimous()); + } + + [Fact] + public void Is_Not_Unanimous_When_A_Vote_Differs() + { + // The scenario reported in the issue: 2, 2, 2, 1 + var state = StateWith( + ("a", 2), + ("b", 2), + ("c", 2), + ("d", 1)); + + Assert.False(state.IsUnanimous()); + } + + [Theory] + [InlineData(VoteStatus.Pending)] + [InlineData(VoteStatus.Coffee)] + [InlineData(VoteStatus.Question)] + public void Is_Not_Unanimous_When_A_Player_Has_Not_Cast_A_Numeric_Vote(VoteStatus vote) + { + // A pending vote may still be in transit, so an apparently unanimous + // result must not celebrate until every player has settled on a number. + var state = StateWith( + ("a", 2), + ("b", 2), + ("c", vote)); + + Assert.False(state.IsUnanimous()); + } + + [Theory] + [InlineData(VoteStatus.Pending)] + [InlineData(VoteStatus.Coffee)] + [InlineData(VoteStatus.Question)] + public void Is_Not_Unanimous_When_Nobody_Cast_A_Numeric_Vote(VoteStatus vote) + { + var state = StateWith( + ("a", vote), + ("b", vote)); + + Assert.False(state.IsUnanimous()); + } + + [Fact] + public void Is_Not_Unanimous_With_A_Single_Player() + { + var state = StateWith(("a", 2)); + + Assert.False(state.IsUnanimous()); + } + + private static GameState StateWith(params (string Player, Vote Vote)[] votes) + { + return new GameState( + string.Empty, + votes.ToImmutableDictionary(x => x.Player, x => x.Vote), + true); + } } diff --git a/PointingParty.Client/Components/GameUi.razor b/PointingParty.Client/Components/GameUi.razor index 20b3ede..31d91ab 100644 --- a/PointingParty.Client/Components/GameUi.razor +++ b/PointingParty.Client/Components/GameUi.razor @@ -98,7 +98,7 @@ private GameAggregate Game => GameContext.Game ?? new GameAggregate(); - private bool _previousShowVotes; + private bool _confettiShown; protected override void OnInitialized() { @@ -108,39 +108,35 @@ protected override async Task OnAfterRenderAsync(bool firstRender) { - if (Game.State.ShowVotes && !_previousShowVotes) + if (!Game.State.ShowVotes) { - _previousShowVotes = true; - await TriggerConfettiIfUnanimous(); + _confettiShown = false; + return; } - else if (!Game.State.ShowVotes) + + // Re-evaluate on every render while votes are shown (not just the reveal + // transition) so a unanimous result that only settles once a late vote + // arrives still celebrates, while a differing late vote never does. + if (!_confettiShown && Game.State.IsUnanimous()) { - _previousShowVotes = false; + _confettiShown = true; + await TriggerConfetti(); } } private static readonly string[] ConfettiShapes = ["circle", "square", "star"]; private static readonly Random Random = new(); - private async Task TriggerConfettiIfUnanimous() + private async Task TriggerConfetti() { - var nonPendingVotes = Game.State.PlayerVotes.Values - .Where(v => v.Status != VoteStatus.Pending) - .ToList(); - - if (nonPendingVotes.Count > 1 && nonPendingVotes.All(v => v.Equals(nonPendingVotes[0]))) + var shape = ConfettiShapes[Random.Next(ConfettiShapes.Length)]; + await JS.InvokeVoidAsync("confetti", new { - var totalPlayers = Game.State.PlayerVotes.Count; - var particleCount = (int)(30 + 170 * (nonPendingVotes.Count / (double)totalPlayers)); - var shape = ConfettiShapes[Random.Next(ConfettiShapes.Length)]; - await JS.InvokeVoidAsync("confetti", new - { - particleCount, - spread = 80, - origin = new { y = 0.8 }, - shapes = new[] { shape } - }); - } + particleCount = 200, + spread = 80, + origin = new { y = 0.8 }, + shapes = new[] { shape } + }); } private async Task Vote(double score) diff --git a/PointingParty.Client/GameStateExtensions.cs b/PointingParty.Client/GameStateExtensions.cs index 337aa04..b1afd1f 100644 --- a/PointingParty.Client/GameStateExtensions.cs +++ b/PointingParty.Client/GameStateExtensions.cs @@ -12,4 +12,25 @@ public static double AverageVote(this GameState gameState) .ToList(); return scoredVotes.Any() ? scoredVotes.Average() : default; } + + /// + /// Returns true only when the round is fully settled and everyone agrees: + /// there is more than one player and every player has cast the same numeric + /// vote. A pending vote means a player has not voted yet or their vote is + /// still in transit, so the result is not settled and we must not celebrate + /// prematurely (which would otherwise fire confetti for a non-unanimous + /// round whenever a differing vote arrives just after the reveal). + /// + public static bool IsUnanimous(this GameState gameState) + { + if (gameState.PlayerVotes.Count < 2) + return false; + + var votes = gameState.PlayerVotes.Values; + if (votes.Any(v => v.Status != VoteStatus.Scored)) + return false; + + var first = votes.First(); + return votes.All(v => v.Equals(first)); + } } From 5c049db5d62a352542d0f4b676dcec011340a319 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 11:48:28 +0000 Subject: [PATCH 2/2] Decide confetti at reveal and lock votes after showing (#16) Make the celebration decision authoritative instead of letting each client re-evaluate against its own, possibly incomplete, state: - The player who reveals the votes computes unanimity (which they can do reliably since they have the full picture) and the result rides along on the VotesShown event as a ShowConfetti flag. Every client simply honours that flag, so confetti can no longer fire on one client for a round that was not unanimous. - VoteCast events are now dropped once votes have been shown, so a late or post-reveal vote can't change a locked round. IsUnanimous goes back to ignoring abstentions (no vote / Coffee / Question): with the decision made once by the revealer there is no race to guard against, so abstaining no longer suppresses the celebration. Added GameAggregate tests for dropping post-reveal votes and for the ShowConfetti flag, and updated the unanimity tests accordingly. --- .../GameAggregateTests.cs | 47 +++++++++++++++++++ .../GameStateExtensionsTests.cs | 25 ++++++++-- PointingParty.Client/Components/GameUi.razor | 10 ++-- PointingParty.Client/GameStateExtensions.cs | 24 ++++------ PointingParty.Domain/Events/VotesShown.cs | 2 +- PointingParty.Domain/GameAggregate.cs | 13 +++-- PointingParty.Domain/GameState.cs | 3 +- 7 files changed, 95 insertions(+), 29 deletions(-) create mode 100644 PointingParty.Client.Tests/GameAggregateTests.cs diff --git a/PointingParty.Client.Tests/GameAggregateTests.cs b/PointingParty.Client.Tests/GameAggregateTests.cs new file mode 100644 index 0000000..3b3ca62 --- /dev/null +++ b/PointingParty.Client.Tests/GameAggregateTests.cs @@ -0,0 +1,47 @@ +using PointingParty.Domain; +using PointingParty.Domain.Events; + +namespace PointingParty.Client.Tests; + +public class GameAggregateTests +{ + private const string GameId = "TestGame"; + private const string PlayerName = "Player"; + + [Fact] + public void Votes_Cast_After_Reveal_Are_Dropped() + { + var game = new GameAggregate(GameId, PlayerName); + game.Handle(new PlayerJoinedGame(GameId, "Other")); + game.Handle(new VoteCast(GameId, "Other", 2)); + game.Handle(new VotesShown(GameId)); + + // A vote that arrives after the round was revealed must not change it. + game.Handle(new VoteCast(GameId, "Other", 5)); + + Assert.Equal(new Vote(2), game.State.PlayerVotes["Other"]); + } + + [Fact] + public void VotesShown_Carries_ShowConfetti_Flag() + { + var game = new GameAggregate(GameId, PlayerName); + + game.Handle(new VotesShown(GameId, ShowConfetti: true)); + + Assert.True(game.State.ShowVotes); + Assert.True(game.State.ShowConfetti); + } + + [Fact] + public void Reset_Clears_ShowConfetti() + { + var game = new GameAggregate(GameId, PlayerName); + game.Handle(new VotesShown(GameId, ShowConfetti: true)); + + game.Handle(new GameReset(GameId)); + + Assert.False(game.State.ShowVotes); + Assert.False(game.State.ShowConfetti); + } +} diff --git a/PointingParty.Client.Tests/GameStateExtensionsTests.cs b/PointingParty.Client.Tests/GameStateExtensionsTests.cs index 195cb43..c7f0257 100644 --- a/PointingParty.Client.Tests/GameStateExtensionsTests.cs +++ b/PointingParty.Client.Tests/GameStateExtensionsTests.cs @@ -85,14 +85,31 @@ public void Is_Not_Unanimous_When_A_Vote_Differs() [InlineData(VoteStatus.Pending)] [InlineData(VoteStatus.Coffee)] [InlineData(VoteStatus.Question)] - public void Is_Not_Unanimous_When_A_Player_Has_Not_Cast_A_Numeric_Vote(VoteStatus vote) + public void Ignores_Abstaining_Players(VoteStatus abstention) { - // A pending vote may still be in transit, so an apparently unanimous - // result must not celebrate until every player has settled on a number. + // Players who did not cast a numeric vote are ignored, so the remaining + // numeric votes still count as unanimous. var state = StateWith( ("a", 2), ("b", 2), - ("c", vote)); + ("c", abstention)); + + Assert.True(state.IsUnanimous()); + } + + [Theory] + [InlineData(VoteStatus.Pending)] + [InlineData(VoteStatus.Coffee)] + [InlineData(VoteStatus.Question)] + public void Is_Not_Unanimous_When_A_Numeric_Vote_Differs_Despite_An_Abstention(VoteStatus abstention) + { + // The scenario reported in the issue: 2, 2, 2, 1, abstained + var state = StateWith( + ("a", 2), + ("b", 2), + ("c", 2), + ("d", 1), + ("e", abstention)); Assert.False(state.IsUnanimous()); } diff --git a/PointingParty.Client/Components/GameUi.razor b/PointingParty.Client/Components/GameUi.razor index 31d91ab..fd520ef 100644 --- a/PointingParty.Client/Components/GameUi.razor +++ b/PointingParty.Client/Components/GameUi.razor @@ -114,10 +114,10 @@ return; } - // Re-evaluate on every render while votes are shown (not just the reveal - // transition) so a unanimous result that only settles once a late vote - // arrives still celebrates, while a differing late vote never does. - if (!_confettiShown && Game.State.IsUnanimous()) + // Whether to celebrate is decided once by the player who reveals the + // votes (see ShowVotes) and broadcast via the VotesShown event, so every + // client agrees regardless of the order votes and the reveal arrive in. + if (!_confettiShown && Game.State.ShowConfetti) { _confettiShown = true; await TriggerConfetti(); @@ -165,7 +165,7 @@ private void ShowVotes() { - Game.VotesShown(); + Game.VotesShown(Game.State.IsUnanimous()); GameContext.PublishEvents(); } } diff --git a/PointingParty.Client/GameStateExtensions.cs b/PointingParty.Client/GameStateExtensions.cs index b1afd1f..78bc5d7 100644 --- a/PointingParty.Client/GameStateExtensions.cs +++ b/PointingParty.Client/GameStateExtensions.cs @@ -14,23 +14,19 @@ public static double AverageVote(this GameState gameState) } /// - /// Returns true only when the round is fully settled and everyone agrees: - /// there is more than one player and every player has cast the same numeric - /// vote. A pending vote means a player has not voted yet or their vote is - /// still in transit, so the result is not settled and we must not celebrate - /// prematurely (which would otherwise fire confetti for a non-unanimous - /// round whenever a differing vote arrives just after the reveal). + /// Returns true when more than one player cast a numeric vote and every such + /// vote is the same. Players who abstained — i.e. did not vote (Pending) or + /// picked a non-numeric option (Coffee, Question) — are ignored. + /// This is evaluated by the player who reveals the votes (who has the full + /// picture) so the result is decided once and broadcast, rather than each + /// client re-deciding against its own, possibly incomplete, state. /// public static bool IsUnanimous(this GameState gameState) { - if (gameState.PlayerVotes.Count < 2) - return false; - - var votes = gameState.PlayerVotes.Values; - if (votes.Any(v => v.Status != VoteStatus.Scored)) - return false; + var scoredVotes = gameState.PlayerVotes.Values + .Where(v => v.Status == VoteStatus.Scored) + .ToList(); - var first = votes.First(); - return votes.All(v => v.Equals(first)); + return scoredVotes.Count > 1 && scoredVotes.All(v => v.Equals(scoredVotes[0])); } } diff --git a/PointingParty.Domain/Events/VotesShown.cs b/PointingParty.Domain/Events/VotesShown.cs index b19e2ea..b44bdd7 100644 --- a/PointingParty.Domain/Events/VotesShown.cs +++ b/PointingParty.Domain/Events/VotesShown.cs @@ -1,3 +1,3 @@ namespace PointingParty.Domain.Events; -public record VotesShown(string GameId) : IGameEvent; \ No newline at end of file +public record VotesShown(string GameId, bool ShowConfetti = false) : IGameEvent; \ No newline at end of file diff --git a/PointingParty.Domain/GameAggregate.cs b/PointingParty.Domain/GameAggregate.cs index a9dc533..2b27ddc 100644 --- a/PointingParty.Domain/GameAggregate.cs +++ b/PointingParty.Domain/GameAggregate.cs @@ -66,7 +66,8 @@ private void Apply(GameReset e) State = State with { PlayerVotes = State.PlayerVotes.ToImmutableDictionary(pv => pv.Key, _ => new Vote(VoteStatus.Pending)), - ShowVotes = false + ShowVotes = false, + ShowConfetti = false }; } @@ -103,6 +104,10 @@ private void Apply(Sync e) private void Apply(VoteCast e) { + // Once votes are revealed the round is locked; drop any further votes + // so a late or post-reveal vote can't alter the result. + if (State.ShowVotes) return; + State = State with { PlayerVotes = State.PlayerVotes.SetItem(e.PlayerName, e.Vote) @@ -111,7 +116,7 @@ private void Apply(VoteCast e) private void Apply(VotesShown e) { - State = State with { ShowVotes = true }; + State = State with { ShowVotes = true, ShowConfetti = e.ShowConfetti }; } public void GameReset() @@ -143,9 +148,9 @@ public void VoteCast(Vote vote) EventsToPublish.Add(voteCastEvent); } - public void VotesShown() + public void VotesShown(bool showConfetti) { - var showVotesEvent = new VotesShown(State.GameId); + var showVotesEvent = new VotesShown(State.GameId, showConfetti); Apply(showVotesEvent); EventsToPublish.Add(showVotesEvent); } diff --git a/PointingParty.Domain/GameState.cs b/PointingParty.Domain/GameState.cs index a38d03e..11a740a 100644 --- a/PointingParty.Domain/GameState.cs +++ b/PointingParty.Domain/GameState.cs @@ -2,4 +2,5 @@ namespace PointingParty.Domain; -public record GameState(string GameId, ImmutableDictionary PlayerVotes, bool ShowVotes); \ No newline at end of file +public record GameState(string GameId, ImmutableDictionary PlayerVotes, bool ShowVotes, + bool ShowConfetti = false); \ No newline at end of file