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()]