From b323bb6d6bfa03c8383f2fc972f51baf3db550ac Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 10:09:29 +0000 Subject: [PATCH] Split a match into periods that add back up to the full match length Period length was an integer number of minutes, so any duration that did not divide evenly lost the remainder: a 50 minute match in quarters became 4 x 12, and the dialog offered 48 minutes of a 50 minute match while the caption next to it still said 50. Every planned-minutes figure multiplied that truncated number back by 60, so the playing-time table planned everyone short too. Carry the length in seconds instead. Sixty divides by every period count there is, so the split is always exact, and the minutes form stays as a decimal for the two captions that put it on screen. MatchClockReport already worked in seconds for this reason and now asks the model rather than dividing itself. Checked with dotnet build -c Release, dotnet test (395), the Playwright suite (39), and scripts/coverage.sh. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01B7iiXxjNQueeYhqQAUKdkr --- docs/known_issues.md | 10 +++++++ docs/models.md | 4 ++- docs/patterns.md | 9 ++++-- src/FootballFormation.Core/Models/Game.cs | 27 +++++++++++++++-- .../Reporting/GameMinutesReport.cs | 2 +- .../Reporting/MatchClockReport.cs | 6 ++-- .../Reporting/PlayingTimeReport.cs | 12 ++++---- .../Pages/FormationBuilder.razor | 2 +- .../Pages/GameDialog.razor | 6 ++-- .../FootballFormation.Core.Tests/GameTests.cs | 30 ++++++++++++++----- .../PlayingTimeReportTests.cs | 5 ++-- 11 files changed, 84 insertions(+), 29 deletions(-) diff --git a/docs/known_issues.md b/docs/known_issues.md index 35638c5..b212ae5 100644 --- a/docs/known_issues.md +++ b/docs/known_issues.md @@ -56,6 +56,16 @@ Avoid repeating these mistakes: one instant share it, and rows written before the column existed all default to `0001-01-01`. Every test that predated this advanced `FakeTimeProvider` between substitutions, which is why it went unseen — the two that pin it now deliberately do not. +- **A period length in minutes silently loses the remainder**: `Game.PeriodDurationMinutes` used to + be `GameDurationMinutes / PeriodCount` in `int`, and every planned-minutes figure multiplied that + truncated number back by 60. A 50 minute match in quarters became 4 × 12, so the dialog offered + 48 minutes of a 50 minute match, the playing-time table planned everyone 4 minutes short, and the + builder's caption disagreed with the duration printed next to it. The fix is the rule to keep: + **period length is carried in seconds** (`Game.PeriodDurationSeconds`), which is always exact + because 60 divides by every period count there is, and the minutes form is a `decimal` for + display only. `MatchClockReport` already worked in seconds for exactly this reason — it just did + the division itself instead of asking the model. Reach for `PeriodDurationSeconds` in any new + arithmetic; `PeriodDurationMinutes` only ever goes on screen. - **Archiving is a filter on the future, not on the past**: only the "add existing player" picker and copy-forward look at `IsArchived`. `PlayerService.GetAllAsync` deliberately still returns archived players — it is the id → name lookup the match report and live screen resolve against, so filtering it would blank a scorer out of a game they scored in, which is the very thing archiving exists to prevent. Same reasoning for `Game.IsInRoster`: a past game has to be judged the way it was played. If a picker ever *should* hide them, filter at that call site, not in the lookup. ## Blazor / MudBlazor 9.x diff --git a/docs/models.md b/docs/models.md index 24ca5d4..5cff422 100644 --- a/docs/models.md +++ b/docs/models.md @@ -138,10 +138,12 @@ lives: an own goal counts for the opponent, so it is excluded from ours and incl **`Game.IsComplete` decides whether a game counts towards statistics at all**: the final whistle went on the live screen, or the game was never run live and has a final score on file. A match in progress is never complete however many goals are logged, or the season table and the scorer lists -would shift while it is still being played. Five more computed members support the reports: +would shift while it is still being played. More computed members support the reports: | Member | Answers | |---|---| +| `PeriodDurationSeconds` | How long one period lasts on an even split. **Seconds, not minutes** — a duration that splits into fractions of a minute (50 in quarters is 4 × 12.5) still splits exactly into seconds, so the periods add back up to the full match length. Every planned-minutes calculation reads this one | +| `PeriodDurationMinutes` | The same length as a `decimal`, fractional when it has to be. Display only | | `HasLineup` | Does any period have someone on the pitch? Needs `PlayerPositions` loaded | | `HasActualTimings` | Was any period actually kicked off, i.e. are there real timings to prefer over the plan? | | `PlayedDurationSeconds` | The same sum in seconds, without the fallback — the denominator for a share of one game's playing time, where truncating to minutes would let an ever-present player round past 100% | diff --git a/docs/patterns.md b/docs/patterns.md index bdad826..bfcc591 100644 --- a/docs/patterns.md +++ b/docs/patterns.md @@ -139,10 +139,15 @@ split was cut along the wrong line. ## Domain logic on the model Anything computable without the database lives on the entity, not in a service or a page: -`Game.PeriodCount`, `Game.PeriodDurationMinutes`, `Game.IsInRoster`, `Game.SelectRoster`, -`Game.LivePeriod()`, `Game.NextPeriod()`, `GameSplitTypeExtensions.PeriodCount()/PeriodLabel()`. `PeriodCount` derives from +`Game.PeriodCount`, `Game.PeriodDurationSeconds`, `Game.IsInRoster`, `Game.SelectRoster`, +`Game.LivePeriod()`, `Game.NextPeriod()`, +`GameSplitTypeExtensions.PeriodCount()/PeriodDurationSeconds()/PeriodLabel()`. `PeriodCount` derives from `PeriodTypeExtensions.ForSplitType`, so the count can never drift from the periods actually created. +The split-type extensions take the duration as a parameter rather than a `Game`, so the game dialog +can preview the split of a duration that has not been saved onto a game yet and get the same answer +the saved game will give. + ### Pass a value object, don't eager-load a navigation When a model rule needs data the entity doesn't own, hand it in as a parameter rather than relying on a navigation property being loaded. `Game.IsInRoster(player, squad)` takes a `SeasonSquad` diff --git a/src/FootballFormation.Core/Models/Game.cs b/src/FootballFormation.Core/Models/Game.cs index ccdacab..7393274 100644 --- a/src/FootballFormation.Core/Models/Game.cs +++ b/src/FootballFormation.Core/Models/Game.cs @@ -63,8 +63,15 @@ public class Game /// How many periods this game is split into. public int PeriodCount => SplitType.PeriodCount(); - /// Minutes each period lasts, assuming an even split of the game duration. - public int PeriodDurationMinutes => PeriodCount == 0 ? 0 : GameDurationMinutes / PeriodCount; + /// Seconds each period lasts on an even split of the game duration. + public int PeriodDurationSeconds => SplitType.PeriodDurationSeconds(GameDurationMinutes); + + /// + /// Minutes each period lasts, fractional when the split does not land on a whole minute + /// (50 in quarters is 4 × 12.5). For display — the arithmetic uses + /// . + /// + public decimal PeriodDurationMinutes => SplitType.PeriodDurationMinutes(GameDurationMinutes); /// /// True when at least one period has a player placed on the pitch. Only meaningful when @@ -226,6 +233,22 @@ public static class GameSplitTypeExtensions public static int PeriodCount(this GameSplitType splitType) => PeriodTypeExtensions.ForSplitType(splitType).Length; + /// + /// How long one period lasts, in seconds. Seconds rather than minutes because a duration that + /// splits into fractions of a minute (50 in quarters, 45 in halves) still splits exactly into + /// seconds — 60 divides by every period count there is — so the periods always add back up to + /// the full match length instead of quietly losing the remainder to integer division. + /// + public static int PeriodDurationSeconds(this GameSplitType splitType, int gameDurationMinutes) + { + var count = splitType.PeriodCount(); + return count == 0 ? 0 : gameDurationMinutes * 60 / count; + } + + /// The same length in minutes, fractional when it has to be. For display only. + public static decimal PeriodDurationMinutes(this GameSplitType splitType, int gameDurationMinutes) => + splitType.PeriodDurationSeconds(gameDurationMinutes) / 60m; + /// Singular noun for one period, for use in sentences ("copy to next half"). public static string PeriodLabel(this GameSplitType splitType) => splitType == GameSplitType.Halves ? "half" : "quarter"; diff --git a/src/FootballFormation.Core/Reporting/GameMinutesReport.cs b/src/FootballFormation.Core/Reporting/GameMinutesReport.cs index e37a768..5d94b4f 100644 --- a/src/FootballFormation.Core/Reporting/GameMinutesReport.cs +++ b/src/FootballFormation.Core/Reporting/GameMinutesReport.cs @@ -76,7 +76,7 @@ public static GameMinutes Build(Game game, int elapsedSeconds = 0) // No timings anywhere in this game: everyone fielded gets the whole period in the // position they were planned for. Substitutes get nothing, as before. foreach (var entry in period.PlayerPositions.Where(p => !p.IsSubstitute)) - Credit(seconds, entry.PlayerId, entry.Position, game.PeriodDurationMinutes * 60); + Credit(seconds, entry.PlayerId, entry.Position, game.PeriodDurationSeconds); continue; } diff --git a/src/FootballFormation.Core/Reporting/MatchClockReport.cs b/src/FootballFormation.Core/Reporting/MatchClockReport.cs index c8544fb..49c7545 100644 --- a/src/FootballFormation.Core/Reporting/MatchClockReport.cs +++ b/src/FootballFormation.Core/Reporting/MatchClockReport.cs @@ -45,9 +45,9 @@ public static MatchClock Build(Game game, GamePeriod? displayPeriod, int elapsed { if (displayPeriod is null) return MatchClock.BeforeKickOff; - // From seconds, not minutes: an odd duration (45 in two halves) would lose half a minute - // per half to integer division and the clock would never reach full time. - var halfSeconds = game.GameDurationMinutes * 60 / 2; + // Always halves, whatever the game is split into — a quarters game is still two halves, + // and the scoreboard counts in halves. + var halfSeconds = GameSplitType.Halves.PeriodDurationSeconds(game.GameDurationMinutes); var half = displayPeriod.PeriodType.Half(); var plannedStart = half == PeriodType.FirstHalf ? 0 : halfSeconds; diff --git a/src/FootballFormation.Core/Reporting/PlayingTimeReport.cs b/src/FootballFormation.Core/Reporting/PlayingTimeReport.cs index 19ad187..71bbb16 100644 --- a/src/FootballFormation.Core/Reporting/PlayingTimeReport.cs +++ b/src/FootballFormation.Core/Reporting/PlayingTimeReport.cs @@ -60,13 +60,13 @@ public static List Build( var actual = game.HasActualTimings ? GameMinutesReport.Build(game) : null; - // Against playable time, not GameDurationMinutes: with odd durations the integer period - // split drops a minute (45 in halves → 2×22), and playing every period should still read - // 100%. A tracked game is measured against the time it really ran, for the same reason — - // a half whistled off early must not cap everyone who played it at 80%. + // Against playable time, not GameDurationMinutes: a game whose periods are not all written + // up yet has less than a full match to share out, and playing every period there should + // still read 100%. A tracked game is measured against the time it really ran, for the same + // reason — a half whistled off early must not cap everyone who played it at 80%. var playableSeconds = actual is not null ? game.PlayedDurationSeconds - : orderedPeriods.Count * game.PeriodDurationMinutes * 60; + : orderedPeriods.Count * game.PeriodDurationSeconds; return roster .Select(player => BuildRow(game, player, orderedPeriods, periodLineups, actual, playableSeconds)) @@ -94,7 +94,7 @@ private static PlayingTimeRow BuildRow( details[period.Id] = Describe(player, entry); - if (entry is { IsSubstitute: false }) plannedSeconds += game.PeriodDurationMinutes * 60; + if (entry is { IsSubstitute: false }) plannedSeconds += game.PeriodDurationSeconds; } var seconds = actual?.SecondsFor(player.Id) ?? plannedSeconds; diff --git a/src/FootballFormation.UI/Pages/FormationBuilder.razor b/src/FootballFormation.UI/Pages/FormationBuilder.razor index 3671377..324ad12 100644 --- a/src/FootballFormation.UI/Pages/FormationBuilder.razor +++ b/src/FootballFormation.UI/Pages/FormationBuilder.razor @@ -166,7 +166,7 @@
@L["Playing Time Overview"]
- @GameData.GameDurationMinutes min @L["Total"].ToString().ToLower() — @GameData.PeriodCount x @GameData.PeriodDurationMinutes min @L["per"] @L[GameData.SplitType.PeriodLabel()] + @GameData.GameDurationMinutes min @L["Total"].ToString().ToLower() — @GameData.PeriodCount x @GameData.PeriodDurationMinutes.ToString("0.##") min @L["per"] @L[GameData.SplitType.PeriodLabel()] diff --git a/src/FootballFormation.UI/Pages/GameDialog.razor b/src/FootballFormation.UI/Pages/GameDialog.razor index 00bc06c..e7f0db8 100644 --- a/src/FootballFormation.UI/Pages/GameDialog.razor +++ b/src/FootballFormation.UI/Pages/GameDialog.razor @@ -50,10 +50,10 @@ @{ - var periods = SplitType == GameSplitType.Halves ? 2 : 4; - var periodLength = GameDurationMinutes / periods; + var periods = SplitType.PeriodCount(); + var periodLength = SplitType.PeriodDurationMinutes(GameDurationMinutes); } - @periods x @periodLength min @L["per"] @L[SplitType.PeriodLabel()] + @periods x @periodLength.ToString("0.##") min @L["per"] @L[SplitType.PeriodLabel()]