diff --git a/docs/architecture.md b/docs/architecture.md index 9f67573..1d92de0 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -12,7 +12,8 @@ Models/ Game.cs — Game entity (incl. SeasonId + live match clock/state), GameSplitType and MatchState enums GamePeriod.cs — GamePeriod entity, PeriodType enum, PeriodTypeExtensions GamePlayerPosition.cs — Links player to position in a period (IsSubstitute flag) - GameGoal.cs — A goal: scorer (null for the opponent), assister, minute (+ stoppage), own/opponent flags + GameGoal.cs — A goal: scorer (null for the opponent), assister, the half + clock reading it was + scored at, own/opponent flags GameSubstitution.cs — A timestamped change made during a live match MatchPreferences.cs — Per-season game defaults (duration, split, formation, match day) GameComment.cs — An admin's note on a game: body, public/private, author, edited marker @@ -52,7 +53,8 @@ Reporting/ PlayerStatsReport.cs — Per-player aggregates (PlayerStats, PositionStat, PlayerGameStat) PositionFitHelper.cs — 5-tier position fit: Preferred, NaturalFit, Alternative, Compatible, OutOfPosition MatchClockReport.cs — Derives the live clock and the half's reading from the stored anchor + - banked total, and the MatchMinute an event is written down against + banked total, the MatchMinute an event is written down against, and the + half it belongs to PlannedChangesReport.cs — What the plan for the middle of a half changes versus the line-up on the pitch, minus the swaps play has already overtaken, for UI/Components/PlannedChangesList diff --git a/docs/known_issues.md b/docs/known_issues.md index 86d56fc..02a51c9 100644 --- a/docs/known_issues.md +++ b/docs/known_issues.md @@ -373,13 +373,31 @@ Avoid repeating these mistakes: it. Q2 and Q4 reach the touchline only as `Game.MidHalfPlan()`, behind the live screen's `Changes (n)` pop-up. Do not "fix" a Q2 with no timings, and do not read `PeriodCount` as a count of stages the clock stops for. -- **A goal's minute is not one number.** `GameGoal.Minute` stops at the end of the half and - `AdditionalMinute` counts the overrun beside it, because the two together are what orders a - timeline: counted on into a single number, a goal at 35+2 reads 37 and sorts after a goal in the - 36th minute of the second half, which happened a minute later. Rows written before the split have - `AdditionalMinute = 0` and keep whatever number they were given — the migration deliberately does - not backfill, because nothing left in the row says whether a 37 was stoppage time or typed in by - hand. +- **A goal's minute is derived, not stored — and two goals in the same table are placed by + different columns.** A goal logged from `/live` carries `GamePeriodId` and `AtSeconds`, the same + pair a substitution carries, and the minute anyone sees comes out of `MatchClockReport.MinuteOf`. + A goal typed in on `/result` has neither and falls back to `Minute`. So do all the goals logged + before `StoreGoalPeriodAndClock` that were not scored in stoppage time: that migration backfills + only what an old row states outright, and a plain minute does not say which half it belonged to. + The trap is reading `Minute` directly and finding it null on a live match, or assuming a row that + has one was typed in by hand. Never reinstate the previous shape — a minute frozen on the row + moved under stored data whenever `GameDurationMinutes` changed, and could not be corrected when a + half's timings were. + The same migration dropped `AdditionalMinute`, but **backfilled the rows that carried one first**: + an overrun on a row says outright that it was stoppage time, so the half follows from the minute + and the clock reading from that half's kick-off, and those goals still read `30+2` afterwards. + `32` would be the 32nd minute — two minutes into a second half — which is a different moment. + Rows with `AdditionalMinute = 0` were left alone, because a stored `37` could equally be a minute + typed in by hand. `GoalClockBackfillTests` migrates a database across that boundary and asserts + what the app then shows. +- **A stored `Minute` is a scoreboard reading, and the timeline is ordered on elapsed seconds — do + not mix the two.** They agree only while the halves run to length. On a match whose first half + was whistled off three minutes long, the scoreboard's 31' is 33 minutes of elapsed play, so + taking `(Minute - 1) * 60` as an ordering key files a second-half goal *before* one scored in + first-half stoppage time — wrong running score out of `ScoreProgressionReport`, and the goal + drawn on the wrong side of the half-time rule. `MatchClockReport.ElapsedOf` is the conversion, + and it is the only thing that should produce an ordering key for a goal. It cost a review round + on the change that introduced it. ## Authentication - **`ExpireTimeSpan` does not keep anyone signed in — `IsPersistent` does.** `SignInAsync` without diff --git a/docs/models.md b/docs/models.md index 271b811..fc32d76 100644 --- a/docs/models.md +++ b/docs/models.md @@ -213,8 +213,9 @@ bench, never both and never twice. | GameId | int | FK → Game (cascade delete) | | ScorerId | int? | FK → Player, **SetNull**. Null for an opponent goal — we don't track their players | | AssisterId | int? | FK → Player, SetNull | -| Minute | int? | Free-typed on `/result`; stamped from the scoreboard clock on `/live`, and never past the end of the half | -| AdditionalMinute | int | Minutes into stoppage time, from 1; 0 in normal play. Stored apart from `Minute` so 35+2 sorts before 36 — see `MatchMinute` | +| GamePeriodId | int? | FK → GamePeriod (cascade delete). The half that was being played. Null for a goal typed in on `/result` | +| AtSeconds | int? | Match-clock second the ball went in. Null for the same reason | +| Minute | int? | Free-typed on `/result`, and the fallback for goals logged before `AtSeconds` existed. Not written by `/live` any more. A scoreboard reading, not elapsed time — convert with `MatchClockReport.ElapsedOf` before ordering on it | | IsOwnGoal | bool | One of ours into our own net. Counts for the opponent | | IsOpponentGoal | bool | The opponent scored. Counts for them, and has no scorer | | RecordedAt | DateTime | UTC entry time — orders events that share a minute | @@ -231,18 +232,21 @@ bench, never both and never twice. | Position | PlayerPosition | The position that changed hands | | RecordedAt | DateTime | UTC entry time — orders events that share a minute | -A substitution has no stored minute: `MatchClockReport.MinuteOf` derives it from `AtSeconds` and -the half the change belongs to, so it reads off the same scoreboard clock a goal was stamped from -rather than the raw elapsed time. A goal cannot be derived that way — one typed in on `/result` has -no clock behind it at all — which is why its minute is stored, both halves of it. - -`RecordedAt` exists on both `GameGoal` and `GameSubstitution` because the minute alone cannot order -a timeline: a goal and the substitution that followed it routinely share one, and several events in -the opening minute is the normal case, not the edge case. The live timeline sorts by minute, then by -`RecordedAt`, then by `Id`, all descending. Rows written before the column existed default to -`0001-01-01`, and two changes entered in one instant share it, so `RecordedAt` cannot settle a -double substitution on its own — the id is the last word, and it is the same one -`RemoveSubstitutionAsync` uses, so the entry the timeline puts on top is the entry whose Undo works. +**Neither kind of event stores the minute it is shown against.** Both store where they happened — +the half, and the reading on the match clock — and `MatchClockReport.MinuteOf` derives the minute +from that pair, so the two kinds read off one code path and correcting a half's `StartedAtSeconds` +corrects the goals in it as well as the substitutions. A goal typed in on `/result` has no clock +behind it and falls back to `Minute`; a goal with neither shows no minute at all, which the result +page allows. + +`RecordedAt` exists on both `GameGoal` and `GameSubstitution` because the clock alone cannot order +a timeline: a goal and the substitution that followed it routinely share a second, and several +events in the opening minute is the normal case, not the edge case. The live timeline sorts by +elapsed seconds (`MatchClockReport.ElapsedOf`, `GameSubstitution.AtSeconds`), then by `RecordedAt`, +then by `Id`, all descending. Rows written before the column existed default to `0001-01-01`, and +two changes entered in one instant share it, so `RecordedAt` cannot settle a double substitution on +its own — the id is the last word, and it is the same one `RemoveSubstitutionAsync` uses, so the +entry the timeline puts on top is the entry whose Undo works. Ids from the two tables are not comparable with each other, so a goal and a substitution that tie on both minute and `RecordedAt` keep an arbitrary (but stable) order. @@ -374,6 +378,8 @@ runs on every startup and does nothing once any account exists, so a changed pas Season 1──* Game 1──* GamePeriod 1──* GamePlayerPosition *──1 Player Season 1──* SeasonSquadMember *──1 Player Game 1──* GameGoal *──1 Player (scorer, assister — both SetNull) +GamePeriod 1──* GameGoal (the half it was scored in — nullable, cascade) +GamePeriod 1──* GameSubstitution (the half it was made in — cascade) Game 1──* GameSubstitution *──1 Player (off, on — both Restrict) Game 1──* GameComment *──1 AppUser (author — SetNull) ``` diff --git a/docs/testing.md b/docs/testing.md index 36ac64d..7550be6 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -33,6 +33,7 @@ Every test class, so a gap here is visible rather than assumed: | **Authorization** | `AuthorizationTests` | That every write refuses a non-admin *at the service*, not only in the markup — the guard the whole write path rests on | | Accounts | `UserServiceTests`, `SeededAdminTests` | Credentials, security stamps, the last-admin guard, and the seeded account being no working login | | Boot safety | `DatabaseSafetyTests`, `HealthReportTests` | The pre-migration snapshot and what `/health` is allowed to call healthy | +| Migrations that rewrite rows | `GoalClockBackfillTests` | The only migration with a backfill in it. Migrates a seeded database across the boundary and asserts what the app then *shows* — a goal written `30+2` still reads `30+2` — rather than what landed in a column. Every other migration is covered implicitly, because `ServiceTestBase` builds the schema from the model | | Service lifetime | `ServiceLifetimeTests` | Concurrent reads, and detached entities round-tripping through update | | `Result` | `ResultTests` | Error keys, arguments, the guard on reading a failed value, and that a cancellation stays one when carried between types | | Cancellation | `CancellationTests` | That a caller going away is an ordinary outcome and not a logged error — including that an `OperationCanceledException` nobody asked for still is one | diff --git a/docs/ui_components.md b/docs/ui_components.md index 4c85103..d51d3fe 100644 --- a/docs/ui_components.md +++ b/docs/ui_components.md @@ -145,14 +145,25 @@ watches the same URL read-only. Every control sits in an ` entity) .WithMany() .HasForeignKey(g => g.AssisterId) .OnDelete(DeleteBehavior.SetNull); + + // Cascade like GameSubstitution's: the half and the events recorded during it are one + // record, and a goal pointing at a line-up that no longer exists has no minute to show. + // Declared without a navigation — see GameGoal.GamePeriodId. + entity.HasOne() + .WithMany() + .HasForeignKey(g => g.GamePeriodId) + .OnDelete(DeleteBehavior.Cascade); } } diff --git a/src/FootballFormation.Core/Migrations/20260814112353_StoreGoalPeriodAndClock.Designer.cs b/src/FootballFormation.Core/Migrations/20260814112353_StoreGoalPeriodAndClock.Designer.cs new file mode 100644 index 0000000..99da9c0 --- /dev/null +++ b/src/FootballFormation.Core/Migrations/20260814112353_StoreGoalPeriodAndClock.Designer.cs @@ -0,0 +1,604 @@ +// +using System; +using FootballFormation.Core.Data; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; + +#nullable disable + +namespace FootballFormation.Core.Migrations +{ + [DbContext(typeof(AppDbContext))] + [Migration("20260814112353_StoreGoalPeriodAndClock")] + partial class StoreGoalPeriodAndClock + { + /// + protected override void BuildTargetModel(ModelBuilder modelBuilder) + { +#pragma warning disable 612, 618 + modelBuilder.HasAnnotation("ProductVersion", "10.0.10"); + + modelBuilder.Entity("FootballFormation.Core.Models.AppUser", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("INTEGER"); + + b.Property("DisplayName") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("TEXT"); + + b.Property("MustChangePassword") + .HasColumnType("INTEGER"); + + b.Property("PasswordHash") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("Role") + .HasColumnType("INTEGER"); + + b.Property("SecurityStamp") + .IsRequired() + .HasMaxLength(64) + .HasColumnType("TEXT"); + + b.Property("Username") + .IsRequired() + .HasMaxLength(50) + .HasColumnType("TEXT"); + + b.HasKey("Id"); + + b.HasIndex("Username") + .IsUnique(); + + b.ToTable("Users", (string)null); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.Game", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("INTEGER"); + + b.Property("ClockAccumulatedSeconds") + .HasColumnType("INTEGER"); + + b.Property("ClockRunningSince") + .HasColumnType("TEXT"); + + b.Property("Date") + .HasColumnType("TEXT"); + + b.Property("FormationType") + .HasColumnType("INTEGER"); + + b.Property("GameDurationMinutes") + .HasColumnType("INTEGER"); + + b.Property("GuestPlayerIds") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("IsHomeGame") + .HasColumnType("INTEGER"); + + b.Property("LivePeriodId") + .HasColumnType("INTEGER"); + + b.Property("MatchState") + .HasColumnType("INTEGER"); + + b.Property("MatchType") + .HasColumnType("INTEGER"); + + b.Property("Opponent") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("TEXT"); + + b.Property("ScoreAway") + .HasColumnType("INTEGER"); + + b.Property("ScoreHome") + .HasColumnType("INTEGER"); + + b.Property("SeasonId") + .HasColumnType("INTEGER"); + + b.Property("SplitType") + .HasColumnType("INTEGER"); + + b.Property("UnavailablePlayerIds") + .IsRequired() + .HasColumnType("TEXT"); + + b.HasKey("Id"); + + b.HasIndex("SeasonId"); + + b.ToTable("Games"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.GameComment", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("INTEGER"); + + b.Property("AuthorId") + .HasColumnType("INTEGER"); + + b.Property("Body") + .IsRequired() + .HasMaxLength(2000) + .HasColumnType("TEXT"); + + b.Property("CreatedAt") + .HasColumnType("TEXT"); + + b.Property("EditedAt") + .HasColumnType("TEXT"); + + b.Property("GameId") + .HasColumnType("INTEGER"); + + b.Property("IsPublic") + .HasColumnType("INTEGER"); + + b.HasKey("Id"); + + b.HasIndex("AuthorId"); + + b.HasIndex("GameId", "CreatedAt"); + + b.ToTable("GameComments"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.GameGoal", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("INTEGER"); + + b.Property("AssisterId") + .HasColumnType("INTEGER"); + + b.Property("AtSeconds") + .HasColumnType("INTEGER"); + + b.Property("GameId") + .HasColumnType("INTEGER"); + + b.Property("GamePeriodId") + .HasColumnType("INTEGER"); + + b.Property("IsOpponentGoal") + .HasColumnType("INTEGER"); + + b.Property("IsOwnGoal") + .HasColumnType("INTEGER"); + + b.Property("Minute") + .HasColumnType("INTEGER"); + + b.Property("RecordedAt") + .HasColumnType("TEXT"); + + b.Property("ScorerId") + .HasColumnType("INTEGER"); + + b.HasKey("Id"); + + b.HasIndex("AssisterId"); + + b.HasIndex("GameId"); + + b.HasIndex("GamePeriodId"); + + b.HasIndex("ScorerId"); + + b.ToTable("GameGoals"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.GamePeriod", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("INTEGER"); + + b.Property("EndedAtSeconds") + .HasColumnType("INTEGER"); + + b.Property("FormationTypeOverride") + .HasColumnType("INTEGER"); + + b.Property("GameId") + .HasColumnType("INTEGER"); + + b.Property("PeriodType") + .HasColumnType("INTEGER"); + + b.Property("StartedAtSeconds") + .HasColumnType("INTEGER"); + + b.HasKey("Id"); + + b.HasIndex("GameId"); + + b.ToTable("GamePeriods"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.GamePlayerPosition", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("INTEGER"); + + b.Property("GamePeriodId") + .HasColumnType("INTEGER"); + + b.Property("IsSubstitute") + .HasColumnType("INTEGER"); + + b.Property("PlayerId") + .HasColumnType("INTEGER"); + + b.Property("Position") + .HasColumnType("INTEGER"); + + b.Property("SlotIndex") + .HasColumnType("INTEGER"); + + b.HasKey("Id"); + + b.HasIndex("PlayerId"); + + b.HasIndex("GamePeriodId", "PlayerId") + .IsUnique(); + + b.ToTable("GamePlayerPositions"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.GameSubstitution", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("INTEGER"); + + b.Property("AtSeconds") + .HasColumnType("INTEGER"); + + b.Property("GameId") + .HasColumnType("INTEGER"); + + b.Property("GamePeriodId") + .HasColumnType("INTEGER"); + + b.Property("PlayerOffId") + .HasColumnType("INTEGER"); + + b.Property("PlayerOnId") + .HasColumnType("INTEGER"); + + b.Property("Position") + .HasColumnType("INTEGER"); + + b.Property("RecordedAt") + .HasColumnType("TEXT"); + + b.Property("SlotIndex") + .HasColumnType("INTEGER"); + + b.HasKey("Id"); + + b.HasIndex("GameId"); + + b.HasIndex("GamePeriodId"); + + b.HasIndex("PlayerOffId"); + + b.HasIndex("PlayerOnId"); + + b.ToTable("GameSubstitutions"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.MatchPreferences", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("INTEGER"); + + b.Property("DefaultFormation") + .HasColumnType("INTEGER"); + + b.Property("DefaultSplitType") + .HasColumnType("INTEGER"); + + b.Property("GameDurationMinutes") + .HasColumnType("INTEGER"); + + b.Property("MatchDay") + .HasColumnType("INTEGER"); + + b.Property("SeasonId") + .HasColumnType("INTEGER"); + + b.HasKey("Id"); + + b.HasIndex("SeasonId") + .IsUnique(); + + b.ToTable("MatchPreferences"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.Player", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("INTEGER"); + + b.Property("AlternativePositions") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("FirstName") + .IsRequired() + .HasMaxLength(50) + .HasColumnType("TEXT"); + + b.Property("IsArchived") + .HasColumnType("INTEGER"); + + b.Property("PreferredPosition") + .HasColumnType("INTEGER"); + + b.Property("ShirtNumber") + .HasColumnType("INTEGER"); + + b.Property("Surname") + .HasMaxLength(50) + .HasColumnType("TEXT"); + + b.HasKey("Id"); + + b.ToTable("Players"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.Season", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("INTEGER"); + + b.Property("EndDate") + .HasColumnType("TEXT"); + + b.Property("IsCurrent") + .HasColumnType("INTEGER"); + + b.Property("Name") + .IsRequired() + .HasMaxLength(20) + .HasColumnType("TEXT"); + + b.Property("StartDate") + .HasColumnType("TEXT"); + + b.HasKey("Id"); + + b.HasIndex("StartDate") + .IsUnique(); + + b.ToTable("Seasons"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.SeasonSquadMember", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("INTEGER"); + + b.Property("IsGuest") + .HasColumnType("INTEGER"); + + b.Property("PlayerId") + .HasColumnType("INTEGER"); + + b.Property("SeasonId") + .HasColumnType("INTEGER"); + + b.HasKey("Id"); + + b.HasIndex("PlayerId"); + + b.HasIndex("SeasonId", "PlayerId") + .IsUnique(); + + b.ToTable("SeasonSquadMembers"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.Game", b => + { + b.HasOne("FootballFormation.Core.Models.Season", "Season") + .WithMany("Games") + .HasForeignKey("SeasonId") + .OnDelete(DeleteBehavior.Restrict) + .IsRequired(); + + b.Navigation("Season"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.GameComment", b => + { + b.HasOne("FootballFormation.Core.Models.AppUser", "Author") + .WithMany() + .HasForeignKey("AuthorId") + .OnDelete(DeleteBehavior.SetNull); + + b.HasOne("FootballFormation.Core.Models.Game", "Game") + .WithMany("Comments") + .HasForeignKey("GameId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.Navigation("Author"); + + b.Navigation("Game"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.GameGoal", b => + { + b.HasOne("FootballFormation.Core.Models.Player", "Assister") + .WithMany() + .HasForeignKey("AssisterId") + .OnDelete(DeleteBehavior.SetNull); + + b.HasOne("FootballFormation.Core.Models.Game", "Game") + .WithMany("Goals") + .HasForeignKey("GameId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.HasOne("FootballFormation.Core.Models.GamePeriod", null) + .WithMany() + .HasForeignKey("GamePeriodId") + .OnDelete(DeleteBehavior.Cascade); + + b.HasOne("FootballFormation.Core.Models.Player", "Scorer") + .WithMany() + .HasForeignKey("ScorerId") + .OnDelete(DeleteBehavior.SetNull); + + b.Navigation("Assister"); + + b.Navigation("Game"); + + b.Navigation("Scorer"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.GamePeriod", b => + { + b.HasOne("FootballFormation.Core.Models.Game", "Game") + .WithMany("Periods") + .HasForeignKey("GameId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.Navigation("Game"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.GamePlayerPosition", b => + { + b.HasOne("FootballFormation.Core.Models.GamePeriod", "GamePeriod") + .WithMany("PlayerPositions") + .HasForeignKey("GamePeriodId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.HasOne("FootballFormation.Core.Models.Player", "Player") + .WithMany() + .HasForeignKey("PlayerId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.Navigation("GamePeriod"); + + b.Navigation("Player"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.GameSubstitution", b => + { + b.HasOne("FootballFormation.Core.Models.Game", "Game") + .WithMany("Substitutions") + .HasForeignKey("GameId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.HasOne("FootballFormation.Core.Models.GamePeriod", "GamePeriod") + .WithMany() + .HasForeignKey("GamePeriodId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.HasOne("FootballFormation.Core.Models.Player", "PlayerOff") + .WithMany() + .HasForeignKey("PlayerOffId") + .OnDelete(DeleteBehavior.Restrict) + .IsRequired(); + + b.HasOne("FootballFormation.Core.Models.Player", "PlayerOn") + .WithMany() + .HasForeignKey("PlayerOnId") + .OnDelete(DeleteBehavior.Restrict) + .IsRequired(); + + b.Navigation("Game"); + + b.Navigation("GamePeriod"); + + b.Navigation("PlayerOff"); + + b.Navigation("PlayerOn"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.MatchPreferences", b => + { + b.HasOne("FootballFormation.Core.Models.Season", "Season") + .WithMany() + .HasForeignKey("SeasonId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.Navigation("Season"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.SeasonSquadMember", b => + { + b.HasOne("FootballFormation.Core.Models.Player", "Player") + .WithMany() + .HasForeignKey("PlayerId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.HasOne("FootballFormation.Core.Models.Season", "Season") + .WithMany("SquadMembers") + .HasForeignKey("SeasonId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.Navigation("Player"); + + b.Navigation("Season"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.Game", b => + { + b.Navigation("Comments"); + + b.Navigation("Goals"); + + b.Navigation("Periods"); + + b.Navigation("Substitutions"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.GamePeriod", b => + { + b.Navigation("PlayerPositions"); + }); + + modelBuilder.Entity("FootballFormation.Core.Models.Season", b => + { + b.Navigation("Games"); + + b.Navigation("SquadMembers"); + }); +#pragma warning restore 612, 618 + } + } +} diff --git a/src/FootballFormation.Core/Migrations/20260814112353_StoreGoalPeriodAndClock.cs b/src/FootballFormation.Core/Migrations/20260814112353_StoreGoalPeriodAndClock.cs new file mode 100644 index 0000000..f45fb83 --- /dev/null +++ b/src/FootballFormation.Core/Migrations/20260814112353_StoreGoalPeriodAndClock.cs @@ -0,0 +1,139 @@ +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace FootballFormation.Core.Migrations +{ + /// + /// Gives a goal the two facts a substitution already carries — the half it happened in and the + /// reading on the match clock — so the minute shown is derived rather than frozen, and + /// correcting a half's timings corrects its goals with it. + /// + /// Backfilled for exactly the rows that can be, which is the ones carrying an overrun. + /// AdditionalMinute > 0 is a row saying outright that it was scored in stoppage time, + /// so nothing has to be guessed: the minute names the half, the half names its kick-off, and + /// the clock reading falls out of the two. Those goals go on reading 30+2 afterwards, + /// which is the point — 32 is the 32nd minute, two minutes into a second half, and not + /// what happened. + /// + /// + /// Every other old row is deliberately left alone. A goal with AdditionalMinute = 0 says + /// nothing about which half it belongs to — a stored 37 could be a minute typed in by + /// hand on the result page — so it keeps Minute and goes on sorting where it always has. + /// Guessing a half for those is how the frozen minute went wrong in the first place. + /// + /// + /// The order below is not what SQLite runs. EF folds the DropColumn into the table + /// rebuild that AddForeignKey already forces — the temp table is simply created without + /// the column — so this is one rebuild whatever order the operations are written in. What that + /// leaves is a migration that cannot be retried: the two ADD COLUMNs commit in + /// their own transaction, and a run interrupted after that point has not recorded itself in + /// __EFMigrationsHistory, so the next boot starts again from the top and dies on + /// duplicate column name: AtSeconds. Recovery is the pre-migration snapshot + /// Program.cs takes, which is the only thing standing behind any of this. + /// + /// + public partial class StoreGoalPeriodAndClock : Migration + { + /// + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.AddColumn( + name: "AtSeconds", + table: "GameGoals", + type: "INTEGER", + nullable: true); + + migrationBuilder.AddColumn( + name: "GamePeriodId", + table: "GameGoals", + type: "INTEGER", + nullable: true); + + // The one set of rows that can be moved across without guessing: a goal with an + // overrun on it says outright that it was scored in stoppage time, so which half it + // belongs to follows from its minute and the clock reading follows from the half's + // own kick-off. Everything else keeps Minute and is left alone — see the remarks. + // + // Reconstructed to the minute, which is all the row ever held. Read back through + // MatchClockReport it lands on the same 30+2 it was written as: the half's clock is + // capped at halfSeconds and the remainder is counted alongside from 1. + // + // A goal whose half was never kicked off, or whose game has no duration, selects NULL + // for both columns — which is exactly the state it would have had with no backfill. + migrationBuilder.Sql(""" + UPDATE GameGoals + SET AtSeconds = ( + SELECT p.StartedAtSeconds + (g.GameDurationMinutes * 60 / 2) + + ((GameGoals.AdditionalMinute - 1) * 60) + FROM GamePeriods p + JOIN Games g ON g.Id = p.GameId + WHERE p.GameId = GameGoals.GameId + AND g.GameDurationMinutes > 0 + AND p.StartedAtSeconds IS NOT NULL + AND (CASE WHEN p.PeriodType IN (0, 2, 3) THEN 0 ELSE 1 END) + = (CASE WHEN GameGoals.Minute * 60 + <= (g.GameDurationMinutes * 60 / 2) THEN 0 ELSE 1 END) + ORDER BY p.StartedAtSeconds + LIMIT 1), + GamePeriodId = ( + SELECT p.Id + FROM GamePeriods p + JOIN Games g ON g.Id = p.GameId + WHERE p.GameId = GameGoals.GameId + AND g.GameDurationMinutes > 0 + AND p.StartedAtSeconds IS NOT NULL + AND (CASE WHEN p.PeriodType IN (0, 2, 3) THEN 0 ELSE 1 END) + = (CASE WHEN GameGoals.Minute * 60 + <= (g.GameDurationMinutes * 60 / 2) THEN 0 ELSE 1 END) + ORDER BY p.StartedAtSeconds + LIMIT 1) + WHERE AdditionalMinute > 0 AND Minute IS NOT NULL; + """); + + migrationBuilder.CreateIndex( + name: "IX_GameGoals_GamePeriodId", + table: "GameGoals", + column: "GamePeriodId"); + + migrationBuilder.AddForeignKey( + name: "FK_GameGoals_GamePeriods_GamePeriodId", + table: "GameGoals", + column: "GamePeriodId", + principalTable: "GamePeriods", + principalColumn: "Id", + onDelete: ReferentialAction.Cascade); + + migrationBuilder.DropColumn( + name: "AdditionalMinute", + table: "GameGoals"); + } + + /// + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.AddColumn( + name: "AdditionalMinute", + table: "GameGoals", + type: "INTEGER", + nullable: false, + defaultValue: 0); + + migrationBuilder.DropForeignKey( + name: "FK_GameGoals_GamePeriods_GamePeriodId", + table: "GameGoals"); + + migrationBuilder.DropIndex( + name: "IX_GameGoals_GamePeriodId", + table: "GameGoals"); + + migrationBuilder.DropColumn( + name: "AtSeconds", + table: "GameGoals"); + + migrationBuilder.DropColumn( + name: "GamePeriodId", + table: "GameGoals"); + } + } +} diff --git a/src/FootballFormation.Core/Migrations/AppDbContextModelSnapshot.cs b/src/FootballFormation.Core/Migrations/AppDbContextModelSnapshot.cs index 87b3827..44928c3 100644 --- a/src/FootballFormation.Core/Migrations/AppDbContextModelSnapshot.cs +++ b/src/FootballFormation.Core/Migrations/AppDbContextModelSnapshot.cs @@ -162,15 +162,18 @@ protected override void BuildModel(ModelBuilder modelBuilder) .ValueGeneratedOnAdd() .HasColumnType("INTEGER"); - b.Property("AdditionalMinute") + b.Property("AssisterId") .HasColumnType("INTEGER"); - b.Property("AssisterId") + b.Property("AtSeconds") .HasColumnType("INTEGER"); b.Property("GameId") .HasColumnType("INTEGER"); + b.Property("GamePeriodId") + .HasColumnType("INTEGER"); + b.Property("IsOpponentGoal") .HasColumnType("INTEGER"); @@ -192,6 +195,8 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasIndex("GameId"); + b.HasIndex("GamePeriodId"); + b.HasIndex("ScorerId"); b.ToTable("GameGoals"); @@ -456,6 +461,11 @@ protected override void BuildModel(ModelBuilder modelBuilder) .OnDelete(DeleteBehavior.Cascade) .IsRequired(); + b.HasOne("FootballFormation.Core.Models.GamePeriod", null) + .WithMany() + .HasForeignKey("GamePeriodId") + .OnDelete(DeleteBehavior.Cascade); + b.HasOne("FootballFormation.Core.Models.Player", "Scorer") .WithMany() .HasForeignKey("ScorerId") diff --git a/src/FootballFormation.Core/Models/GameGoal.cs b/src/FootballFormation.Core/Models/GameGoal.cs index e35dab0..caee347 100644 --- a/src/FootballFormation.Core/Models/GameGoal.cs +++ b/src/FootballFormation.Core/Models/GameGoal.cs @@ -14,19 +14,32 @@ public class GameGoal public Player? Assister { get; set; } /// - /// The minute on the scoreboard clock, which never runs past the end of the half — the overrun - /// is . Null on a goal recorded with no minute at all, which the - /// result page allows. + /// The half that was being played when the ball went in, as the line-up playing it — the same + /// fact carries. Null for a goal typed in on the + /// result page, which has no half behind it. + /// + /// The id alone, with no navigation beside it: every reader resolves it against the half + /// already loaded on the game (MatchClockReport), so a navigation would only be an + /// invitation to Include the same row a second time. + /// /// - public int? Minute { get; set; } + public int? GamePeriodId { get; set; } /// - /// Minutes into stoppage time, counted from 1, or zero for a goal in normal play. Stored beside - /// the minute rather than added into it because the two together are what orders the timeline: - /// a goal at 35+2 belongs before one at 36, and a single number that had counted on to 37 would - /// put it after. Always zero on a goal typed in by hand, which has no clock behind it. + /// Match-clock second the ball went in, from the same clock a substitution is stamped from. + /// Null for a goal typed in by hand. With it is everything the + /// displayed minute is derived from, so correcting a half's timings corrects its goals too. /// - public int AdditionalMinute { get; set; } + public int? AtSeconds { get; set; } + + /// + /// The minute somebody typed in on the result page, where there is no clock to read. Also what + /// a goal logged before this row carried a clock still shows, unless it was scored in stoppage + /// time — those said so on the row and were moved onto the clock by StoreGoalPeriodAndClock. + /// The rest keep only this, because nothing left in one says whether a 37 in a 35-minute half + /// was stoppage time or a number typed in by hand. + /// + public int? Minute { get; set; } /// One of ours put it in our own net. Counts for the opponent. public bool IsOwnGoal { get; set; } diff --git a/src/FootballFormation.Core/Models/MatchMinute.cs b/src/FootballFormation.Core/Models/MatchMinute.cs index 3163b60..c1d63ad 100644 --- a/src/FootballFormation.Core/Models/MatchMinute.cs +++ b/src/FootballFormation.Core/Models/MatchMinute.cs @@ -4,23 +4,16 @@ namespace FootballFormation.Core.Models; /// The minute an event is written down against, the way football writes one: 35, or 35+2 once the /// half has been played out and the clock is into stoppage time. /// -/// The pair is also what puts events in order, which a single number cannot do. A goal in first-half -/// stoppage time and one just after the restart are barely a minute apart, but a clock that counted -/// straight on would write the first as 37 and the second as 36 and list them the wrong way round. -/// Comparing on first and second is -/// chronological across the whole match, because a half's clock stops at the half and the half that -/// follows starts above it. +/// Display only. Ordering a timeline is the elapsed clock's job — MatchClockReport.ElapsedOf +/// and — which runs on across the break and needs no pair +/// to say that a goal at 35+2 came before one in the 36th minute of the second half. /// /// /// The minute on the clock, never past the end of the half being played. /// Minutes into stoppage time, counted from 1; zero during normal play. -public readonly record struct MatchMinute(int Minute, int Additional) : IComparable +public readonly record struct MatchMinute(int Minute, int Additional) { public bool IsAdditional => Additional > 0; - public int CompareTo(MatchMinute other) => Minute != other.Minute - ? Minute.CompareTo(other.Minute) - : Additional.CompareTo(other.Additional); - public override string ToString() => IsAdditional ? $"{Minute}+{Additional}" : $"{Minute}"; } diff --git a/src/FootballFormation.Core/Reporting/MatchClockReport.cs b/src/FootballFormation.Core/Reporting/MatchClockReport.cs index 93ff336..e9a5091 100644 --- a/src/FootballFormation.Core/Reporting/MatchClockReport.cs +++ b/src/FootballFormation.Core/Reporting/MatchClockReport.cs @@ -24,7 +24,7 @@ public record MatchClock(int Seconds, int AdditionalSeconds, MatchMinute Minute) /// The second half starts at half the match duration whatever the first half actually cost, /// so an over-running first half does not push the whole second half out of step. /// The minute an event is written down against follows that same reading, as a -/// — 35+2 rather than a 37 that would sort after the restart. +/// — 35+2 rather than a 37 nobody at the pitch would recognise. /// /// This is presentation only. What is stored stays the real elapsed time, so playing time, /// substitution timings and the season statistics are unaffected. @@ -56,7 +56,7 @@ public static MatchClock Build(Game game, GamePeriod? displayHalf, int elapsedSe // Once the half is played out the clock stands still at the cap, so the minute stands still // with it and the overrun is counted alongside as 35+1, 35+2 — the reading a scoreboard - // shows, and the only way several stoppage-time events keep their order. + // shows, and what an event in stoppage time is written down against. if (intoHalf >= halfSeconds) { return new MatchClock( @@ -70,20 +70,80 @@ public static MatchClock Build(Game game, GamePeriod? displayHalf, int elapsedSe /// /// The minute a substitution is written down against: the reading the clock showed when it was - /// made, which is the half's reading and not the raw elapsed time. Falls back to the raw minute - /// for a substitution whose half was not loaded — a wrong-looking minute beats claiming 1'. + /// made, which is the half's reading and not the raw elapsed time. /// public static MatchMinute MinuteOf(Game game, GameSubstitution substitution) => - game.Periods.FirstOrDefault(p => p.Id == substitution.GamePeriodId) is { } half - ? Build(game, half, substitution.AtSeconds).Minute - : PlainMinute(substitution.AtSeconds); + MinuteAt(game, substitution.GamePeriodId, substitution.AtSeconds); /// - /// The minute a goal was written down against, or null for one recorded without a minute — - /// which the result page allows and the timeline then has nothing to place. + /// The minute a goal is written down against — derived the same way a substitution's is, from + /// the clock reading and the half it was scored in, so correcting a half's timings corrects + /// its goals with it. A goal with no clock behind it falls back to the minute stored on the + /// row, and one with neither has no minute at all, which the result page allows. /// - public static MatchMinute? MinuteOf(GameGoal goal) => - goal.Minute is { } minute ? new MatchMinute(minute, goal.AdditionalMinute) : null; + public static MatchMinute? MinuteOf(Game game, GameGoal goal) => goal switch + { + { AtSeconds: { } at } => MinuteAt(game, goal.GamePeriodId, at), + { Minute: { } minute } => new MatchMinute(minute, 0), + _ => null + }; + + /// + /// Where a goal sits on the elapsed match clock — the scale the timeline is ordered on, and + /// the one a substitution is already stored in. + /// + /// A goal logged from the touchline has that reading on the row. A goal typed in on the result + /// page has only the minute somebody wrote, which is a scoreboard reading, and the two + /// scales part company the moment a half over-runs: on a 60-minute match whose first half ran + /// to 33, the scoreboard's 32' is 34 minutes of elapsed play. Reading the typed minute as + /// though it were elapsed time filed it before the restart, under the half-time rule and ahead + /// of goals that were really scored first, so it is converted back through the half timings + /// here — the same arithmetic does, run the other way. + /// + /// + public static int ElapsedOf(Game game, GameGoal goal) + { + if (goal.AtSeconds is { } at) return at; + if (goal.Minute is not { } minute) return 0; + + // The start of the minute written down, on the scoreboard's scale. + var onScoreboard = Math.Max(0, (minute - 1) * 60); + + var halfSeconds = GameSplitType.Halves.PeriodDurationSeconds(game.GameDurationMinutes); + if (halfSeconds <= 0) return onScoreboard; + + // A match never run from the touchline has no timings to convert through, and the fallbacks + // are what Build assumes in the same position — so its goals keep the order the typed + // minutes put them in, which is the only order they have. + return onScoreboard < halfSeconds + ? (HalfKickedOffAt(game, PeriodType.FirstHalf) ?? 0) + onScoreboard + : (HalfKickedOffAt(game, PeriodType.SecondHalf) ?? halfSeconds) + (onScoreboard - halfSeconds); + } + + /// + /// The half an event belongs to, which is what puts the half-time break on the timeline. Its + /// own line-up's half when it has one; otherwise whichever side of the second half's kick-off + /// its elapsed reading falls, which is all a goal typed in by hand leaves to go on. First half + /// when nothing says otherwise — a match never run from the touchline has no kick-off to be + /// past, and one unbroken list is the honest way to show it. + /// + public static PeriodType HalfOf(Game game, int? periodId, int atSeconds) => + FindPeriod(game, periodId) is { } period ? period.PeriodType.Half() + : HalfKickedOffAt(game, PeriodType.SecondHalf) is { } restart && atSeconds >= restart + ? PeriodType.SecondHalf + : PeriodType.FirstHalf; + + /// + /// The reading the clock showed at a moment in a given half. Falls back to the raw minute when + /// that half was not loaded — a wrong-looking minute beats claiming 1'. + /// + private static MatchMinute MinuteAt(Game game, int? periodId, int atSeconds) => + FindPeriod(game, periodId) is { } half + ? Build(game, half, atSeconds).Minute + : PlainMinute(atSeconds); + + private static GamePeriod? FindPeriod(Game game, int? periodId) => + periodId is { } id ? game.Periods.FirstOrDefault(p => p.Id == id) : null; /// The minute a plain clock reading falls in. The first minute of play is 1'. private static MatchMinute PlainMinute(int seconds) => new((seconds / 60) + 1, 0); diff --git a/src/FootballFormation.Core/Reporting/ScoreProgressionReport.cs b/src/FootballFormation.Core/Reporting/ScoreProgressionReport.cs index e2e5dac..ce663e0 100644 --- a/src/FootballFormation.Core/Reporting/ScoreProgressionReport.cs +++ b/src/FootballFormation.Core/Reporting/ScoreProgressionReport.cs @@ -18,18 +18,19 @@ namespace FootballFormation.Core.Reporting; /// public static class ScoreProgressionReport { - /// Every goal in the match, in any order. - public static IReadOnlyDictionary Build(IEnumerable goals) + /// The match, for the half timings a typed-in minute is placed through. + public static IReadOnlyDictionary Build(Game game) { var progression = new Dictionary(); var us = 0; var them = 0; - // The order the live screen shows events in, read forwards: the match minute first — both - // halves of it, so a stoppage-time goal stays inside the half it was scored in — then the - // moment it was entered, then the id. See LiveMatch.Timeline for why all three. - var chronological = goals - .OrderBy(g => MatchClockReport.MinuteOf(g) ?? default) + // The order the live screen shows events in, read forwards: the elapsed match clock first, + // which runs on across the break and so keeps a stoppage-time goal inside the half it was + // scored in, then the moment it was entered, then the id. See LiveMatch.Timeline for why + // all three. + var chronological = game.Goals + .OrderBy(g => MatchClockReport.ElapsedOf(game, g)) .ThenBy(g => g.RecordedAt) .ThenBy(g => g.Id); diff --git a/src/FootballFormation.Core/Services/MatchGoalService.cs b/src/FootballFormation.Core/Services/MatchGoalService.cs index 092bdba..77fd5a1 100644 --- a/src/FootballFormation.Core/Services/MatchGoalService.cs +++ b/src/FootballFormation.Core/Services/MatchGoalService.cs @@ -1,6 +1,5 @@ using FootballFormation.Core.Data; using FootballFormation.Core.Models; -using FootballFormation.Core.Reporting; using FootballFormation.Core.Security; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; @@ -9,7 +8,8 @@ namespace FootballFormation.Core.Services; /// /// Goals as they are logged at the touchline. What this adds is the one thing only a match in -/// progress knows: the minute the clock showed when the ball went in. Storing the goal — and, at +/// progress knows: where in the match the ball went in — the half being played and the reading on +/// the clock, the same pair a substitution carries. Storing the goal — and, at /// the touchline, recounting the scoreline in the same save — is delegated to /// , so there is one implementation of it and the two rows are written /// together rather than across two contexts. @@ -34,27 +34,23 @@ public Task> LogGoalAsync( { await using var db = await dbFactory.CreateDbContextAsync(cancellationToken); - // Line-ups included: the minute follows the scoreboard clock, which is measured from - // the half being played rather than from kick-off. + // Line-ups included: the half being played is half of what places the goal. var game = await db.LoadWithPeriodsAsync(gameId, cancellationToken); if (game is null) return LiveMatchQueries.GameNotFound(gameId); if (scorerId is null && !isOpponentGoal) return Result.Failure("A goal for us needs a scorer"); - var clock = MatchClockReport.Build( - game, game.CurrentOrLastHalf(), game.ElapsedSecondsAt(UtcNow)); - var goal = new GameGoal { GameId = gameId, ScorerId = scorerId, AssisterId = assisterId, - // The minute the clock showed, so an over-running first half does not push every - // second-half goal out by the overrun. Stoppage time is kept in the second half of - // the pair — see GameGoal.AdditionalMinute for why it is not folded into the first. - Minute = clock.Minute.Minute, - AdditionalMinute = clock.Minute.Additional, + // Where it happened, not what a scoreboard made of it. The displayed minute is + // derived from these two, so a half whose timings are corrected later takes its + // goals with it — exactly as it already does its substitutions. + GamePeriodId = game.CurrentOrLastHalf()?.Id, + AtSeconds = game.ElapsedSecondsAt(UtcNow), IsOwnGoal = isOwnGoal, IsOpponentGoal = isOpponentGoal }; diff --git a/src/FootballFormation.UI/Pages/LiveMatch.razor b/src/FootballFormation.UI/Pages/LiveMatch.razor index 579aae0..b0d7191 100644 --- a/src/FootballFormation.UI/Pages/LiveMatch.razor +++ b/src/FootballFormation.UI/Pages/LiveMatch.razor @@ -197,11 +197,19 @@
@foreach (var entry in Timeline) { + @* The list runs newest first, so the break lands between the second half's + events and the first's — the only thing on the timeline that is not + something somebody did. *@ + @if (entry.HalfTimeAbove) + { +
@L["Half time"]
+ } + var against = entry.Goal is { } g && (g.IsOwnGoal || g.IsOpponentGoal);
- @(entry.Minute)' + @(entry.Minute is { } at ? $"{at}'" : "—") + Icon="@(entry.Goal is not null ? Icons.Material.Filled.SportsSoccer : Icons.Material.Filled.SwapHoriz)" />
@if (entry.Goal is { } goal) { diff --git a/src/FootballFormation.UI/Pages/LiveMatch.razor.cs b/src/FootballFormation.UI/Pages/LiveMatch.razor.cs index bc4771f..bddc8d9 100644 --- a/src/FootballFormation.UI/Pages/LiveMatch.razor.cs +++ b/src/FootballFormation.UI/Pages/LiveMatch.razor.cs @@ -13,18 +13,23 @@ namespace FootballFormation.UI.Pages; /// /// One entry on the match timeline — a goal or a substitution — so both can be listed together. -/// is the scoreboard reading rather than the raw elapsed time, which is -/// what keeps a first-half stoppage entry above the restart instead of below it. -/// orders events that share a minute, which the minute alone cannot. +/// is the elapsed match clock, which is what orders the list: it runs +/// on across the break, so a first-half stoppage entry stays above the restart without anyone +/// comparing scoreboard readings. is that reading, for display only, and +/// null for a goal recorded without one. +/// orders events that share a second, which the clock alone cannot. /// settles the rest: two entries of the same kind entered in one instant /// share a , and rows older than that column all read /// 0001-01-01. Across the two kinds the ids come from different tables, so a tie there is /// arbitrary — but it is stable, which is what the list needs. +/// is where the break falls; marks the one +/// entry the break is drawn above, which only a neighbour can decide. /// is the scoreline as it stood after a goal, and null for a substitution. /// public record MatchEvent( - MatchMinute Minute, DateTime RecordedAt, int Id, bool IsGoal, GameGoal? Goal, - GameSubstitution? Substitution, MatchScore? Score = null); + int AtSeconds, MatchMinute? Minute, PeriodType Half, DateTime RecordedAt, int Id, + GameGoal? Goal, GameSubstitution? Substitution, MatchScore? Score = null, + bool HalfTimeAbove = false); /// /// The sideline screen. An admin runs the clock and records what happens; everyone else sees the @@ -255,23 +260,41 @@ private List Timeline // Counted forwards over the whole match, then looked up per goal: this list runs // newest first, so a total accumulated while rendering it would count down. - var progression = ScoreProgressionReport.Build(GameData.Goals); + var progression = ScoreProgressionReport.Build(GameData); - var goals = GameData.Goals.Select(g => new MatchEvent( - MatchClockReport.MinuteOf(g) ?? default, g.RecordedAt, g.Id, true, g, null, progression[g.Id])); + var goals = GameData.Goals.Select(g => + { + var at = MatchClockReport.ElapsedOf(GameData, g); + return new MatchEvent( + at, + MatchClockReport.MinuteOf(GameData, g), + MatchClockReport.HalfOf(GameData, g.GamePeriodId, at), + g.RecordedAt, g.Id, g, null, progression[g.Id]); + }); IEnumerable subs = ShowSubstitutions ? GameData.Substitutions.Select(s => new MatchEvent( - MatchClockReport.MinuteOf(GameData, s), s.RecordedAt, s.Id, false, null, s)) + s.AtSeconds, + MatchClockReport.MinuteOf(GameData, s), + MatchClockReport.HalfOf(GameData, s.GamePeriodId, s.AtSeconds), + s.RecordedAt, s.Id, null, s)) : []; - // A goal and the sub that followed it commonly share a minute; the entry time keeps + // A goal and the sub that followed it commonly share a second; the entry time keeps // them in the order they actually happened rather than the order they were queried. // The id then settles a double substitution, so the entry this list shows on top is // the one MatchSubstitutionService.RemoveSubstitutionAsync will let an admin undo. - return [.. goals.Concat(subs) - .OrderByDescending(e => e.Minute) + var ordered = goals.Concat(subs) + .OrderByDescending(e => e.AtSeconds) .ThenByDescending(e => e.RecordedAt) - .ThenByDescending(e => e.Id)]; + .ThenByDescending(e => e.Id) + .ToList(); + + // Where the second half's events give way to the first's, reading down a list that + // runs newest first. Marked here because the markup renders one entry at a time and + // cannot see the one above it — and because the filter above decides who the + // neighbours are. + return [.. ordered.Select((e, i) => + i > 0 && ordered[i - 1].Half != e.Half ? e with { HalfTimeAbove = true } : e)]; } } diff --git a/src/FootballFormation.UI/Pages/LiveMatch.razor.css b/src/FootballFormation.UI/Pages/LiveMatch.razor.css index 86406cf..2df7883 100644 --- a/src/FootballFormation.UI/Pages/LiveMatch.razor.css +++ b/src/FootballFormation.UI/Pages/LiveMatch.razor.css @@ -182,6 +182,28 @@ border-bottom: none; } +/* Half time: a dashed rule with the words sitting in it, so the two halves read apart at a glance + without the break looking like another event. The row above it keeps its own solid rule, which + is why nothing here draws a top border. */ +.live-event-break { + display: flex; + align-items: center; + gap: 10px; + padding: 6px 0; + font-size: 0.7rem; + font-weight: 700; + letter-spacing: 0.06em; + text-transform: uppercase; + color: var(--ink-faint); +} + +.live-event-break::before, +.live-event-break::after { + content: ""; + flex: 1; + border-top: 1px dashed color-mix(in srgb, var(--ink) 25%, transparent); +} + .live-event-min { min-width: 34px; font-weight: 700; diff --git a/src/FootballFormation.UI/Pages/MatchResult.razor b/src/FootballFormation.UI/Pages/MatchResult.razor index 66b5d0b..1a9882c 100644 --- a/src/FootballFormation.UI/Pages/MatchResult.razor +++ b/src/FootballFormation.UI/Pages/MatchResult.razor @@ -74,14 +74,16 @@ @if (GameData.Goals.Count > 0) {
- @* Both halves of the minute, so a goal in first-half stoppage time keeps its place - above the restart rather than sorting past it — see MatchMinute. *@ - @foreach (var goal in GameData.Goals.OrderBy(g => MatchClockReport.MinuteOf(g) ?? default)) + @* Ordered on the elapsed match clock, which runs on across the break, so a goal in + first-half stoppage time keeps its place above the restart rather than sorting + past it. A minute typed in on this page is a scoreboard reading and is converted + onto that clock — see MatchClockReport.ElapsedOf. *@ + @foreach (var goal in GameData.Goals.OrderBy(g => MatchClockReport.ElapsedOf(GameData, g))) { @* Own goals and opponent goals both count against us, so they share the styling. *@
- @(MatchClockReport.MinuteOf(goal) is { } at ? $"{at}'" : "—") + @(MatchClockReport.MinuteOf(GameData, goal) is { } at ? $"{at}'" : "—")
diff --git a/tests/FootballFormation.Core.Tests/GoalClockBackfillTests.cs b/tests/FootballFormation.Core.Tests/GoalClockBackfillTests.cs new file mode 100644 index 0000000..cf81b47 --- /dev/null +++ b/tests/FootballFormation.Core.Tests/GoalClockBackfillTests.cs @@ -0,0 +1,181 @@ +using FootballFormation.Core.Data; +using FootballFormation.Core.Reporting; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; + +namespace FootballFormation.Core.Tests; + +/// +/// The one migration in here that rewrites rows rather than adding columns to them. +/// +/// StoreGoalPeriodAndClock moves goals logged in stoppage time onto the clock, and it runs +/// unattended against the live volume on the next deploy — so what it does is checked here rather +/// than only rehearsed by hand. Every other migration is exercised implicitly by +/// ServiceTestBase, which builds the schema from the model; this one has a body worth +/// asserting on, and the property that matters is not a column value but what the app then shows: +/// a goal written 30+2 still reads 30+2. +/// +/// +/// Migrated rather than created, because a backfill only exists on the path from the old schema. +/// Two contexts over one held-open in-memory connection, the same arrangement +/// ServiceTestBase uses. +/// +/// +public class GoalClockBackfillTests : IDisposable +{ + private const string BeforeTheBackfill = "20260813062317_AddGoalAdditionalMinute"; + + /// Ids given to the two halves below, so the assertions can name them. + private const int FirstHalfId = 10; + private const int SecondHalfId = 11; + + private readonly SqliteConnection _connection; + + public GoalClockBackfillTests() + { + _connection = new SqliteConnection("Filename=:memory:"); + _connection.Open(); + } + + private AppDbContext Context() => + new(new DbContextOptionsBuilder() + .UseSqlite(_connection) + .AddInterceptors(new DateInSqlInterceptor()) + .Options); + + /// + /// A 60-minute match in halves whose first half over-ran to 33:00, so the second half kicked + /// off there and not at the 30:00 its scoreboard restarts at. That gap is the whole difficulty: + /// a reconstruction that ignored the real kick-off would put every second-half goal three + /// minutes out. + /// + private async Task SeedBeforeTheBackfillAsync() + { + await using (var db = Context()) + await db.GetService().MigrateAsync(BeforeTheBackfill); + + var cmd = _connection.CreateCommand(); + cmd.CommandText = $""" + INSERT INTO Seasons (Name, StartDate, EndDate, IsCurrent) + VALUES ('25/26','2026-08-01','2027-06-30',1); + INSERT INTO Games (SeasonId, Opponent, Date, ScoreHome, ScoreAway, FormationType, + SplitType, GameDurationMinutes, IsHomeGame, MatchState, + ClockAccumulatedSeconds, MatchType, GuestPlayerIds, UnavailablePlayerIds) + VALUES (1,'Opp','2026-08-10',0,4,0,0,60,1,2,4200,0,'',''); + INSERT INTO GamePeriods (Id, GameId, PeriodType, StartedAtSeconds, EndedAtSeconds) + VALUES ({FirstHalfId},1,0,0,1980), ({SecondHalfId},1,1,1980,4200); + + -- Written by the live screen as a minute and an overrun, the shape being replaced. + INSERT INTO GameGoals (Id, GameId, Minute, AdditionalMinute, IsOwnGoal, IsOpponentGoal, RecordedAt) + VALUES (1,1,30,2,0,1,'2026-08-10 10:32:00'), -- first-half stoppage + (2,1,60,1,0,1,'2026-08-10 11:05:00'), -- second-half stoppage + (3,1,37, 0,0,1,'2026-08-10 10:40:00'), -- a plain minute: ambiguous + (4,1,NULL,0,0,1,'2026-08-10 10:41:00'); -- no minute at all + """; + await cmd.ExecuteNonQueryAsync(); + + await using (var db = Context()) await db.Database.MigrateAsync(); + } + + [Fact] + public async Task A_goal_scored_in_stoppage_time_still_reads_as_stoppage_time_afterwards() + { + await SeedBeforeTheBackfillAsync(); + + await using var db = Context(); + var game = await db.Games.Include(g => g.Periods).Include(g => g.Goals).FirstAsync(); + + string Shown(int goalId) => + MatchClockReport.MinuteOf(game, game.Goals.Single(g => g.Id == goalId))?.ToString() ?? "—"; + + // 32 would be the 32nd minute — two minutes into the second half — which is not when + // either of these was scored. + Assert.Equal("30+2", Shown(1)); + Assert.Equal("60+1", Shown(2)); + + Assert.Equal("37", Shown(3)); + Assert.Equal("—", Shown(4)); + } + + [Fact] + public async Task The_overrun_is_rewritten_as_the_half_and_the_clock_it_was_scored_on() + { + await SeedBeforeTheBackfillAsync(); + + await using var db = Context(); + var goals = await db.GameGoals.ToDictionaryAsync(g => g.Id); + + // 30:00 into a first half that started at 0, plus the minute of stoppage already counted. + Assert.Equal(FirstHalfId, goals[1].GamePeriodId); + Assert.Equal(31 * 60, goals[1].AtSeconds); + + // The second half really kicked off at 33:00, so its 60+1 is 63 minutes of play — not the + // 61 the scoreboard reading alone would have suggested. + Assert.Equal(SecondHalfId, goals[2].GamePeriodId); + Assert.Equal(63 * 60, goals[2].AtSeconds); + } + + /// + /// A goal without an overrun says nothing about which half it belongs to — a stored 37 on a + /// 30-minute half could be a minute somebody typed on the result page. Guessing one is how the + /// frozen minute went wrong in the first place, so these rows are left exactly as they are. + /// + [Fact] + public async Task A_goal_with_only_a_minute_is_left_alone_rather_than_guessed_at() + { + await SeedBeforeTheBackfillAsync(); + + await using var db = Context(); + var goals = await db.GameGoals.ToDictionaryAsync(g => g.Id); + + Assert.Null(goals[3].GamePeriodId); + Assert.Null(goals[3].AtSeconds); + Assert.Equal(37, goals[3].Minute); + + Assert.Null(goals[4].GamePeriodId); + Assert.Null(goals[4].AtSeconds); + Assert.Null(goals[4].Minute); + } + + /// + /// The reason the backfill is worth doing at all: on the elapsed clock the rewritten goals sit + /// inside the halves they were scored in, so the timeline and the running score agree with the + /// minutes on screen. + /// + [Fact] + public async Task The_rewritten_goals_sort_inside_the_halves_they_were_scored_in() + { + await SeedBeforeTheBackfillAsync(); + + await using var db = Context(); + var game = await db.Games.Include(g => g.Periods).Include(g => g.Goals).FirstAsync(); + + var order = game.Goals + .OrderBy(g => MatchClockReport.ElapsedOf(game, g)) + .ThenBy(g => g.RecordedAt) + .Select(g => g.Id); + + // No minute, then 30+2, then the 37th minute, then 60+1. + Assert.Equal([4, 1, 3, 2], order); + } + + [Fact] + public async Task The_migration_leaves_the_foreign_keys_intact() + { + await SeedBeforeTheBackfillAsync(); + + var check = _connection.CreateCommand(); + check.CommandText = "PRAGMA foreign_key_check"; + await using var violations = await check.ExecuteReaderAsync(); + + Assert.False(await violations.ReadAsync(), "the rebuilt GameGoals table has a broken row"); + } + + public void Dispose() + { + _connection.Dispose(); + GC.SuppressFinalize(this); + } +} diff --git a/tests/FootballFormation.Core.Tests/MatchClockReportTests.cs b/tests/FootballFormation.Core.Tests/MatchClockReportTests.cs index 98405b3..c13f4eb 100644 --- a/tests/FootballFormation.Core.Tests/MatchClockReportTests.cs +++ b/tests/FootballFormation.Core.Tests/MatchClockReportTests.cs @@ -149,10 +149,12 @@ public void The_minute_stops_with_the_clock_and_additional_time_is_counted_besid /// /// The whole point of the pair: a goal in first-half stoppage time and one just after the - /// restart are a minute apart, and a single number would have written them 32 and 31. + /// restart are a minute apart, and a single counted-on number would have written them 32 and + /// 31. Which of them came first is the elapsed clock's answer, not the pair's — the pair only + /// has to say something a scoreboard would. /// [Fact] - public void A_stoppage_time_minute_comes_before_the_first_minute_of_the_next_half() + public void A_stoppage_time_minute_stays_inside_the_half_it_was_played_in() { var game = QuartersGame(); Period(game, PeriodType.FirstQuarter).StartedAtSeconds = 0; @@ -162,8 +164,8 @@ public void A_stoppage_time_minute_comes_before_the_first_minute_of_the_next_hal var afterTheBreak = MatchClockReport.Build(game, Period(game, PeriodType.ThirdQuarter), 32 * 60); Assert.Equal(new MatchMinute(30, 2), stoppage.Minute); + Assert.Equal("30+2", stoppage.Minute.ToString()); Assert.Equal(new MatchMinute(31, 0), afterTheBreak.Minute); - Assert.True(stoppage.Minute.CompareTo(afterTheBreak.Minute) < 0); } [Fact] @@ -199,12 +201,137 @@ public void A_substitution_is_written_against_the_clock_its_own_half_was_showing Assert.Equal(new MatchMinute(35, 0), MatchClockReport.MinuteOf(game, orphan)); } + /// + /// A goal is placed the same way a substitution is, off its own half's clock — which is the + /// point of storing the pair on the row rather than the minute they produce. + /// [Fact] - public void A_goal_recorded_without_a_minute_has_none_to_show() + public void A_goal_is_written_against_the_clock_its_own_half_was_showing() { - Assert.Null(MatchClockReport.MinuteOf(new GameGoal())); - Assert.Equal(new MatchMinute(35, 2), - MatchClockReport.MinuteOf(new GameGoal { Minute = 35, AdditionalMinute = 2 })); + var game = QuartersGame(); + Period(game, PeriodType.FirstQuarter).StartedAtSeconds = 0; + var secondHalf = Period(game, PeriodType.ThirdQuarter); + secondHalf.StartedAtSeconds = 32 * 60; + secondHalf.Id = 7; + + var goal = new GameGoal { GamePeriodId = 7, AtSeconds = 34 * 60 }; + Assert.Equal(new MatchMinute(33, 0), MatchClockReport.MinuteOf(game, goal)); + + // Played out and still going: the reading a scoreboard would show, not a counted-on 32. + var stoppage = new GameGoal { GamePeriodId = 7, AtSeconds = 63 * 60 }; + Assert.Equal(new MatchMinute(60, 2), MatchClockReport.MinuteOf(game, stoppage)); + } + + /// + /// Correcting a half's timings corrects the goals scored in it. That is what deriving the + /// minute buys over storing it, and it is the whole reason the column moved. + /// + [Fact] + public void Moving_a_halfs_kick_off_moves_the_goals_scored_in_it() + { + var game = QuartersGame(); + Period(game, PeriodType.FirstQuarter).StartedAtSeconds = 0; + var secondHalf = Period(game, PeriodType.ThirdQuarter); + secondHalf.Id = 7; + secondHalf.StartedAtSeconds = 32 * 60; + + // Eight minutes into a second half whose clock starts at 30: the 39th minute. + var goal = new GameGoal { GamePeriodId = 7, AtSeconds = 40 * 60 }; + Assert.Equal(new MatchMinute(39, 0), MatchClockReport.MinuteOf(game, goal)); + + // The half really kicked off three minutes later than recorded, so the goal moves back + // with it. A stored minute would have stayed where it was and disagreed with the + // substitutions around it. + secondHalf.StartedAtSeconds = 35 * 60; + Assert.Equal(new MatchMinute(36, 0), MatchClockReport.MinuteOf(game, goal)); + } + + [Fact] + public void A_goal_with_no_clock_behind_it_falls_back_to_the_minute_on_the_row() + { + var game = QuartersGame(); + + // Typed in on the result page, and every goal logged before the clock reading was stored. + Assert.Equal(new MatchMinute(35, 0), + MatchClockReport.MinuteOf(game, new GameGoal { Minute = 35 })); + + // Neither one nor the other: the result page allows a goal with no minute at all. + Assert.Null(MatchClockReport.MinuteOf(game, new GameGoal())); + } + + /// + /// The timeline orders on elapsed seconds, so a minute typed in by hand has to be converted + /// onto that scale rather than read as though it already were one. The two only agree while + /// the halves run to length: this first half is three minutes long, and from the restart the + /// scoreboard trails the elapsed clock by exactly that. + /// + [Fact] + public void A_typed_in_minute_is_converted_onto_the_elapsed_clock_through_its_half() + { + var game = QuartersGame(); + Period(game, PeriodType.FirstQuarter).StartedAtSeconds = 0; + Period(game, PeriodType.ThirdQuarter).StartedAtSeconds = 33 * 60; + + int Elapsed(int minute) => MatchClockReport.ElapsedOf(game, new GameGoal { Minute = minute }); + + // First half: the scoreboard is the elapsed clock, because it kicked off at zero. + Assert.Equal(0, Elapsed(1)); + Assert.Equal(20 * 60, Elapsed(21)); + + // Second half: 31' is the first minute after a scoreboard restart at 30, which really + // happened at 33:00. Read as elapsed seconds it would have landed at 30:00 — before a goal + // scored in first-half stoppage time, and on the wrong side of the half-time rule. + Assert.Equal(33 * 60, Elapsed(31)); + Assert.Equal(35 * 60, Elapsed(33)); + Assert.Equal(PeriodType.SecondHalf, MatchClockReport.HalfOf(game, null, Elapsed(31))); + + // The clock on the row always wins — it is the reading, not a reconstruction of one. + Assert.Equal(1234, MatchClockReport.ElapsedOf(game, new GameGoal { AtSeconds = 1234, Minute = 5 })); + + // Nothing at all: the top of the match, which is where a goal with no minute has always sat. + Assert.Equal(0, MatchClockReport.ElapsedOf(game, new GameGoal())); + } + + /// + /// A match nobody ran from the touchline has no timings to convert through, so the typed + /// minutes keep the only order they have — and go on reading exactly as they did before goals + /// carried a clock at all. + /// + [Fact] + public void Typed_in_minutes_stand_on_their_own_when_no_half_was_ever_kicked_off() + { + var game = QuartersGame(); + + Assert.Equal(0, MatchClockReport.ElapsedOf(game, new GameGoal { Minute = 1 })); + Assert.Equal(30 * 60, MatchClockReport.ElapsedOf(game, new GameGoal { Minute = 31 })); + Assert.Equal(59 * 60, MatchClockReport.ElapsedOf(game, new GameGoal { Minute = 60 })); + } + + /// + /// Where the timeline draws half time. An event knows its own half; a goal typed in by hand + /// knows only a clock reading, and the second half's kick-off is the line it falls one side of. + /// + [Fact] + public void An_event_belongs_to_the_half_its_line_up_played_or_to_the_side_of_the_restart_it_falls() + { + var game = QuartersGame(); + Period(game, PeriodType.FirstQuarter).StartedAtSeconds = 0; + var secondHalf = Period(game, PeriodType.ThirdQuarter); + secondHalf.Id = 7; + secondHalf.StartedAtSeconds = 32 * 60; + + // Q4 is planned for the middle of the second half, and is still the second half. + var fourth = Period(game, PeriodType.FourthQuarter); + fourth.Id = 9; + + Assert.Equal(PeriodType.SecondHalf, MatchClockReport.HalfOf(game, 7, 34 * 60)); + Assert.Equal(PeriodType.SecondHalf, MatchClockReport.HalfOf(game, 9, 34 * 60)); + Assert.Equal(PeriodType.FirstHalf, MatchClockReport.HalfOf(game, null, 31 * 60)); + Assert.Equal(PeriodType.SecondHalf, MatchClockReport.HalfOf(game, null, 32 * 60)); + + // Nothing kicked off after the break, so there is no line to be the far side of. + secondHalf.StartedAtSeconds = null; + Assert.Equal(PeriodType.FirstHalf, MatchClockReport.HalfOf(game, null, 55 * 60)); } [Fact] diff --git a/tests/FootballFormation.Core.Tests/MatchGoalServiceTests.cs b/tests/FootballFormation.Core.Tests/MatchGoalServiceTests.cs index 68eda05..47df388 100644 --- a/tests/FootballFormation.Core.Tests/MatchGoalServiceTests.cs +++ b/tests/FootballFormation.Core.Tests/MatchGoalServiceTests.cs @@ -1,6 +1,7 @@ using System.Data.Common; using FootballFormation.Core.Data; using FootballFormation.Core.Models; +using FootballFormation.Core.Reporting; using FootballFormation.Core.Services; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Diagnostics; @@ -9,34 +10,40 @@ namespace FootballFormation.Core.Tests; /// -/// What logging a goal at the touchline adds over storing one: the minute it is stamped with, and -/// the scoreline that has to follow from the goals on file. +/// What logging a goal at the touchline adds over storing one: where in the match it happened — +/// the half being played and the reading on the clock — and the scoreline that has to follow from +/// the goals on file. The minute anyone sees is derived from that pair; these tests are about the +/// pair being written, and about what it is read back as. /// public class MatchGoalServiceTests : LiveMatchTestBase { [Fact] - public async Task A_goal_is_stamped_with_the_minute_the_clock_showed_counting_from_one() + public async Task A_goal_is_stamped_with_the_half_being_played_and_the_clock_it_was_scored_on() { var game = await SeedGameAsync(); await MatchClock.StartMatchAsync(game.Id); var players = await PlayersAsync(); - - // Minute 0 reads oddly on a timeline, so the first minute of play is 1'. - var opening = await Goals.LogGoalAsync(game.Id, players[1].Id, null, false, false); - Assert.Equal(1, opening.Value!.Minute); + var firstHalf = (await ReloadAsync(game.Id)).Periods.Single(p => p.PeriodType == PeriodType.FirstHalf); Time.Advance(TimeSpan.FromSeconds(1500)); // 25:00 - var later = await Goals.LogGoalAsync(game.Id, players[1].Id, null, false, false); - Assert.Equal(26, later.Value!.Minute); + + var goal = await Goals.LogGoalAsync(game.Id, players[1].Id, null, false, false); + + Assert.Equal(firstHalf.Id, goal.Value!.GamePeriodId); + Assert.Equal(1500, goal.Value.AtSeconds); + + // Nothing presentational on the row: the minute is the clock's to derive, and stays null + // so that a half whose timings are corrected later takes its goals with it. + Assert.Null(goal.Value.Minute); } /// - /// The second half's clock starts at half the match however long the first half really took, - /// and the goal minute follows the clock — otherwise every second-half goal is pushed out by - /// the first half's overrun. + /// The half is recorded, not inferred from the clock reading. The two disagree the moment a + /// first half over-runs — this one is three minutes long past its 30 — and inferring is what + /// let a change to the match duration silently reinterpret goals already on file. /// [Fact] - public async Task A_second_half_goal_is_stamped_off_the_scoreboard_clock_not_the_elapsed_time() + public async Task A_second_half_goal_is_stamped_with_the_second_half_however_long_the_first_ran() { var game = await SeedGameAsync(); await MatchClock.StartMatchAsync(game.Id); @@ -50,16 +57,21 @@ public async Task A_second_half_goal_is_stamped_off_the_scoreboard_clock_not_the var goal = await Goals.LogGoalAsync(game.Id, players[1].Id, null, false, false); - // 35:xx on the clock, so the 36th minute — not the 39th the raw elapsed time would give. - Assert.Equal(36, goal.Value!.Minute); + var reloaded = await ReloadAsync(game.Id); + var secondHalf = reloaded.Periods.Single(p => p.PeriodType == PeriodType.SecondHalf); + Assert.Equal(secondHalf.Id, goal.Value!.GamePeriodId); + Assert.Equal(38 * 60, goal.Value.AtSeconds); + + // 35:xx on the scoreboard, so the 36th minute — not the 39th the raw elapsed time reads. + Assert.Equal(new MatchMinute(36, 0), MatchClockReport.MinuteOf(reloaded, goal.Value)); } /// - /// A goal after the half has been played out is written 30+2, not 32. The two halves of the - /// minute are stored apart because that is what keeps it above the restart on the timeline. + /// A goal after the half has been played out belongs to that half, and is shown 30+2 rather + /// than 32. The clock reading runs on past the cap; only the display stops at it. /// [Fact] - public async Task A_goal_in_stoppage_time_is_stamped_with_the_minute_it_is_added_to() + public async Task A_goal_in_stoppage_time_belongs_to_the_half_that_is_over_running() { var game = await SeedGameAsync(); await MatchClock.StartMatchAsync(game.Id); @@ -69,8 +81,11 @@ public async Task A_goal_in_stoppage_time_is_stamped_with_the_minute_it_is_added var goal = await Goals.LogGoalAsync(game.Id, players[1].Id, null, false, false); - Assert.Equal(30, goal.Value!.Minute); - Assert.Equal(2, goal.Value.AdditionalMinute); + var reloaded = await ReloadAsync(game.Id); + Assert.Equal(PeriodType.FirstHalf, + reloaded.Periods.Single(p => p.Id == goal.Value!.GamePeriodId).PeriodType); + Assert.Equal(31 * 60, goal.Value!.AtSeconds); + Assert.Equal(new MatchMinute(30, 2), MatchClockReport.MinuteOf(reloaded, goal.Value)); } [Fact] diff --git a/tests/FootballFormation.Core.Tests/ScoreProgressionReportTests.cs b/tests/FootballFormation.Core.Tests/ScoreProgressionReportTests.cs index 2e858ac..b721cbc 100644 --- a/tests/FootballFormation.Core.Tests/ScoreProgressionReportTests.cs +++ b/tests/FootballFormation.Core.Tests/ScoreProgressionReportTests.cs @@ -10,13 +10,21 @@ namespace FootballFormation.Core.Tests; ///
public class ScoreProgressionReportTests { + /// A 60-minute match — two 30-minute halves — carrying the goals it was scored in. + private static Game Match(params GameGoal[] goals) + { + var game = TestData.Game(durationMinutes: 60); + game.Goals.AddRange(goals); + return game; + } + + /// A goal off the touchline, placed by the elapsed match clock it was logged at. private static GameGoal Goal( - int id, int minute, int additional = 0, bool ownGoal = false, bool opponentGoal = false) => + int id, int minute, bool ownGoal = false, bool opponentGoal = false) => new() { Id = id, - Minute = minute, - AdditionalMinute = additional, + AtSeconds = (minute - 1) * 60, IsOwnGoal = ownGoal, IsOpponentGoal = opponentGoal, RecordedAt = new DateTime(2026, 8, 11, 14, 0, minute, DateTimeKind.Utc) @@ -32,7 +40,7 @@ public void Each_goal_carries_the_score_it_made_it() Goal(3, 30) }; - var progression = ScoreProgressionReport.Build(goals); + var progression = ScoreProgressionReport.Build(Match([.. goals])); Assert.Equal(new MatchScore(1, 0), progression[1]); Assert.Equal(new MatchScore(1, 1), progression[2]); @@ -44,7 +52,7 @@ public void An_own_goal_climbs_the_opponents_half_of_the_scoreline() { var goals = new List { Goal(1, 20, ownGoal: true) }; - var progression = ScoreProgressionReport.Build(goals); + var progression = ScoreProgressionReport.Build(Match([.. goals])); Assert.Equal(new MatchScore(0, 1), progression[1]); } @@ -54,7 +62,7 @@ public void The_goals_are_counted_in_the_order_they_were_scored_not_the_order_th { var goals = new List { Goal(2, 40), Goal(1, 10, opponentGoal: true) }; - var progression = ScoreProgressionReport.Build(goals); + var progression = ScoreProgressionReport.Build(Match([.. goals])); Assert.Equal(new MatchScore(0, 1), progression[1]); Assert.Equal(new MatchScore(1, 1), progression[2]); @@ -69,7 +77,7 @@ public void Two_goals_in_the_same_minute_are_separated_by_when_they_were_entered var second = Goal(4, 20); second.RecordedAt = first.RecordedAt.AddSeconds(20); - var progression = ScoreProgressionReport.Build([second, first]); + var progression = ScoreProgressionReport.Build(Match(second, first)); Assert.Equal(new MatchScore(0, 1), progression[7]); Assert.Equal(new MatchScore(1, 1), progression[4]); @@ -80,7 +88,7 @@ public void The_last_goals_score_is_the_final_score() { var goals = new List { Goal(1, 5), Goal(2, 25), Goal(3, 50, ownGoal: true) }; - var progression = ScoreProgressionReport.Build(goals); + var progression = ScoreProgressionReport.Build(Match([.. goals])); var final = progression[3]; Assert.Equal(Game.CountOurGoals(goals), final.Us); @@ -89,17 +97,66 @@ public void The_last_goals_score_is_the_final_score() /// /// A goal in first-half stoppage time was scored before one just after the restart, and the - /// running total has to follow that — a minute counted straight on would have made 30+2 read - /// as 32 and put it after the 31st minute of the second half. + /// running total follows that without anyone comparing scoreboard readings — the elapsed clock + /// runs on across the break, where the scoreboard reads 30+2 and then 31 all over again. /// [Fact] public void A_stoppage_time_goal_is_counted_inside_the_half_it_was_scored_in() { - var stoppage = Goal(1, 30, additional: 2); - var afterTheBreak = Goal(2, 31, opponentGoal: true); + // Two minutes past a 30-minute half, then a minute into a second half that kicked off at 32. + var stoppage = Goal(1, 32); + var afterTheBreak = Goal(2, 33, opponentGoal: true); afterTheBreak.RecordedAt = stoppage.RecordedAt.AddMinutes(16); - var progression = ScoreProgressionReport.Build([afterTheBreak, stoppage]); + var progression = ScoreProgressionReport.Build(Match(afterTheBreak, stoppage)); + + Assert.Equal(new MatchScore(1, 0), progression[1]); + Assert.Equal(new MatchScore(1, 1), progression[2]); + } + + /// + /// A goal typed in on the result page has no clock behind it, so the minute on the row is what + /// places it — which is the only thing keeping goals recorded before the clock was stored in + /// the order they have always had. + /// + [Fact] + public void A_goal_with_only_a_minute_is_counted_in_that_minute() + { + var typedIn = new GameGoal { Id = 1, Minute = 40 }; + var logged = Goal(2, 10, opponentGoal: true); + + var progression = ScoreProgressionReport.Build(Match(typedIn, logged)); + + Assert.Equal(new MatchScore(0, 1), progression[2]); + Assert.Equal(new MatchScore(1, 1), progression[1]); + } + + /// + /// A typed-in minute is a scoreboard reading, and on a match whose first half over-ran the + /// scoreboard and the elapsed clock disagree by the overrun. Counting the typed minute as + /// elapsed time filed a second-half goal ahead of one scored in first-half stoppage time, and + /// handed both the wrong running score. + /// + [Fact] + public void A_minute_typed_in_afterwards_is_placed_through_the_half_it_names() + { + // A first half whistled off three minutes long, so the second half kicks off at 33:00 + // while its scoreboard still starts at 30'. + var game = Match(); + game.AddPeriod(PeriodType.FirstHalf); + game.AddPeriod(PeriodType.SecondHalf); + game.Periods.Single(p => p.PeriodType == PeriodType.FirstHalf).StartedAtSeconds = 0; + game.Periods.Single(p => p.PeriodType == PeriodType.SecondHalf).StartedAtSeconds = 33 * 60; + + // Logged live two minutes into first-half stoppage — the scoreboard read 30+2. + var stoppage = new GameGoal { Id = 1, AtSeconds = 32 * 60 }; + + // Typed in afterwards as the 32nd minute, which is two minutes into the second half. + var typedIn = new GameGoal { Id = 2, Minute = 32, IsOpponentGoal = true }; + + game.Goals.AddRange([typedIn, stoppage]); + + var progression = ScoreProgressionReport.Build(game); Assert.Equal(new MatchScore(1, 0), progression[1]); Assert.Equal(new MatchScore(1, 1), progression[2]); @@ -108,6 +165,6 @@ public void A_stoppage_time_goal_is_counted_inside_the_half_it_was_scored_in() [Fact] public void A_match_with_no_goals_has_nothing_to_report() { - Assert.Empty(ScoreProgressionReport.Build([])); + Assert.Empty(ScoreProgressionReport.Build(Match())); } } diff --git a/tests/ui/specs/match-day.spec.js b/tests/ui/specs/match-day.spec.js index 7ac0479..70dfce4 100644 --- a/tests/ui/specs/match-day.spec.js +++ b/tests/ui/specs/match-day.spec.js @@ -239,6 +239,44 @@ test('the timeline can be narrowed to the goals', async ({ page }) => { await expect(page.locator('.live-bench')).toBeVisible(); }); +test('the timeline draws half time between the two halves', async ({ page }) => { + const id = await matchWithId(page, 'FC Rust'); + await fillLineup(page, 2); + + await goto(page, `/games/${id}/live`); + await clickFor( + page.getByRole('button', { name: 'Start match' }), + () => expect(page.getByRole('button', { name: 'Finish match' })).toBeVisible(), + ); + + const events = page.locator('.live-event'); + const halfTime = page.locator('.live-event-break'); + + // One goal in each half. Until the second one there is only one half on the list, and a break + // above the only thing on it would be a line drawn through nothing. + await clickFor(page.getByRole('button', { name: 'Goal against' }), () => expect(events).toHaveCount(1)); + await expect(halfTime).toHaveCount(0); + + const controls = page.locator('.live-controls'); + await clickFor( + controls.getByRole('button', { name: 'Half time' }), + () => expect(controls.getByRole('button', { name: 'Start 2nd Half' })).toBeVisible(), + ); + await clickFor( + controls.getByRole('button', { name: 'Start 2nd Half' }), + () => expect(controls.getByRole('button', { name: 'Half time' })).toHaveCount(0), + ); + + await clickFor(page.getByRole('button', { name: 'Goal against' }), () => expect(events).toHaveCount(2)); + + // Exactly one break, and it sits between the two — the list runs newest first, so the second + // half's goal is above it and the first half's below. + await expect(halfTime).toHaveCount(1); + await expect(halfTime).toHaveText('Half time'); + await expect(page.locator('.live-timeline > *')).toHaveCount(3); + await expect(page.locator('.live-timeline > *').nth(1)).toHaveClass(/live-event-break/); +}); + test('the playing-time table drops its estimate once the match has been run', async ({ page }) => { const id = await matchWithId(page, 'FC Speeltijd'); await fillLineup(page, 2); diff --git a/tests/ui/specs/selectors.spec.js b/tests/ui/specs/selectors.spec.js index 7afecc2..7da8229 100644 --- a/tests/ui/specs/selectors.spec.js +++ b/tests/ui/specs/selectors.spec.js @@ -31,7 +31,8 @@ const SELECTORS = { 'the formation builder': ['pitch', 'pitch-empty', 'pitch-player', 'draggable-player'], 'the playing-time table': ['playtime-table', 'pt-total', 'playtime-note'], 'the live screen': ['live-lineup', 'live-controls', 'live-score-value', 'live-score-away', - 'live-event', 'live-event-score', 'live-bench', 'live-timeline-toggle', + 'live-event', 'live-event-score', 'live-event-break', 'live-bench', + 'live-timeline-toggle', 'live-timeline', 'live-minutes-card', 'card-label', 'planned-row'], 'the phone layout': ['dialog-sheet', 'stacked-table', 'topbar-nav'], 'the squad': ['badge-archived'],