From 4b48a2cdaace6ab0ff7403464e1e66b6f93b8d85 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 19:51:35 +0000 Subject: [PATCH] Make the live screen say what it is about to do, and then do it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four fixes to the touchline screen, all of them about the gap between what a plan says and what has actually happened. The minutes table is headed "Planned minutes" until the first kick-off. Its figures there are the line-up costed at a full period each, which is not time anyone has played — and the numbers themselves cannot say which they are, so the heading does. "Show subs" folded the bench strip away, which was never the strip anyone wanted rid of. It now folds the substitutions out of the timeline instead, leaving the goals; a rotated squad buries them among swaps nobody is scrolling back for. The bench is always drawn. "Next line-up" leaves the clock controls for the foot of the "Changes at half-way" card, and the dialog that used to ask first goes with it: the list it now sits under is what that dialog was there to say. That only holds if the list is true, and it was not. Play overtakes a plan: once the player a planned swap takes off has gone off live, the difference between the two line-ups still names their slot but proposes withdrawing whoever came on for them. Those swaps are dropped from the card — and from what the button does, so an injury replacement keeps the place for the rest of the half instead of lasting exactly one quarter. AdvancePeriodAsync applies PlannedChangesReport's answer rather than forming a second one, because a card promising one thing while the button under it does another is worse than either behaviour alone. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AfrvWcNR8hMjw3ny6dMemX --- CLAUDE.md | 2 +- docs/architecture.md | 16 +-- docs/known_issues.md | 17 ++- docs/ui_components.md | 45 +++++-- .../Reporting/PlannedChangesReport.cs | 127 ++++++++++++++---- .../Services/MatchClockService.cs | 64 ++++++++- .../Components/PlannedChangesList.razor | 4 +- .../Helpers/DialogPrompts.cs | 17 --- .../Pages/LiveMatch.razor | 119 ++++++++-------- .../Pages/LiveMatch.razor.cs | 52 ++++--- .../Pages/LiveMatch.razor.css | 14 +- .../Pages/LiveNextLineupDialog.razor | 28 ---- .../Pages/LiveNextLineupDialog.razor.cs | 24 ---- .../Pages/LiveNextLineupDialog.razor.css | 5 - src/FootballFormation.UI/Strings.nl.resx | 5 +- src/FootballFormation.Web/wwwroot/app.css | 14 +- .../MatchClockServiceTests.cs | 80 +++++++++++ .../PlannedChangesReportTests.cs | 96 ++++++++++++- tests/ui/specs/match-day.spec.js | 65 ++++++--- tests/ui/specs/selectors.spec.js | 4 +- 20 files changed, 558 insertions(+), 240 deletions(-) delete mode 100644 src/FootballFormation.UI/Pages/LiveNextLineupDialog.razor delete mode 100644 src/FootballFormation.UI/Pages/LiveNextLineupDialog.razor.cs delete mode 100644 src/FootballFormation.UI/Pages/LiveNextLineupDialog.razor.css diff --git a/CLAUDE.md b/CLAUDE.md index 2de7002..aa6cdf3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -16,7 +16,7 @@ as in the markup. The UI is Dutch by default with English available. ```bash dotnet build -c Release # what CI builds — warnings are errors here (see below) -dotnet test # 389 tests, xUnit v3, real SQLite +dotnet test # 401 tests, xUnit v3, real SQLite cd src/FootballFormation.Web && dotnet run # http://localhost:5228 cd tests/ui && npm test # 39 Playwright tests in a browser, ~1 min (npm install first) scripts/visual-check.sh # screenshots every page, then measures every touch target diff --git a/docs/architecture.md b/docs/architecture.md index 3e9dbee..ec4f68a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -52,8 +52,10 @@ 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 period state from the stored anchor + banked total - PlannedChangesReport.cs — What the next period changes versus the one on the pitch (rendered by - UI/Components/PlannedChangesList, shared by the live card and its dialog) + PlannedChangesReport.cs — What the next period changes versus the one on the pitch, minus the + swaps play has already overtaken (Build() for the card in + UI/Components/PlannedChangesList, Swaps() for MatchClockService, which + applies the overtaken half rather than deciding again) ScoreProgressionReport.cs — The score after each goal (MatchScore), for the live timeline — counted forwards because that list runs newest first HealthReport.cs — Whether a booted container is actually serving: the /health payload and @@ -112,8 +114,6 @@ Pages/ LiveGoalDialog.razor(.cs) — Dialog: scorer, assister, own-goal toggle LiveSubDialog.razor(.cs)(.css) — Dialog: for a player tapped on the pitch, either a replacement from the bench or a position swap with someone already on - LiveNextLineupDialog.razor(.cs)(.css) — Dialog: what rolling on to the next line-up will change, - asked before it is done — advancing a period has no undo SeasonDialog.razor(.cs) — Dialog: season name, start date, end date Settings.razor(.cs) — /settings — Match preferences, own password, season management Users.razor(.cs) — /users — Accounts: add, edit, reset password, delete (Admin only) @@ -126,8 +126,8 @@ Components/ Pitch.razor(.cs)(.css) — The pitch. Read-only by default; Draggable for the builder, OnPlayerClicked for the live screen, Size for chip scale PlayerLabel.razor — A player as one line of text: "#7 Jasper" - PlannedChangesList.razor(.css) — What the next line-up does, as a team sheet; shared by the live - screen's card and the dialog that asks before applying it + PlannedChangesList.razor(.css) — What the next line-up does, as a team sheet, for the live + screen's "Changes at half-way" card CancellableComponent.cs — Base for any component that reads: owns the CancellationToken its service reads take, tripped when the component is disposed SeasonAwarePage.cs — Base for pages that follow the season picker (a CancellableComponent) @@ -150,8 +150,8 @@ Helpers/ PitchPositionHelper.cs — Maps PlayerPosition → (left%, top%) coordinates UiFeedback.cs — Snackbar.Report()/ReportFailure() over Result (translates the error, stays silent about a cancelled one), shared LockedDialog options - DialogPrompts.cs — ConfirmAsync()/ConfirmDeleteAsync(), and PromptAsync()/PromptValueAsync() - for an editing dialog that returns a value + DialogPrompts.cs — ConfirmAsync()/ConfirmDeleteAsync(), and PromptAsync() for an + editing dialog that returns a value LineupDragState.cs — In-flight drag on the formation builder PrincipalExtensions.cs — ClaimsPrincipal.IsAdmin()/DisplayName()/UserId(). Use IsAdmin(), never Identity.IsAuthenticated — the two only agree while Admin is the one role diff --git a/docs/known_issues.md b/docs/known_issues.md index b212ae5..f36adde 100644 --- a/docs/known_issues.md +++ b/docs/known_issues.md @@ -276,9 +276,10 @@ Avoid repeating these mistakes: specify different base classes*, because the generated Razor partial already declares `: ComponentBase`. Use `@inherits SeasonAwarePage` in the markup file. - **A generic dialog result can't tell `default` from "cancelled"**: `PromptAsync` - is constrained to `class` for that reason; a dialog returning a value type uses - `PromptValueAsync`, which hands back `TValue?`. A dialog closing with `0` is otherwise - indistinguishable from the user pressing Cancel. + is constrained to `class` for that reason. A dialog closing with `0` is otherwise + indistinguishable from the user pressing Cancel, so one returning a value type needs its own + helper handing back `TValue?` — there was a `PromptValueAsync` doing exactly that until its last + caller went, and adding another value-typed dialog means writing it again. ## Result - **A cancelled call is a failure with no message, and both halves matter.** Threading a @@ -346,11 +347,19 @@ Avoid repeating these mistakes: from the lineup as it finally stands, so a swap credits **the position moved into** for the whole period, earlier minutes included — the opposite of what its comment used to claim. Totals are right either way; only the split by position is affected, and a test pins it. +- **Advancing a period rewrites the next period's stored line-up.** `AdvancePeriodAsync` is not + purely a clock move: where a live substitution has already answered one of the next line-up's + swaps, it keeps the player who came on and benches the arrival the plan named — otherwise an + injury replacement is pulled straight back off at the quarter boundary. So the line-up the + formation builder shows for Q2 after a match has been run is not necessarily the one that was + saved, and that is deliberate rather than a lost edit. The rule is `PlannedChangesReport.Swaps`, + shared with the live screen's "Changes at half-way" card so the list and the button cannot part + ways; change one and the other follows. - **Removing a control does not remove the state it could leave behind.** Deleting pause/resume left `MatchState=InProgress` + a live period + `ClockRunningSince=null` unreachable going forward, but still storable by a row an older build wrote. `AdvancePeriodAsync` deliberately leaves the anchor alone — so on such a row it would roll on to the next line-up with the clock - still frozen, banking no minutes for the rest of the half while the dialog promised the opposite. + still frozen, banking no minutes for the rest of the half while the screen said it kept running. It now restarts a stopped anchor, which is a no-op for every game that was never paused. ## General diff --git a/docs/ui_components.md b/docs/ui_components.md index 49d2b94..5d86f4a 100644 --- a/docs/ui_components.md +++ b/docs/ui_components.md @@ -107,11 +107,11 @@ watches the same URL read-only. Every control sits in an `` instead. -- **`PlannedChangesList` is the one rendering of what the next line-up does.** The card and the - dialog show the same thing, so the markup and its `.planned-*` styling live in the component — - scoped CSS follows the file that owns the elements, so moving the markup and leaving the CSS - behind would have left both callers unstyled. + whose button says "Delete"). +- **The "Changes at half-way" card is what the mid-half line-up change is made from.** It lists + what `PlannedChangesReport` makes of the difference between the two planned line-ups + (`PlannedChangesList`, which owns the `.planned-*` styling), and carries the "Next line-up" + button underneath. Admin only, like the minutes table. It stays on screen while the change can + be made even when nothing differs — otherwise the only way on to the next quarter would vanish + exactly when nobody needs swapping — and appears before kick-off, without the button, as + something to read. +- **Only viable changes are listed, and only viable changes are made.** The report is handed the + substitutions already made in the period so it can rewind to the line-up that kicked off. A swap + whose outgoing player has since been taken off is dropped: the difference between the line-ups + still names their slot, but it now proposes withdrawing whoever came on for them, which nobody + planned. `AdvancePeriodAsync` drops the same swaps when it rolls the line-up on — it applies + `PlannedChangesReport.Swaps(...).Overtaken` rather than forming a second opinion, because a card + promising one thing while the button under it does another is worse than either behaviour alone. + An injury replacement therefore keeps the place for the rest of the half instead of lasting + exactly one quarter. - **Minutes played is admin-only** (`LiveMinutesReport`), and shows exact time on the pitch rather than the `periodsPlaying × periodDuration` estimate the planning screens use. It is a computed - property, so the running player's total climbs with the clock tick. + property, so the running player's total climbs with the clock tick. Until the first kick-off + there is no time played at all and the figures are the planned line-up costed at a full period + each, so the card is headed **"Planned minutes"** rather than "Minutes played" + (`Game.HasActualTimings`) — the numbers cannot say which they are, so the heading does. - **Mobile reorders the column with flex `order`**: what just happened matters more at a touchline than where everyone stands, so the line-up card (`.live-lineup`, `order: 1`) and the minutes table (`.live-minutes-card`, `order: 2`) drop below the timeline under 600px. Both rules live in diff --git a/src/FootballFormation.Core/Reporting/PlannedChangesReport.cs b/src/FootballFormation.Core/Reporting/PlannedChangesReport.cs index 7e15903..54d0ca6 100644 --- a/src/FootballFormation.Core/Reporting/PlannedChangesReport.cs +++ b/src/FootballFormation.Core/Reporting/PlannedChangesReport.cs @@ -11,6 +11,23 @@ public record PlannedSubstitution(Player? PlayerOff, Player? PlayerOn, PlayerPos /// A player who stays on the pitch but in a different position. public record PlannedMove(Player Player, PlayerPosition From, PlayerPosition To); +/// +/// The same swap in terms of the line-up rows rather than the players, which is what carrying one +/// out needs: a slot and a position change hands, and neither is a property of a name. +/// +public record PlannedSwap(GamePlayerPosition? Off, GamePlayerPosition? On); + +/// +/// The swaps the next line-up implies, split by whether play has already answered them. +/// +/// is the ones it has: the player the plan takes off went off live and +/// somebody came on for them, so the plan's arrival is no longer wanted and the slot's occupant is +/// no longer the player the plan meant to withdraw. is never null there — a +/// swap with nobody named to come off has nothing for play to overtake. +/// +/// +public record PlannedSwaps(List Viable, List Overtaken); + /// What the next line-up does: who is swapped, and who shifts position. public record PlannedChanges(List Substitutions, List Moves) { @@ -28,6 +45,13 @@ public record PlannedChanges(List Substitutions, List +/// +/// Only the swaps still open to the coach are reported. Play overtakes a plan: once the player it +/// takes off has been taken off live, the difference between the two line-ups still names their +/// slot, but it now proposes to withdraw whoever came on for them — a substitution nobody planned. +/// is the same walk without the names, and MatchClockService applies +/// what it calls overtaken rather than deciding again, so the card and the button cannot part ways. +/// /// public static class PlannedChangesReport { @@ -35,39 +59,97 @@ public static class PlannedChangesReport /// applied to it, so the changes shown stay true to who is actually on the pitch. /// The period whose line-up takes over. /// Resolves an id to a player; unknown ids come back as null. - public static PlannedChanges Build(GamePeriod current, GamePeriod next, Func findPlayer) + /// The substitutions already made in . + /// They decide which swaps are still worth showing — see . + public static PlannedChanges Build( + GamePeriod current, + GamePeriod next, + Func findPlayer, + IEnumerable liveChanges) { var before = StartersBySlot(current); var after = StartersBySlot(next); - var beforeIds = before.Values.Select(p => p.PlayerId).ToHashSet(); - var afterIds = after.Values.Select(p => p.PlayerId).ToHashSet(); - - // In slot order, so the list reads back to front like a team sheet rather than in - // whatever order the lineup rows happen to have been stored. - var leaving = before.OrderBy(e => e.Key).Select(e => e.Value) - .Where(p => !afterIds.Contains(p.PlayerId)).ToList(); - var arriving = after.OrderBy(e => e.Key).Select(e => e.Value) - .Where(p => !beforeIds.Contains(p.PlayerId)).ToList(); - return new PlannedChanges( - PairUp(leaving, arriving, before, findPlayer), + [.. PairUp(before, after, KickOffStarters(before.Values, liveChanges)).Viable + .Select(swap => Name(swap, findPlayer))], Moves(before, after, findPlayer)); } + /// + /// The same swaps as line-up rows, for the caller that has to carry them out rather than + /// print them. See for what the two halves mean. + /// + public static PlannedSwaps Swaps( + GamePeriod current, GamePeriod next, IEnumerable liveChanges) + { + var before = StartersBySlot(current); + + return PairUp(before, StartersBySlot(next), KickOffStarters(before.Values, liveChanges)); + } + + /// + /// A swap as the screen says it. The position is the one being taken over, which for a player + /// coming off with nobody named to replace them is the one they are vacating. + /// + private static PlannedSubstitution Name(PlannedSwap swap, Func findPlayer) => + new(swap.Off is null ? null : findPlayer(swap.Off.PlayerId), + swap.On is null ? null : findPlayer(swap.On.PlayerId), + (swap.On ?? swap.Off)!.Position); + + /// + /// Whether the coach can still make this swap as planned. It names the player the plan takes + /// off, and once the touchline has already taken them off the swap is about somebody else — + /// whoever inherited the slot — which is not what was planned for them. A swap with nobody + /// named to come off is kept: an unbalanced line-up is worth flagging. + /// + private static bool IsStillViable(PlannedSwap swap, HashSet kickOffStarters) => + swap.Off is null || kickOffStarters.Contains(swap.Off.PlayerId); + + /// + /// Who was on the pitch when the period kicked off. The line-up records where everyone stands + /// now, so rewinding the substitutions made since is the only way back to the eleven + /// the plan was written against — the same walk makes. + /// + private static HashSet KickOffStarters( + IEnumerable onPitchNow, IEnumerable liveChanges) + { + var starters = onPitchNow.Select(p => p.PlayerId).ToHashSet(); + + // Newest first, so a slot changing hands twice unwinds through the player who held it in + // between rather than skipping straight past them. The id settles a double substitution, + // where both changes share a second. + foreach (var sub in liveChanges.OrderByDescending(s => s.AtSeconds).ThenByDescending(s => s.Id)) + { + starters.Remove(sub.PlayerOnId); + starters.Add(sub.PlayerOffId); + } + + return starters; + } + /// /// Matches who goes off to who comes on. An arrival is paired with whoever held the slot they /// are taking, which is the swap a coach would call out; when that player is staying on the /// pitch — a shuffle rather than a straight swap — the next unpaired departure is used instead. /// - private static List PairUp( - List leaving, - List arriving, + private static PlannedSwaps PairUp( Dictionary before, - Func findPlayer) + Dictionary after, + HashSet kickOffStarters) { + var beforeIds = before.Values.Select(p => p.PlayerId).ToHashSet(); + var afterIds = after.Values.Select(p => p.PlayerId).ToHashSet(); + + // In slot order, so the list reads back to front like a team sheet rather than in + // whatever order the lineup rows happen to have been stored. + var leaving = before.OrderBy(e => e.Key).Select(e => e.Value) + .Where(p => !afterIds.Contains(p.PlayerId)).ToList(); + var arriving = after.OrderBy(e => e.Key).Select(e => e.Value) + .Where(p => !beforeIds.Contains(p.PlayerId)).ToList(); + var unpaired = new List(leaving); - var substitutions = new List(); + var swaps = new List(); foreach (var on in arriving) { @@ -77,17 +159,14 @@ private static List PairUp( if (off is not null) unpaired.Remove(off); - substitutions.Add(new PlannedSubstitution( - off is null ? null : findPlayer(off.PlayerId), - findPlayer(on.PlayerId), - on.Position)); + swaps.Add(new PlannedSwap(off, on)); } // Anyone left over comes off with nobody named to replace them. - substitutions.AddRange(unpaired.Select(off => - new PlannedSubstitution(findPlayer(off.PlayerId), null, off.Position))); + swaps.AddRange(unpaired.Select(off => new PlannedSwap(off, null))); - return substitutions; + var byViability = swaps.ToLookup(swap => IsStillViable(swap, kickOffStarters)); + return new PlannedSwaps([.. byViability[true]], [.. byViability[false]]); } private static List Moves( diff --git a/src/FootballFormation.Core/Services/MatchClockService.cs b/src/FootballFormation.Core/Services/MatchClockService.cs index cf743e7..da5f74f 100644 --- a/src/FootballFormation.Core/Services/MatchClockService.cs +++ b/src/FootballFormation.Core/Services/MatchClockService.cs @@ -1,5 +1,6 @@ using FootballFormation.Core.Data; using FootballFormation.Core.Models; +using FootballFormation.Core.Reporting; using FootballFormation.Core.Security; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; @@ -118,7 +119,8 @@ public Task> StartNextPeriodAsync(int gameId, CancellationToken can /// /// Rolls straight from the current period into the next one without stopping the clock, for the /// quarter boundaries that are not a real break (see ). - /// The lineup changes over, the running time does not. + /// The lineup changes over, the running time does not — minus the swaps play has already + /// answered, which drops. /// public Task> AdvancePeriodAsync(int gameId, CancellationToken cancellationToken = default) => LiveMatchOperation.RunAdminAsync(notifier, gameId, currentUser, logger, "start the next period", @@ -136,6 +138,14 @@ public Task> AdvancePeriodAsync(int gameId, CancellationToken cance if (next is null) return Result.Failure("Every period has been played — finish the match instead"); + await db.Entry(current).Collection(p => p.PlayerPositions).LoadAsync(cancellationToken); + await db.Entry(next).Collection(p => p.PlayerPositions).LoadAsync(cancellationToken); + var liveChanges = await db.GameSubstitutions + .Where(s => s.GamePeriodId == current.Id) + .ToListAsync(cancellationToken); + + KeepLiveArrivalsOn(current, next, liveChanges); + // Both ends read the same instant, so no seconds fall between the two periods. The // clock anchor is deliberately left alone: it must keep running through the change. var elapsed = game.ElapsedSecondsAt(UtcNow); @@ -157,6 +167,58 @@ public Task> AdvancePeriodAsync(int gameId, CancellationToken cance return Result.Success(game); }); + /// + /// Keeps the players brought on during the period that is ending on the pitch for the next one. + /// + /// The line-up for was written before the match. Where play has already + /// answered one of its swaps — the player it takes off went off live, and somebody came on for + /// them — carrying it out would pull that substitute straight back off for an arrival nobody is + /// waiting for, and an injury replacement would last exactly one quarter. So the swap is + /// dropped: the player who came on takes the place the plan's arrival was to have, and that + /// arrival goes to the bench. + /// + /// + /// Which swaps those are is 's answer, not a second opinion + /// formed here — the live screen lists exactly the ones it does not drop, directly above the + /// button that calls this, and a card promising one thing while the button does another is + /// worse than either behaviour on its own. + /// + /// + private static void KeepLiveArrivalsOn( + GamePeriod current, GamePeriod next, List liveChanges) + { + foreach (var swap in PlannedChangesReport.Swaps(current, next, liveChanges).Overtaken) + { + // Never null in this half of the split — see PlannedSwaps. + var stayingOn = swap.Off!; + + // The slot the plan's arrival was taking, or the one the substitute already holds when + // the next line-up names nobody for it. + var slot = swap.On?.SlotIndex ?? stayingOn.SlotIndex; + var position = swap.On?.Position ?? stayingOn.Position; + + foreach (var displaced in next.PlayerPositions + .Where(p => !p.IsSubstitute && p.SlotIndex == slot).ToList()) + { + displaced.SlotIndex = null; + displaced.IsSubstitute = true; + } + + var entry = next.PlayerPositions.FirstOrDefault(p => p.PlayerId == stayingOn.PlayerId); + if (entry is null) + { + // Someone who was not in the next line-up at all — a late arrival, or a bench that + // was only filled in for the first quarter. + entry = new GamePlayerPosition { GamePeriodId = next.Id, PlayerId = stayingOn.PlayerId }; + next.PlayerPositions.Add(entry); + } + + entry.SlotIndex = slot; + entry.Position = position; + entry.IsSubstitute = false; + } + } + public Task> FinishMatchAsync(int gameId, CancellationToken cancellationToken = default) => LiveMatchOperation.RunAdminAsync(notifier, gameId, currentUser, logger, "finish the match", cancellationToken, async () => diff --git a/src/FootballFormation.UI/Components/PlannedChangesList.razor b/src/FootballFormation.UI/Components/PlannedChangesList.razor index ea16f0d..717ab97 100644 --- a/src/FootballFormation.UI/Components/PlannedChangesList.razor +++ b/src/FootballFormation.UI/Components/PlannedChangesList.razor @@ -1,8 +1,8 @@ @* What the next line-up does, as a team sheet: who is swapped, and underneath, who only moves. - Two places show exactly this — the live screen's "Changes at half-way" card and the dialog that - asks before the same changes are carried out — so the markup and its styling live here once. + A component rather than markup on the page, because the .planned-* styling has to travel with + it: scoped CSS follows the file that owns the elements. *@ @inject IStringLocalizer L diff --git a/src/FootballFormation.UI/Helpers/DialogPrompts.cs b/src/FootballFormation.UI/Helpers/DialogPrompts.cs index 672c0ee..8022e27 100644 --- a/src/FootballFormation.UI/Helpers/DialogPrompts.cs +++ b/src/FootballFormation.UI/Helpers/DialogPrompts.cs @@ -58,23 +58,6 @@ public static async Task ConfirmAsync( return result is { Canceled: false, Data: TResult value } ? value : null; } - /// - /// for a dialog that returns a value type. Separate - /// because default(int) is a perfectly good answer, so "cancelled" has to be a null - /// rather than a default value nobody can tell apart from a real one. - /// - public static async Task PromptValueAsync( - this IDialogService dialogService, - string title, - Action>? configure = null) - where TDialog : ComponentBase - where TValue : struct - { - var result = await ShowAsync(dialogService, title, configure); - - return result is { Canceled: false, Data: TValue value } ? value : null; - } - private static async Task ShowAsync( IDialogService dialogService, string title, diff --git a/src/FootballFormation.UI/Pages/LiveMatch.razor b/src/FootballFormation.UI/Pages/LiveMatch.razor index bd69f09..4924488 100644 --- a/src/FootballFormation.UI/Pages/LiveMatch.razor +++ b/src/FootballFormation.UI/Pages/LiveMatch.razor @@ -59,26 +59,16 @@ else if (GameData.MatchState == MatchState.InProgress) {
- @if (IsLivePeriod) + @* A quarter boundary is not a stoppage but a planned line-up change, so it is + carried out from the card that lists what it changes rather than from here. *@ + @if (BreakFollowsCurrentPeriod) { - @* A quarter boundary is not a stoppage, it is the planned line-up change - partway through the half — so it is named after what it does. *@ - @if (BreakFollowsCurrentPeriod) - { - - @L["Half time"] - - } - else if (MidHalfSuccessor is not null) - { - - @L["Next line-up"] - - } + + @L["Half time"] + } - else if (NextHalfLabel is { } startingNext) + else if (!IsLivePeriod && NextHalfLabel is { } startingNext) { @@ -128,53 +118,64 @@ HidePositionFit="!_isAdmin" OnPlayerClicked="@(CanSubstitute ? EventCallback.Factory.Create(this, OpenSubDialog) : default)" /> - @* The bench is reference material, not something to act on, so it can be folded away - to keep the pitch and the controls on one phone screen. *@ -
- - @L["Show subs"] - -
- - @if (ShowSubs) - { -
- @L["Subs"]: - @if (OnBench.Count == 0) - { - @L["No subs"] - } - else +
+ @L["Subs"]: + @if (OnBench.Count == 0) + { + @L["No subs"] + } + else + { + @foreach (var sub in OnBench) { - @foreach (var sub in OnBench) - { - - @if (sub.Player?.ShirtNumber is not null) - { - #@sub.Player.ShirtNumber - } - @PlayerLabel(sub.PlayerId) - - } + + @if (sub.Player?.ShirtNumber is not null) + { + #@sub.Player.ShirtNumber + } + @PlayerLabel(sub.PlayerId) + } -
- } + } +
} @* The quarters the line-up was planned in are never named on this screen, but the difference between them is exactly the set of changes due halfway through the half — which is the part - worth knowing at the touchline. Measured against the pitch as it stands, so a swap already - made live disappears from the list. + worth knowing at the touchline. Measured against the pitch as it stands, and stripped of the + swaps play has already overtaken, so nothing here is a change the coach cannot still make. + The button that carries them out sits at the foot of the list rather than up with the clock + controls, so the tap is made while reading what it is about to do. Admin only, like the minutes table: this is what the coach is about to do, not what has happened, and a plan on a shared screen invites being asked about before it is carried out. *@ @{ var planned = PlannedChanges; } - @if (!planned.IsEmpty) + @* An empty list is still worth a card while the change can be made — otherwise the only + way on to the next line-up would vanish exactly when nobody needs swapping. *@ + @if (!planned.IsEmpty || CanAdvanceLineup) {
@L["Changes at half-way"]
- + + @if (planned.IsEmpty) + { + + @L["Nobody changes — the next line-up is the one already on the pitch."] + + } + else + { + + } + + @if (CanAdvanceLineup) + { + + @L["Next line-up"] + + } } @@ -200,10 +201,20 @@
@L["Timeline"]
+ @* Rotation fills this list with swaps nobody is scrolling back for, so they fold away and + leave the goals on their own. *@ +
+ + @L["Show substitutions"] + +
+ @if (Timeline.Count == 0) { + @* Two different emptinesses, and saying "nothing has happened" during a match with + three substitutions in it would be the checkbox's doing, not the truth. *@ - @L["Nothing has happened yet"] + @(HasEvents ? L["No goals yet"] : L["Nothing has happened yet"]) } else @@ -265,7 +276,9 @@ -
@L["Minutes played"]
+ @* Before kick-off these are the minutes the line-up plans for, not minutes anyone has + played — the heading says which, because the numbers themselves cannot. *@ +
@(MinutesAreActual ? L["Minutes played"] : L["Planned minutes"])
@if (MinutesPlayed.Count == 0) { diff --git a/src/FootballFormation.UI/Pages/LiveMatch.razor.cs b/src/FootballFormation.UI/Pages/LiveMatch.razor.cs index 3393f51..6ba6d68 100644 --- a/src/FootballFormation.UI/Pages/LiveMatch.razor.cs +++ b/src/FootballFormation.UI/Pages/LiveMatch.razor.cs @@ -55,11 +55,11 @@ public partial class LiveMatch private bool _isAdmin; /// - /// Whether the bench strip under the pitch is drawn. Per circuit and not stored: it is a - /// glance-vs-space choice made in the moment, and it survives the live reloads because those - /// replace the data rather than the component. + /// Whether the timeline lists substitutions alongside the goals. Per circuit and not stored: + /// it is a glance-vs-detail choice made in the moment, and it survives the live reloads + /// because those replace the data rather than the component. /// - private bool ShowSubs { get; set; } = true; + private bool ShowSubstitutions { get; set; } = true; /// /// Drives the clock display only. The elapsed value is derived from the anchor the server @@ -158,10 +158,17 @@ private GamePeriod? MidHalfSuccessor /// on the pitch right now — so a live substitution already made drops out of the list. /// private PlannedChanges PlannedChanges => - DisplayPeriod is { } current && MidHalfSuccessor is { } next - ? PlannedChangesReport.Build(current, next, FindPlayer) + GameData is { } game && DisplayPeriod is { } current && MidHalfSuccessor is { } next + ? PlannedChangesReport.Build(current, next, FindPlayer, + game.Substitutions.Where(s => s.GamePeriodId == current.Id)) : PlannedChanges.None; + /// + /// Whether the next line-up can be rolled on. Only during play: before kick-off the changes + /// are worth reading but there is no period running to advance out of. + /// + private bool CanAdvanceLineup => IsLivePeriod && MidHalfSuccessor is not null; + /// What the match is doing right now, in one phrase under the clock. private string StatusLabel => GameData?.MatchState switch { @@ -232,7 +239,21 @@ private List SubCandidates private List MinutesPlayed => GameData is null ? [] : LiveMinutesReport.Build(GameData, ElapsedSeconds, FindPlayer); - /// Goals and substitutions on one timeline, most recent first. + /// + /// Whether those minutes are time actually played. Until the first kick-off there is none, and + /// the figures are the planned line-up costed at a full period each — a different thing, which + /// is why the card is headed differently rather than claiming minutes nobody has played yet. + /// + private bool MinutesAreActual => GameData?.HasActualTimings == true; + + /// Whether anything at all has been recorded, filter or no filter. + private bool HasEvents => GameData is { } game && (game.Goals.Count > 0 || game.Substitutions.Count > 0); + + /// + /// Goals and substitutions on one timeline, most recent first. Substitutions can be left out: + /// a match with a lot of rotation buries the goals among them, and the goals are what someone + /// scrolling back is usually after. + /// private List Timeline { get @@ -245,7 +266,9 @@ private List Timeline var goals = GameData.Goals.Select(g => new MatchEvent(g.Minute ?? 0, g.RecordedAt, g.Id, true, g, null, progression[g.Id])); - var subs = GameData.Substitutions.Select(s => new MatchEvent(s.Minute, s.RecordedAt, s.Id, false, null, s)); + IEnumerable subs = ShowSubstitutions + ? GameData.Substitutions.Select(s => new MatchEvent(s.Minute, s.RecordedAt, s.Id, false, null, s)) + : []; // A goal and the sub that followed it commonly share a minute; the entry time keeps // them in the order they actually happened rather than the order they were queried. @@ -326,18 +349,11 @@ private async Task StartNextPeriod() => Snackbar.Report(L, await ClockService.StartNextPeriodAsync(GameId), L["Next period started"]); /// - /// Rolls the next planned line-up onto the pitch, after showing what that changes. Confirmed - /// rather than immediate because it is the one control here with no way back: advancing a - /// period rewrites who is on, and the timeline records no such event to undo. + /// Rolls the next planned line-up onto the pitch. Asks nothing first: the button sits under + /// the list of exactly the changes it makes, which is what a confirmation would have said. /// - private async Task AdvancePeriod() - { - var confirmed = await DialogService.PromptValueAsync( - L["Next line-up"], p => p.Add(x => x.Changes, PlannedChanges)); - if (confirmed is null) return; - + private async Task AdvancePeriod() => Snackbar.Report(L, await ClockService.AdvancePeriodAsync(GameId), L["Next period started"]); - } private async Task FinishMatch() { diff --git a/src/FootballFormation.UI/Pages/LiveMatch.razor.css b/src/FootballFormation.UI/Pages/LiveMatch.razor.css index 851db8b..86406cf 100644 --- a/src/FootballFormation.UI/Pages/LiveMatch.razor.css +++ b/src/FootballFormation.UI/Pages/LiveMatch.razor.css @@ -114,12 +114,7 @@ color: var(--ink-subtle); } -/* Bench strip under the pitch, and the checkbox that folds it away. The checkbox's own touch - target is sized in app.css — this rule reaches the wrapper, not the MudBlazor component in it. */ -.live-bench-toggle { - margin-top: 6px; -} - +/* Bench strip under the pitch */ .live-bench { display: flex; flex-wrap: wrap; @@ -164,7 +159,12 @@ gap: 8px; } -/* Timeline */ +/* Timeline, and the checkbox that folds the substitutions out of it. The checkbox's own touch + target is sized in app.css — this rule reaches the wrapper, not the MudBlazor component in it. */ +.live-timeline-toggle { + margin-bottom: 2px; +} + .live-timeline { display: flex; flex-direction: column; diff --git a/src/FootballFormation.UI/Pages/LiveNextLineupDialog.razor b/src/FootballFormation.UI/Pages/LiveNextLineupDialog.razor deleted file mode 100644 index cbf521b..0000000 --- a/src/FootballFormation.UI/Pages/LiveNextLineupDialog.razor +++ /dev/null @@ -1,28 +0,0 @@ -@inject IStringLocalizer L - - - - - @L["The line-up planned for the rest of this half takes over. The clock keeps running."] - - - @if (Changes.IsEmpty) - { - - @L["Nobody changes — the next line-up is the one already on the pitch."] - - } - else - { -
- -
- } - - - @L["Cancel"] - - @L["Next line-up"] - - - diff --git a/src/FootballFormation.UI/Pages/LiveNextLineupDialog.razor.cs b/src/FootballFormation.UI/Pages/LiveNextLineupDialog.razor.cs deleted file mode 100644 index bd07888..0000000 --- a/src/FootballFormation.UI/Pages/LiveNextLineupDialog.razor.cs +++ /dev/null @@ -1,24 +0,0 @@ -using FootballFormation.Core.Reporting; -using Microsoft.AspNetCore.Components; -using MudBlazor; - -namespace FootballFormation.UI.Pages; - -/// -/// Shows what rolling on to the next line-up will actually do, and asks first. The button used to -/// carry it out on the tap, which meant reading the card further down the screen to find out what -/// was about to change — and there is no undo for a period that has been advanced. -/// -public partial class LiveNextLineupDialog -{ - [CascadingParameter] - private IMudDialogInstance MudDialog { get; set; } = null!; - - /// The swaps and moves the next period's line-up implies, measured against the pitch. - [Parameter, EditorRequired] - public PlannedChanges Changes { get; set; } = PlannedChanges.None; - - private void Accept() => MudDialog.Close(DialogResult.Ok(true)); - - private void Cancel() => MudDialog.Cancel(); -} diff --git a/src/FootballFormation.UI/Pages/LiveNextLineupDialog.razor.css b/src/FootballFormation.UI/Pages/LiveNextLineupDialog.razor.css deleted file mode 100644 index c5dac38..0000000 --- a/src/FootballFormation.UI/Pages/LiveNextLineupDialog.razor.css +++ /dev/null @@ -1,5 +0,0 @@ -/* The list is a child component, so only its wrapper can be reached from here — which is all the - spacing this dialog needs. */ -.next-lineup-changes { - margin-top: 12px; -} diff --git a/src/FootballFormation.UI/Strings.nl.resx b/src/FootballFormation.UI/Strings.nl.resx index 2e48d39..6a2225b 100644 --- a/src/FootballFormation.UI/Strings.nl.resx +++ b/src/FootballFormation.UI/Strings.nl.resx @@ -194,7 +194,6 @@ {0} starten Volgende opstelling Wissels halverwege - De opstelling die voor de rest van deze helft gepland staat neemt het over. De klok loopt door. Er verandert niemand — de volgende opstelling is die al in het veld staat. Positiewissels Wedstrijd afsluiten @@ -205,8 +204,10 @@ Tegen doelpunt Tijdlijn Er is nog niets gebeurd - Wissels tonen + Wissels tonen + Nog geen doelpunten Gespeelde minuten + Opgestelde minuten In het veld Nu live diff --git a/src/FootballFormation.Web/wwwroot/app.css b/src/FootballFormation.Web/wwwroot/app.css index 3f31684..dde38ca 100644 --- a/src/FootballFormation.Web/wwwroot/app.css +++ b/src/FootballFormation.Web/wwwroot/app.css @@ -766,14 +766,22 @@ html, body { grid-column: 1 / -1; } -/* The "Show subs" checkbox is dense, which is right on a desktop and under the 44px floor on the - phone this screen is actually used on. Its label is the target, so the height goes there. */ +/* The timeline's "Show substitutions" checkbox is dense, which is right on a desktop and under the + 44px floor on the phone this screen is actually used on. Its label is the target, so the height + goes there. */ @media (pointer: coarse) { - .live-bench-toggle .mud-checkbox { + .live-timeline-toggle .mud-checkbox { min-height: 44px; } } +/* Carrying out the changes listed above it, so it is spaced off the list rather than crowding it. + A MudButton's root element is a child component's, which scoped CSS cannot reach. */ +.live-advance-btn { + margin-top: 14px; + min-height: 48px; +} + /* On a phone what just happened matters more than where everyone stands, so the line-up card drops below the timeline, taking the minutes table with it. Every other child of .live-layout keeps the default order of 0 and so stays in source order above them. */ diff --git a/tests/FootballFormation.Core.Tests/MatchClockServiceTests.cs b/tests/FootballFormation.Core.Tests/MatchClockServiceTests.cs index cbda286..70e49f1 100644 --- a/tests/FootballFormation.Core.Tests/MatchClockServiceTests.cs +++ b/tests/FootballFormation.Core.Tests/MatchClockServiceTests.cs @@ -1,4 +1,5 @@ using FootballFormation.Core.Models; +using Microsoft.EntityFrameworkCore; namespace FootballFormation.Core.Tests; @@ -180,6 +181,85 @@ public async Task Advancing_restarts_a_clock_that_an_older_build_left_stopped() Assert.Equal(1200, advanced.ElapsedSecondsAt(Time.GetUtcNow().UtcDateTime)); } + /// + /// The plan for the next quarter was written before the match. If it still takes off a player + /// who has already gone off, carrying it out pulls their replacement straight back off — so an + /// injury replacement would last exactly one quarter. The live screen drops that swap from + /// "Changes at half-way"; this is the half that makes the button agree with the card. + /// + [Fact] + public async Task Advancing_keeps_a_player_brought_on_live_rather_than_carrying_out_the_swap_they_answered() + { + var game = await SeedQuartersWithASwapAsync(); + var players = await PlayersAsync(); + + await MatchClock.StartMatchAsync(game.Id); + Time.Advance(TimeSpan.FromMinutes(5)); + // Not P3, who Q2 was going to bring on — an injury, and whoever was warm goes on. + Assert.True((await Subs.SubstituteAsync(game.Id, players[1].Id, players[3].Id)).IsSuccess); + + Time.Advance(TimeSpan.FromMinutes(10)); + Assert.True((await MatchClock.AdvancePeriodAsync(game.Id)).IsSuccess); + + var q2 = await LineupAsync(game.Id, PeriodType.SecondQuarter); + var stayedOn = Assert.Single(q2, p => p.PlayerId == players[3].Id); + Assert.False(stayedOn.IsSubstitute); + Assert.Equal(5, stayedOn.SlotIndex); + Assert.Equal(PlayerPosition.CM, stayedOn.Position); + + // And the arrival the plan named is on the bench rather than in the same slot. + Assert.True(q2.Single(p => p.PlayerId == players[2].Id).IsSubstitute); + Assert.Single(q2, p => p.SlotIndex == 5); + } + + [Fact] + public async Task Advancing_carries_out_a_swap_the_match_has_not_already_answered() + { + var game = await SeedQuartersWithASwapAsync(); + var players = await PlayersAsync(); + + await MatchClock.StartMatchAsync(game.Id); + Time.Advance(TimeSpan.FromMinutes(15)); + Assert.True((await MatchClock.AdvancePeriodAsync(game.Id)).IsSuccess); + + // Nothing overtook it, so the planned line-up rolls on untouched. + var q2 = await LineupAsync(game.Id, PeriodType.SecondQuarter); + Assert.Equal(5, q2.Single(p => p.PlayerId == players[2].Id).SlotIndex); + Assert.True(q2.Single(p => p.PlayerId == players[1].Id).IsSubstitute); + } + + /// + /// A quarters game whose second quarter plans one swap: P2 comes off at CM for P3. Every + /// period is seeded with the same line-up, so the second one is rewritten here. + /// + private async Task SeedQuartersWithASwapAsync() + { + var game = await SeedGameAsync(GameSplitType.Quarters); + var players = await PlayersAsync(); + + var q2 = game.Periods.Single(p => p.PeriodType == PeriodType.SecondQuarter); + await Db.Entry(q2).Collection(p => p.PlayerPositions).LoadAsync(); + + var comingOff = q2.PlayerPositions.Single(p => p.PlayerId == players[1].Id); + var comingOn = q2.PlayerPositions.Single(p => p.PlayerId == players[2].Id); + + (comingOff.SlotIndex, comingOff.IsSubstitute) = (null, true); + (comingOn.SlotIndex, comingOn.IsSubstitute) = (5, false); + comingOn.Position = PlayerPosition.CM; + + await Db.SaveChangesAsync(); + return game; + } + + private async Task> LineupAsync(int gameId, PeriodType period) + { + Db.ChangeTracker.Clear(); + + return await Db.GamePlayerPositions + .Where(p => p.GamePeriod.GameId == gameId && p.GamePeriod.PeriodType == period) + .ToListAsync(); + } + [Fact] public async Task Advancing_past_the_last_period_is_refused() { diff --git a/tests/FootballFormation.Core.Tests/PlannedChangesReportTests.cs b/tests/FootballFormation.Core.Tests/PlannedChangesReportTests.cs index d7668db..137a6e5 100644 --- a/tests/FootballFormation.Core.Tests/PlannedChangesReportTests.cs +++ b/tests/FootballFormation.Core.Tests/PlannedChangesReportTests.cs @@ -25,7 +25,7 @@ public void A_player_swapped_for_another_is_one_substitution() TestData.Starter(3, PlayerPosition.CM, 5), TestData.Sub(2)); - var changes = PlannedChangesReport.Build(q1, q2, Find); + var changes = PlannedChangesReport.Build(q1, q2, Find, []); var swap = Assert.Single(changes.Substitutions); Assert.Equal(2, swap.PlayerOff!.Id); @@ -51,7 +51,7 @@ public void A_reshuffle_around_one_swap_reports_one_substitution_and_the_moves_s TestData.Starter(1, PlayerPosition.CB, 3), TestData.Starter(4, PlayerPosition.RB, 4)); - var changes = PlannedChangesReport.Build(q3, q4, Find); + var changes = PlannedChangesReport.Build(q3, q4, Find, []); var swap = Assert.Single(changes.Substitutions); Assert.Equal(2, swap.PlayerOff!.Id); @@ -83,7 +83,7 @@ public void An_arrival_is_paired_with_whoever_held_the_slot_they_take() TestData.Starter(3, PlayerPosition.CB, 3), TestData.Starter(4, PlayerPosition.ST, 10)); - var changes = PlannedChangesReport.Build(q1, q2, Find); + var changes = PlannedChangesReport.Build(q1, q2, Find, []); Assert.Collection(changes.Substitutions, cb => Assert.Equal((1, 3), (cb.PlayerOff!.Id, cb.PlayerOn!.Id)), @@ -103,7 +103,7 @@ public void An_unchanged_lineup_produces_nothing() // Who sits on the bench is not a change to the pitch. TestData.Sub(3)); - Assert.True(PlannedChangesReport.Build(q1, q2, Find).IsEmpty); + Assert.True(PlannedChangesReport.Build(q1, q2, Find, []).IsEmpty); } [Fact] @@ -116,7 +116,7 @@ public void Line_ups_that_do_not_balance_still_name_everyone_they_concern() var q2 = game.AddPeriod(PeriodType.SecondQuarter, TestData.Starter(3, PlayerPosition.LB, 2)); - var changes = PlannedChangesReport.Build(q1, q2, Find); + var changes = PlannedChangesReport.Build(q1, q2, Find, []); Assert.Collection(changes.Substitutions, swap => Assert.Equal((2, 3), (swap.PlayerOff!.Id, swap.PlayerOn!.Id)), @@ -138,10 +138,94 @@ public void Someone_coming_off_the_bench_counts_as_arriving() TestData.Starter(2, PlayerPosition.CM, 5), TestData.Sub(1)); - var swap = Assert.Single(PlannedChangesReport.Build(q1, q2, Find).Substitutions); + var swap = Assert.Single(PlannedChangesReport.Build(q1, q2, Find, []).Substitutions); Assert.Equal((1, 2), (swap.PlayerOff!.Id, swap.PlayerOn!.Id)); } + /// + /// Play overtakes the plan. The line-up still differs from the next one, so the difference + /// still names the slot — but it now proposes to withdraw the player who came on for the one + /// the plan meant to take off, which is a substitution nobody planned. + /// + [Fact] + public void A_swap_whose_outgoing_player_has_already_been_taken_off_drops_out() + { + var game = TestData.Game(split: GameSplitType.Quarters); + // As the pitch stands after 2 went off for 4 — the lineup records where everyone is now. + var q1 = game.AddPeriod(PeriodType.FirstQuarter, + TestData.Starter(1, PlayerPosition.GK, 0), + TestData.Starter(4, PlayerPosition.CM, 5), + TestData.Sub(2)); + var q2 = game.AddPeriod(PeriodType.SecondQuarter, + TestData.Starter(1, PlayerPosition.GK, 0), + TestData.Starter(3, PlayerPosition.CM, 5)); + var live = TestData.Substitution(game, q1, offId: 2, onId: 4, atSeconds: 300, PlayerPosition.CM, slot: 5); + + // Without the substitution the difference reads as a swap of the player who just came on. + var swap = Assert.Single(PlannedChangesReport.Build(q1, q2, Find, []).Substitutions); + Assert.Equal((4, 3), (swap.PlayerOff!.Id, swap.PlayerOn!.Id)); + + Assert.Empty(PlannedChangesReport.Build(q1, q2, Find, [live]).Substitutions); + } + + [Fact] + public void A_swap_is_kept_when_its_outgoing_player_is_still_on_the_pitch() + { + var game = TestData.Game(split: GameSplitType.Quarters); + var q1 = game.AddPeriod(PeriodType.FirstQuarter, + TestData.Starter(1, PlayerPosition.GK, 0), + TestData.Starter(2, PlayerPosition.CM, 5), + TestData.Starter(4, PlayerPosition.ST, 10), + TestData.Sub(6)); + var q2 = game.AddPeriod(PeriodType.SecondQuarter, + TestData.Starter(1, PlayerPosition.GK, 0), + TestData.Starter(3, PlayerPosition.CM, 5), + TestData.Starter(4, PlayerPosition.ST, 10)); + var live = TestData.Substitution(game, q1, offId: 6, onId: 4, atSeconds: 300, PlayerPosition.ST, slot: 10); + + var swap = Assert.Single(PlannedChangesReport.Build(q1, q2, Find, [live]).Substitutions); + Assert.Equal((2, 3), (swap.PlayerOff!.Id, swap.PlayerOn!.Id)); + } + + /// + /// The rewind has to unwind the substitutions newest first. Taken the other way round, a player + /// who left and returned reads as somebody who was never in the starting line-up, and the swap + /// the plan still holds for them disappears. + /// + [Fact] + public void A_player_who_went_off_and_came_back_is_still_the_one_the_plan_takes_off() + { + var game = TestData.Game(split: GameSplitType.Quarters); + var q1 = game.AddPeriod(PeriodType.FirstQuarter, + TestData.Starter(1, PlayerPosition.GK, 0), + TestData.Starter(2, PlayerPosition.CM, 5), + TestData.Sub(3)); + var q2 = game.AddPeriod(PeriodType.SecondQuarter, + TestData.Starter(1, PlayerPosition.GK, 0), + TestData.Starter(4, PlayerPosition.CM, 5)); + var off = TestData.Substitution(game, q1, offId: 2, onId: 3, atSeconds: 300, PlayerPosition.CM, slot: 5); + var back = TestData.Substitution(game, q1, offId: 3, onId: 2, atSeconds: 600, PlayerPosition.CM, slot: 5); + + var swap = Assert.Single(PlannedChangesReport.Build(q1, q2, Find, [off, back]).Substitutions); + Assert.Equal((2, 4), (swap.PlayerOff!.Id, swap.PlayerOn!.Id)); + } + + /// An arrival with nobody named to come off is a line-up worth flagging, not hiding. + [Fact] + public void An_arrival_with_nobody_to_come_off_survives_the_viability_check() + { + var game = TestData.Game(split: GameSplitType.Quarters); + var q1 = game.AddPeriod(PeriodType.FirstQuarter, + TestData.Starter(1, PlayerPosition.GK, 0)); + var q2 = game.AddPeriod(PeriodType.SecondQuarter, + TestData.Starter(1, PlayerPosition.GK, 0), + TestData.Starter(2, PlayerPosition.CM, 5)); + + var swap = Assert.Single(PlannedChangesReport.Build(q1, q2, Find, []).Substitutions); + Assert.Null(swap.PlayerOff); + Assert.Equal(2, swap.PlayerOn!.Id); + } + [Theory] [InlineData(PeriodType.FirstHalf, PeriodType.FirstHalf)] [InlineData(PeriodType.FirstQuarter, PeriodType.FirstHalf)] diff --git a/tests/ui/specs/match-day.spec.js b/tests/ui/specs/match-day.spec.js index 30a3406..8027b21 100644 --- a/tests/ui/specs/match-day.spec.js +++ b/tests/ui/specs/match-day.spec.js @@ -146,7 +146,7 @@ test('tapping a player on the pitch offers a substitution and a position swap', await expect(page.locator('.live-event')).toHaveCount(0); }); -test('"Next line-up" says what it is about to change, and cancelling changes nothing', async ({ page }) => { +test('the next line-up is rolled on from the card that lists what it changes', async ({ page }) => { // Quarters, so the first half is planned as two line-ups and the mid-half control appears. const id = await matchWithId(page, 'FC Kwarten', { split: 'Quarters' }); @@ -173,35 +173,33 @@ test('"Next line-up" says what it is about to change, and cancelling changes not ); await goto(page, `/games/${id}/live`); - const nextLineup = page.locator('.live-controls').getByRole('button', { name: 'Next line-up' }); + const nextLineup = page.getByRole('button', { name: 'Next line-up' }); + + // The changes are worth reading before kick-off, but there is no period running to advance out + // of yet, so the button that carries them out is not there. + await expect(page.locator('.planned-row').first()).toBeVisible(); + await expect(nextLineup).toHaveCount(0); + await clickFor( page.getByRole('button', { name: 'Start match' }), () => expect(nextLineup).toBeVisible(), ); - await clickFor(nextLineup, () => expect(page.locator('.mud-dialog')).toBeVisible()); - const dialog = await openDialog(page); - - // The point of the dialog: the changes are in it, not on a card further down the screen. - await expect(dialog.locator('.planned-row').first()).toBeVisible(); - + // It belongs to the card, not to the clock controls: the tap is made while reading the list it + // sits under, which is why it no longer asks in a dialog first. + await expect(page.locator('.live-controls').getByRole('button', { name: 'Next line-up' })).toHaveCount(0); await clickFor( - dialog.getByRole('button', { name: 'Cancel' }), - () => expect(page.locator('.mud-dialog')).toHaveCount(0), + nextLineup, + () => expect(page.getByText('Next period started', { exact: false })).toBeVisible(), ); - // Still the first quarter — cancelling a change with no undo has to actually cancel it. - await expect(nextLineup).toBeVisible(); - - await clickFor(nextLineup, () => expect(page.locator('.mud-dialog')).toBeVisible()); - await submitDialog(page, 'Next line-up'); - await expect(page.getByText('Next period started', { exact: false })).toBeVisible(); + await expect(page.locator('.mud-dialog')).toHaveCount(0); // The second quarter is the last of the half, so the control it now offers is half time. await expect(page.locator('.live-controls').getByRole('button', { name: 'Half time' })).toBeVisible(); }); -test('the bench strip can be folded away while the match is being run', async ({ page }) => { - const id = await matchWithId(page, 'FC Bank'); +test('the timeline can be narrowed to the goals', async ({ page }) => { + const id = await matchWithId(page, 'FC Tijdlijn'); await fillLineup(page, 2); await goto(page, `/games/${id}/live`); @@ -210,11 +208,29 @@ test('the bench strip can be folded away while the match is being run', async ({ () => expect(page.getByRole('button', { name: 'Finish match' })).toBeVisible(), ); - await expect(page.locator('.live-bench')).toBeVisible(); + const events = page.locator('.live-event'); + + // One of each kind, so the filter has something to keep and something to drop. + await clickFor(page.getByRole('button', { name: 'Goal against' }), () => expect(events).toHaveCount(1)); + await clickFor( - page.locator('.live-bench-toggle input[type=checkbox]'), - () => expect(page.locator('.live-bench')).toHaveCount(0), + page.locator('.live-lineup .pitch-player').first(), + () => expect(page.locator('.mud-dialog')).toBeVisible(), + ); + const dialog = await openDialog(page); + await chooseOption(page, dialog, 'Comes on', '#'); + await submitDialog(page, 'Make substitution'); + await expect(events).toHaveCount(2); + + // Only a goal carries a scoreline, so what is left is the goal rather than the substitution. + await clickFor( + page.locator('.live-timeline-toggle input[type=checkbox]'), + () => expect(events).toHaveCount(1), ); + await expect(page.locator('.live-event .live-event-score')).toHaveCount(1); + + // The bench is no longer what this checkbox folds away — it stays put. + await expect(page.locator('.live-bench')).toBeVisible(); }); test('the playing-time table drops its estimate once the match has been run', async ({ page }) => { @@ -227,11 +243,18 @@ test('the playing-time table drops its estimate once the match has been run', as await expect(totals.first()).toContainText('~'); await expect(page.locator('.playtime-note')).toBeVisible(); + // The live screen's own table says the same thing in its heading, because its numbers cannot: + // before kick-off they are what the line-up plans for, not time anyone has played. await goto(page, `/games/${id}/live`); + const minutesLabel = page.locator('.live-minutes-card .card-label'); + await expect(minutesLabel).toHaveText('Planned minutes'); + await clickFor( page.getByRole('button', { name: 'Start match' }), () => expect(page.getByRole('button', { name: 'Finish match' })).toBeVisible(), ); + await expect(minutesLabel).toHaveText('Minutes played'); + await clickFor( page.locator('.live-controls').getByRole('button', { name: 'Finish match' }), () => expect(page.locator('.mud-dialog')).toBeVisible(), diff --git a/tests/ui/specs/selectors.spec.js b/tests/ui/specs/selectors.spec.js index d7c9155..7afecc2 100644 --- a/tests/ui/specs/selectors.spec.js +++ b/tests/ui/specs/selectors.spec.js @@ -31,8 +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-bench-toggle', - 'planned-row'], + 'live-event', 'live-event-score', 'live-bench', 'live-timeline-toggle', + 'live-minutes-card', 'card-label', 'planned-row'], 'the phone layout': ['dialog-sheet', 'stacked-table', 'topbar-nav'], 'the squad': ['badge-archived'], };