Conversation
reiserFSs
left a comment
There was a problem hiding this comment.
Reviewed c956332. Changes requested before merge.
-
Hydrate previously claimed weekly tasks into login FinishedTasks. WeeklyTwoActiveness counts MissionProgress.ClaimedTaskIds, but AccountModule.cs:1318 sends only session.stage.FinishedTasks and EN XTaskManager.GetWeeklyTaskActiveness counts the client's FinishedTasks map. Real FinishTask handlers claimed30039-30042, BSON relog produced NotifyTaskData.FinishedTasks=[], then task30043 gave visible count1 versus server count5; direct weekly RPC returnedCode0/progress[5], while the normal UI cannot issue it. Include persisted claimed weekly tasks in the login projection and add a claim/relog/further-claim regression. Relevant new code TaskModule.cs:492-496.
-
Exclude previous-week GuildWeekly completions after rollover. WeeklyTaggedTaskIds includes Type23 GuildWeekly tasks, but EnsureMissionResets clears only CurrentTask Type3 claims. A valid persisted prior-week task3001728 survived reset; four current-week completions then incorrectly granted the five-task milestone. Reset all refreshable weekly-tag claims consistently, preserving unrelated lifetime tasks, and cover prior-week guild claims plus current-week threshold boundaries. Relevant TaskModule.cs:149-151 and weekly reset implementation.
Verification note: Release build and --weekly-activeness-compat-only pass, but external real-handler/BSON relog and rollover probes reproduced these cases; full harness stops at the known pre-existing Pain Cage rollover assertion. No native UI execution claimed.
|
Two review findings, reproduced by external probes, are fixed here. 1. Persisted weekly claims missing from the login projection
2. Prior-week GuildWeekly (Type 23) claims survived rollover
Differential verification
|
reiserFSs
left a comment
There was a problem hiding this comment.
Changes requested: three verified weekly lifecycle defects remain.
-
P1 — Weekly rollover reuses lifetime guild reward receipts (
AscNet.GameServer/Handlers/TaskModule.cs:2448–2452). Three real guild-stage first uploads reach condition 35010=3 and allow task 3001728 to grant rewards. After persisting a stale week and running the real login routine, the claimed-task marker is cleared, but progress andguild-achievement:3001728receipts remain. A second FinishTaskRequest returns Code 0 and two reward entries with zero inventory delta, then counts the task as finished in the new week. Scope Type-23 progress and claim receipts to the weekly period without incorrectly resetting the shared Type-6 progress. -
P2 — One receipt blocks recovery of a partial reward save (
TaskModule.cs:505–507). Injecting character-save failure after inventory persistence returns 20026003 and leaves an inventory-only receipt. After durable BSON reload, retry returns 20026012 with no pushes and never repairs the missing character receipt. Only a fully completed grant should exclude the milestone; allow the existing receipt-aware grant helper to finish partial saves without duplicate rewards. -
P2 — Login materializes stale FinishedTasks before reset (
AscNet.GameServer/Handlers/AccountModule.cs:832–839). With old-week historical task 35011 and no guild membership, the actual login routine sends that ID in NotifyTaskData.FinishedTasks even though persisted state has removed it. Reconcile the weekly period before materializing any login task projection.
Verification: registered handlers, three real guild-stage uploads, actual login routine against isolated MongoDB, and durable BSON plus injected-save-failure checks. Existing weekly/login selectors pass but do not defend these boundaries.
Support
GetWeeklyActivenessRewardRequestProblem
The game server logged an unhandled request on login/task-panel use:
The client calls this with an empty body
{}when the player claims Weekly (WeeklyTwo) activeness rewards. No handler was registered, so the reward was never granted.What the request is
XTaskManager:GetWeeklyActivenessRewardRequest(XTaskManager.lua:782).Code,RewardGoodsList,WeeklyTaskActivenessProgress.WeeklyTaskActivenessProgressis the full set of activeness milestone values already claimed this week (the client replaces its local cache with this list).CurrentTaskActivenessTable.Type == 5(CurrentTaskActiveness.tsv): milestones[5, 10], rewards[68860, 68870].WeeklyinTask.tab(XTaskManager.GetWeeklyTaskActiveness,XTaskManager.lua:1700). Server-side equivalent is the count of(ClaimedTaskIds ∪ stage.FinishedTasks)whoseTaskTable.Tag == 1.68860/68870resolve through the normalReward.tab(688600/688700→ item106"Makeup Sign-in Card"), notCurrentReward.tab, so this path usesRewardHandler.GetRewardGoods.Implementation
GetWeeklyActivenessRewardResponse { Code, RewardGoodsList, WeeklyTaskActivenessProgress }.[RequestPacketHandler("GetWeeklyActivenessRewardRequest")]; the body is empty and ignored.RewardHandler.ApplyRewardsOnceAndPersistwith a week-scoped claim keyweekly-two:{week}:{milestone}stored inAppliedRewardClaims(mirrorscurrent-task:{taskId}:{week}). No new persisted DB field.WeeklyTaskActivenessProgressis derived by scanning the current week's claim keys.MissionProgress.WeeklyResetWeek.NotifyTaskData.TaskData.WeeklyTaskActivenessProgressat login so claimed checkmarks survive relog (the client already readsdata.TaskData.WeeklyTaskActivenessProgress,XTaskManager.lua:370).20026010(not reached / config or reward missing),20026012(already claimed),20026003(persist failure). Success is0.Files changed
AscNet.GameServer/Handlers/TaskModule.csWeeklyTaggedTaskIds,GetWeeklyActivenessRewardRequesthandler,ClaimWeeklyTwoRewards/BuildWeeklyTwoProgress/ claim-key helpersAscNet.Common/MsgPack/Types.csWeeklyTaskActivenessProgressonNotifyTaskDataTaskDataAscNet.GameServer/Handlers/AccountModule.csWeeklyTaskActivenessProgressat loginAscNet.Test/Program.csValidateWeeklyActivenessRewardCompatibility(wired intoValidateLoginAccountCompatibility) +--weekly-activeness-compat-onlyswitchVerification
Builds clean (0 warnings / 0 errors) for
AscNet.Common,AscNet.GameServer,AscNet.Test.Focused harness (
dotnet run --project AscNet.Test/AscNet.Test.csproj -- --weekly-activeness-compat-only, exit 0):Code = 20026010, no rewardsCode = 0, reward688600, progress[5]Code = 0, reward688700, progress[5, 10]Code = 20026012, no rewards--login-account-compat-onlypasses (exit 0), confirming the login payload addition does not regress login shape or startup push order.Known unrelated failure
The full default harness stops at a pre-existing failure in
ValidateBossSingleCompatibility:Reproduced with the login field temporarily removed, so it is unrelated to this change (and aborts the harness before later validations run).
AGENTS.MD compliance
CurrentTaskActiveness.tsv,Reward.tsv,Task.tsv); claim state comes from persistedAppliedRewardClaims. No captured payloads, retail IDs, balances, or timestamps are embedded.Type == 5andTag == 1are table semantics (same style as the existingx.Type == 3/4checks), and the error codes reuse existing server protocol values.GetWeeklyActivenessRewardResponseuses the named-key[MessagePackObject(true)]pattern, co-located with the sibling task DTOs per the module's existing convention; the shared/persisted login schema change lives inAscNet.Common/MsgPack/Types.csas AGENTS prescribes.[RequestPacketHandler("GetWeeklyActivenessRewardRequest")], response sent with the request'spacket.Id. The request body is empty ({}), so no request DTO is deserialized — consistent withGetNewbieRewardRequestHandler/GetNewbieHonorRewardRequestHandler.RewardHandler.ApplyRewardsOnceAndPersist, which saves inventory/character with rollback on failure.--weekly-activeness-compat-onlyfirst, then the full harness; derived fields covered with distinct states (4 / 5 / 10). The one full-harness failure is pre-existing and unrelated (see above)..runtime/,bin/,obj/, or runtime logs added.Notes / follow-ups
current-task:keys.InvalidDataException/error handling and reward rollback are delegated toRewardHandler.ApplyRewardsOnceAndPersist.