diff --git a/docs/backlog/issues.md b/docs/backlog/issues.md index 97681892..2ff21152 100644 --- a/docs/backlog/issues.md +++ b/docs/backlog/issues.md @@ -1,6 +1,78 @@ # Last ID used -029 +032 + +## ISSUE-032 - Recurring season creates duplicate next-run on each cache refresh before new run starts + +Status: DONE +Priority: CRITICAL +Area: Seasons / Recurring + +### Problem +After a recurring season ends and a new run is cloned (but not yet started — its `start_time` is in the future), `SeasonService` keeps creating additional clones on every subsequent `RefreshCache()` call, producing duplicate season rows. + +### Root Cause (Confirmed) +`GetPendingRecurringSeason()` lacked a filter on `end_time`. Its query: +```sql +WHERE is_active = 0 AND is_recurring = 1 AND start_time <= GETUTCDATE() +``` +matches the already-completed previous season (S1) because S1 has `is_active = 0`, `is_recurring = 1`, and `start_time` in the past — even though it has already ended. The future clone (S2) is excluded by the `start_time <= now` predicate until its own start time arrives. + +This caused `RefreshCache()` to re-activate S1 every 5 minutes → S1 ends immediately → `ProcessSeasonEnd(S1)` runs again → another clone is created → indefinitely. + +### Fix +1. **Primary** — Added `AND end_time > GETUTCDATE()` to `GetPendingRecurringSeason()`. Ended seasons are no longer candidates, so only truly future pending runs are returned. +2. **Defense-in-depth** — Added `HasFutureClone(Season)` repository method that checks for any existing future inactive clone in the same recurring chain. `ProcessSeasonEnd` now guards `CloneSeasonForNextIteration` with this check, preventing a second clone even if the method fires twice. + +### Files Changed +- `src/Perpetuum/Services/Seasons/SeasonRepository.cs` — fixed query in `GetPendingRecurringSeason()`; added `HasFutureClone()` +- `src/Perpetuum/Services/Seasons/SeasonService.cs` — guarded clone call in `ProcessSeasonEnd` + +### Notes +- Any orphan clone rows already accumulated in the DB (`start_time` in the future, `is_active = 0`) are harmless and will be correctly activated when their `start_time` arrives. Duplicates with the same `start_time` should be deleted manually. + +--- + +## ISSUE-031 - Season leaderboard rewards not delivered automatically or via admin command + +Status: DONE +Priority: CRITICAL +Area: Seasons / Rewards / Leaderboard + +### Problem +Participants of a season are not receiving leaderboard rewards, neither automatically when the season ends nor when an admin manually triggers the reward delivery command. + +### Related Error +The following exception fires in `SeasonService.Update` on every tick, which may block the reward delivery path: + +``` +System.InvalidCastException: Unable to cast object of type 'System.Byte' to type 'System.Int32'. + at Perpetuum.Data.DataRecordExtensions.GetValue[T](IDataRecord record, Int32 index) + at Perpetuum.Data.DataRecordExtensions.GetValue[T](IDataRecord record, String name) + at Perpetuum.Services.Seasons.SeasonRepository.GetPendingRecurringSeason() + at Perpetuum.Services.Seasons.SeasonService.RefreshCache() + at Perpetuum.Services.Seasons.SeasonService.Update(TimeSpan time) +``` + +A column returned by the `GetPendingRecurringSeason` query is typed as `tinyint` (or similar `BYTE`-width type) in the DB but is read as `int` in C#. The exception throws every update tick, causing `RefreshCache()` to abort. This may be preventing the service from ever seeing the active season — and therefore from running leaderboard reward delivery. + +### Impact +- Leaderboard rewards are silently not delivered to top season participants. +- `SeasonService.RefreshCache()` crashes on every update tick due to the type mismatch. +- Admin re-deliver command has no effect if the service cannot load season state. +- Players expect rewards after season end; silent failure erodes trust. + +### Proposed Fix +1. **Fix the type mismatch** — identify which column in the `GetPendingRecurringSeason` result set is a `tinyint`/`smallint`/`byte` in SQL but is read as `int` in C#. Change the C# read to use the correct numeric type, or `CAST` the column to `int` in the query. +2. **Verify reward delivery path** — once `RefreshCache()` no longer throws, confirm that leaderboard reward delivery runs for the ended season. If not, trace the delivery trigger separately. +3. **Investigate admin command** — check whether the admin re-deliver command (`SeasonRedeliverLeaderboardRewards`, if implemented per ISSUE-025) also depends on `RefreshCache()` or uses a separate repository path that may have its own bug. + +### Notes +- Stack trace points to `SeasonRepository.cs:511` inside `GetPendingRecurringSeason()`. +- Cross-reference ISSUE-025 (leaderboard rewards not delivered — root cause was swapped `rank_min`/`rank_max`). Verify those DB rows are correctly set before concluding the reward path itself is broken. +- The exception is non-fatal to the process but fires on every tick — investigate whether it swallows the exception or propagates to the caller and aborts the season update loop. + +--- ## ISSUE-029 - Insurance price recalculation crashes with SP nesting level exceeded (limit 32) @@ -127,6 +199,37 @@ Operators cannot meaningfully browse or audit market orders. The broken type and --- +## ISSUE-030 - SeasonService ignores season start time, activating seasons before they should begin + +Status: DONE +Priority: CRITICAL +Area: Seasons + +### Problem +`SeasonService` does not enforce `start_time` anywhere. A season marked `is_active = 1` with a future `start_time` is immediately treated as live: `GetActiveSeason()` queries only `WHERE is_active = 1` with no `start_time <= GETUTCDATE()` guard, and `RefreshCache()`, `RecordActivity()`, and `OnCharacterLogin()` all check only `EndTime` — `StartTime` is never compared against `DateTime.UtcNow` at runtime. + +### Impact +- Activity points accumulate before the season is intended to start. +- Players receive intro mails and leaderboard announcements prematurely. +- Recurring season clones (whose `start_time` is set to a future date) go live immediately after the previous season ends instead of waiting for their scheduled start. + +### Proposed Fix +Two-layer enforcement: + +1. **DB layer** — add `AND start_time <= GETUTCDATE()` to the `GetActiveSeason()` query in `SeasonRepository` so a future-dated active season is invisible to the service until its start time arrives. +2. **Service layer** — in `RefreshCache()`, after loading the season, assert `DateTime.UtcNow >= season.StartTime`; if not, treat as no active season (clear cache, do not notify). Guard `RecordActivity()` and `OnCharacterLogin()` with the same check so the in-memory `_activeSeason` cannot process activity before start even if the cache is stale. + +The DB guard is the primary fix. The service-layer check is a defence-in-depth backstop. + +### Notes +- `SeasonService.cs` line 114: `_repository.GetActiveSeason()` — fix in repository query. +- `SeasonRepository.cs` lines 11-15: the `WHERE is_active = 1` query needs the `start_time` predicate. +- `RecordActivity()` line 188: only guards `EndTime`; add `DateTime.UtcNow < season.StartTime` early return. +- `OnCharacterLogin()` line 273: same pattern. +- The recurring season clone path (`CloneSeasonForNextIteration`) already sets a future `start_time`, so the DB fix automatically gates the clone. + +--- + ## ISSUE-025 - Top leaderboard participants did not receive rewards after Active Season ended Status: IN_PROGRESS diff --git a/src/Perpetuum/Services/Seasons/SeasonRepository.cs b/src/Perpetuum/Services/Seasons/SeasonRepository.cs index 6d4644b8..466f18f5 100644 --- a/src/Perpetuum/Services/Seasons/SeasonRepository.cs +++ b/src/Perpetuum/Services/Seasons/SeasonRepository.cs @@ -12,7 +12,7 @@ public class SeasonRepository "SELECT id, name, description, start_time, end_time, is_active, " + "is_recurring, recurrence_gap_days, recurrence_iteration, recurrence_base_name, scoring_mode, " + "daily_objectives_per_day " + - "FROM seasons WHERE is_active = 1") + "FROM seasons WHERE is_active = 1 AND start_time <= GETUTCDATE()") .ExecuteSingleRow(); if (record == null) return null; @@ -490,7 +490,7 @@ public void AddLeaderboardReward(int seasonId, int rankMin, int rankMax, int pac RecurrenceGapDays = record.GetValue("recurrence_gap_days"), RecurrenceIteration = record.GetValue("recurrence_iteration"), RecurrenceBaseName = record.GetValue("recurrence_base_name"), - ScoringMode = (SeasonScoringMode)record.GetValue("scoring_mode"), + ScoringMode = (SeasonScoringMode)record.GetValue("scoring_mode"), DailyObjectivesPerDay = (int?)record.GetValue("daily_objectives_per_day"), }; } @@ -502,7 +502,8 @@ public void AddLeaderboardReward(int seasonId, int rankMin, int rankMax, int pac "is_recurring, recurrence_gap_days, recurrence_iteration, recurrence_base_name, scoring_mode, " + "daily_objectives_per_day " + "FROM seasons " + - "WHERE is_active = 0 AND is_recurring = 1 AND start_time <= GETUTCDATE() " + + "WHERE is_active = 0 AND is_recurring = 1 " + + "AND start_time <= GETUTCDATE() AND end_time > GETUTCDATE() " + "ORDER BY start_time ASC") .ExecuteSingleRow(); @@ -520,11 +521,28 @@ public void AddLeaderboardReward(int seasonId, int rankMin, int rankMax, int pac RecurrenceGapDays = record.GetValue("recurrence_gap_days"), RecurrenceIteration = record.GetValue("recurrence_iteration"), RecurrenceBaseName = record.GetValue("recurrence_base_name"), - ScoringMode = (SeasonScoringMode)record.GetValue("scoring_mode"), + ScoringMode = (SeasonScoringMode)record.GetValue("scoring_mode"), DailyObjectivesPerDay = (int?)record.GetValue("daily_objectives_per_day"), }; } + /// + /// Returns true if any future (not yet started) inactive clone already exists for this recurring season chain. + /// Used to prevent duplicate clones when ProcessSeasonEnd fires more than once. + /// + public bool HasFutureClone(Season season) + { + string baseName = season.RecurrenceBaseName ?? season.Name; + var count = Db.Query( + "SELECT COUNT(1) FROM seasons " + + "WHERE is_recurring = 1 AND is_active = 0 " + + "AND start_time > GETUTCDATE() " + + "AND recurrence_base_name = @baseName") + .SetParameter("@baseName", baseName) + .ExecuteScalar(); + return count > 0; + } + public Season CloneSeasonForNextIteration(Season previous) { if (previous.RecurrenceGapDays == null) diff --git a/src/Perpetuum/Services/Seasons/SeasonService.cs b/src/Perpetuum/Services/Seasons/SeasonService.cs index ffa5ef9f..8fa2c547 100644 --- a/src/Perpetuum/Services/Seasons/SeasonService.cs +++ b/src/Perpetuum/Services/Seasons/SeasonService.cs @@ -113,6 +113,11 @@ internal void RefreshCache() var previous = _activeSeason; var season = _repository.GetActiveSeason(); + // Defence-in-depth: reject any season whose start_time has not yet arrived + // (DB guard already handles this; this backstop covers clock skew / in-flight state). + if (season != null && DateTime.UtcNow < season.StartTime) + season = null; + if (season == null) { // If admin deactivated before natural end, trigger end processing now @@ -185,7 +190,7 @@ internal void RefreshCache() public void RecordActivity(int characterId, SeasonActivityType activityType, ActivityEvent evt) { var season = _activeSeason; - if (season == null || DateTime.UtcNow > season.EndTime) + if (season == null || DateTime.UtcNow < season.StartTime || DateTime.UtcNow > season.EndTime) return; var rates = _activeRates.Where(r => r.ActivityType == activityType).ToList(); @@ -270,7 +275,7 @@ public void OnCharacterLogin(Character character) _pendingIntroChars.Enqueue(character); return; } - if (DateTime.UtcNow > season.EndTime) + if (DateTime.UtcNow < season.StartTime || DateTime.UtcNow > season.EndTime) return; if (_repository.TryMarkIntroMailSent(character.Id, season.Id)) SendIntroMail(character, season); @@ -397,11 +402,19 @@ public int RedeliverLeaderboardRewards(int seasonId) var entry = rankings[rank - 1]; if (entry.LeaderboardRewardDelivered) continue; - var reward = leaderboard.FirstOrDefault(r => rank >= r.RankMin && rank <= r.RankMax); - if (reward != null && DeliverLeaderboardReward(entry.CharacterId, reward)) + try + { + var reward = leaderboard.FirstOrDefault(r => rank >= r.RankMin && rank <= r.RankMax); + if (reward != null && DeliverLeaderboardReward(entry.CharacterId, reward)) + { + delivered++; + _repository.MarkLeaderboardDelivered(entry.CharacterId, seasonId); + } + } + catch (Exception ex) { - delivered++; - _repository.MarkLeaderboardDelivered(entry.CharacterId, seasonId); + System.Diagnostics.Trace.TraceError( + $"[SeasonService] Failed to redeliver leaderboard reward for character {entry.CharacterId} (rank {rank}): {ex}"); } } return delivered; @@ -414,7 +427,7 @@ private void ProcessSeasonEnd(Season season) var seasonChannel = _channelManager.Value.GetChannelByName(SeasonChannelName); if (seasonChannel != null) { - seasonChannel.SetTopic("No active seasons"); + _channelManager.Value.SetTopic(SeasonChannelName, _announcer.Value, "No active seasons"); } // Null the cache immediately so no further activity is recorded @@ -433,12 +446,21 @@ private void ProcessSeasonEnd(Season season) if (entry.LeaderboardRewardDelivered) continue; - var reward = leaderboard.FirstOrDefault(r => rank >= r.RankMin && rank <= r.RankMax); - bool rewardDelivered = reward != null && DeliverLeaderboardReward(entry.CharacterId, reward); + try + { + var reward = leaderboard.FirstOrDefault(r => rank >= r.RankMin && rank <= r.RankMax); + bool rewardDelivered = reward != null && DeliverLeaderboardReward(entry.CharacterId, reward); - _repository.MarkLeaderboardDelivered(entry.CharacterId, season.Id); - SendFinalStandingsMail(entry.CharacterId, rank, entry.TotalPoints, - rewardDelivered, season.Name); + if (reward == null || rewardDelivered) + _repository.MarkLeaderboardDelivered(entry.CharacterId, season.Id); + SendFinalStandingsMail(entry.CharacterId, rank, entry.TotalPoints, + rewardDelivered, season.Name); + } + catch (Exception ex) + { + System.Diagnostics.Trace.TraceError( + $"[SeasonService] Failed to deliver leaderboard reward for character {entry.CharacterId} (rank {rank}): {ex}"); + } } _activeRates = ImmutableList.Empty; @@ -468,7 +490,7 @@ private void ProcessSeasonEnd(Season season) _channelManager.Value.Announcement(SeasonChannelName, _announcer.Value, chatMessage.ToString()); - if (season.IsRecurring) + if (season.IsRecurring && !_repository.HasFutureClone(season)) _repository.CloneSeasonForNextIteration(season); } @@ -635,7 +657,7 @@ private void NotifyOnlinePlayersSeasonStarted(Season season) var seasonChannel = _channelManager.Value.GetChannelByName(SeasonChannelName); if (seasonChannel != null) { - seasonChannel.SetTopic($"Season {season.Name}: {season.StartTime} - {season.EndTime}"); + _channelManager.Value.SetTopic(SeasonChannelName, _announcer.Value, $"Season {season.Name}: {season.StartTime} - {season.EndTime}"); } foreach (var character in _sessionManager.SelectedCharacters) @@ -669,7 +691,7 @@ private void NotifyOnlinePlayersSeasonStarted(Season season) foreach (var obj in _activeObjectives.OrderBy(o => o.DisplayOrder)) { chatMessage.AppendLine($" {obj.Name}: {obj.Description} (Bonus: {obj.BonusPoints} pts)"); - chatMessage.AppendLine($" Progress by performing {ActivityTypeName(obj.ActivityType)}. Target: {obj.TargetValue:N0}"); + //chatMessage.AppendLine($" Progress by performing {ActivityTypeName(obj.ActivityType)}. Target: {obj.TargetValue:N0}"); } chatMessage.AppendLine();