feat(session): add pre-migration session save - #575

Merged
AngeloTadeucci merged 3 commits into
masterfrom
feat-session-save
Sep 28, 2025
Merged

feat(session): add pre-migration session save#575
AngeloTadeucci merged 3 commits into
masterfrom
feat-session-save

Conversation

@AngeloTadeucci

@AngeloTadeucciAngeloTadeucci commented Sep 28, 2025

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features
    • Added a 5-minute grace period before closing empty dungeon lobbies, preventing abrupt shutdowns while players transition.
  • Bug Fixes
    • Improved reliability during migrations (channel changes, global portals, dungeon transfers, and quit-to-menu) by saving player state prior to transfer, reducing risk of progress loss.
  • Refactor
    • Streamlined internal save flow during migrations and session cleanup for greater consistency.
  • Style
    • Enhanced logging around dungeon lobby creation and disposal timing for clearer operational visibility.

@coderabbitai

coderabbitaiBot commented Sep 28, 2025

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

Walkthrough

Adds a pre-migration persistence step via GameSession.MigrationSave() across multiple migration paths, switches EnterServer locking to account-based, introduces a dungeon lobby empty-grace disposal flow with logging, and defines a new DungeonDisposeEmptyTime constant. Minor formatting/logging tweaks and non-functional collection initializer change included.

Changes

Cohort / File(s)Summary
Session persistence and locking
Maple2.Server.Game/Session/GameSession.cs
Adds preMigrationSaved flag and public MigrationSave() that saves player and component states; invokes MigrationSave() in migration and dispose paths; changes EnterServer locking from CharacterId to AccountId (AcquireLock/ReleaseLock).
Early migration save integration
Maple2.Server.Game/Commands/WarpCommand.cs, Maple2.Server.Game/Manager/DungeonManager.cs, Maple2.Server.Game/PacketHandlers/ChannelHandler.cs, Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs, Maple2.Server.Game/PacketHandlers/QuitHandler.cs
Inserts session.MigrationSave() before initiating various migration flows (warp, dungeon migrate, channel change, global portal, quit non-exit). DungeonManager also formats a collection initializer.
Dungeon disposal timing/logging
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs, Maple2.Model/Metadata/Constants.cs
Replaces immediate dungeon lobby disposal with grace-timer flow that totals players across lobby and sub-rooms, logs state, and disposes after elapsed grace; adds DungeonDisposeEmptyTime = TimeSpan.FromMinutes(5).

Sequence Diagram(s)

sequenceDiagram
autonumber
participant C as Client
participant H as Handler/Manager
participant S as GameSession
participant DB as Persistence
participant P as Planner/Login
C->>H: Action triggers migration (warp/channel/portal/quit)
H->>S: MigrationSave()
alt first-call
S->>DB: Save Player and components (UgcMarket, Config, Shop, Item, Survival, Housing, GameEvent, Achievement, Quest, Dungeon)
DB-->>S: Save results (per-component guarded)
else already-saved
S-->>H: No-op (idempotent)
end
H->>P: MigrateOut request
P-->>H: Migration ticket/ack
H->>C: Instruct disconnect/reconnect (migration)
Loading
sequenceDiagram
autonumber
participant FM as FieldManager
participant DL as Dungeon Lobby Field
participant SR as Sub-Rooms (all)
participant T as Timer
loop DisposeLoop tick
FM->>DL: Check players
FM->>SR: Sum players across rooms
alt Any players > 0
FM->>DL: Reset fieldEmptySince
note right of DL: Skip disposal this tick
else None and no timer set
DL->>T: Start grace timer (DungeonDisposeEmptyTime)
else None and timer elapsed
FM->>DL: DisposeDungeon()
end
end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • Zintixx
  • mettaursp

Poem

I thump my paw—save first, then hop,
Pack carrots, state, and make a stop.
Through portals wide, I glide with grace,
A timed-out dungeon leaves no trace.
Locks on accounts, I twitch and grin—
Migrate, persist… then burrow in! 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 6.67% which is insufficient. The required threshold is 80.00%.You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title Check✅ PassedThe title “feat(session): add pre-migration session save” accurately reflects the primary change in the pull request, which is the introduction of a MigrationSave method in GameSession and its invocation before migration flows. It is concise, clear, and highlights the feature context (session) without extraneous details or noise. This phrasing gives a teammate scanning the repository history a direct understanding that the changeset implements a new pre-migration save capability.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7de323d and d25aa11.

📒 Files selected for processing (1)
  • Maple2.Server.Game/Session/GameSession.cs (6 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1)

26-50: Reset migration persistence when the channel switch fails

When the channel swap RPC throws, we return an error to the client and keep the session alive. Because MigrationSave() ran just before the try/catch, preMigrationSaved remains stuck at true, so every later migration (and even the final dispose) skips saving. Any progress the player makes after the failed channel change is now lost. Please undo or reset the guard in this failure path (e.g., call a ResetMigrationSave() helper or only mark the save as finalized once we know we’re disconnecting).

🧹 Nitpick comments (1)
Maple2.Database/Storage/Game/GameStorage.User.cs (1)

283-407: Remove the dead commented SavePlayer rework

This 120+ line commented block makes the method noisy, obscures the true code path, and risks drifting from reality. If you plan to revisit this logic, keep it in a ticket or design note; otherwise, drop it so the file stays readable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8143d28 and 7de323d.

📒 Files selected for processing (9)
  • Maple2.Database/Storage/Game/GameStorage.User.cs (1 hunks)
  • Maple2.Model/Metadata/Constants.cs (1 hunks)
  • Maple2.Server.Game/Commands/WarpCommand.cs (1 hunks)
  • Maple2.Server.Game/Manager/DungeonManager.cs (2 hunks)
  • Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (2 hunks)
  • Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1 hunks)
  • Maple2.Server.Game/Session/GameSession.cs (6 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Session/GameSession.cs (16)
Maple2.Server.Login/Session/LoginSession.cs (2)
  • AcquireLock (55-73)
  • ReleaseLock (75-86)
Maple2.Database/Storage/Game/DatabaseRequest.cs (3)
  • Commit (20-28)
  • BeginTransaction (16-18)
  • SaveChanges (30-32)
Maple2.Database/Storage/Game/GameStorage.User.cs (4)
  • GameStorage (22-661)
  • Request (23-659)
  • SavePlayer (280-445)
  • Player (211-278)
Maple2.Database/Storage/Game/GameStorage.cs (5)
  • GameStorage (11-80)
  • GameStorage (22-40)
  • Request (42-49)
  • Request (51-79)
  • Request (53-55)
Maple2.Server.Game/Manager/DungeonManager.cs (1)
  • Save (617-619)
Maple2.Server.Game/Manager/AchievementManager.cs (1)
  • Save (302-305)
Maple2.Server.Game/Manager/Items/InventoryManager.cs (2)
  • Save (729-736)
  • Item (569-577)
Maple2.Server.Game/Manager/Items/EquipManager.cs (4)
  • Save (361-368)
  • Item (58-62)
  • Item (64-68)
  • Item (70-72)
Maple2.Server.Game/Manager/HousingManager.cs (1)
  • Save (342-345)
Maple2.Server.Game/Manager/QuestManager.cs (1)
  • Save (669-672)
Maple2.Server.Game/Manager/Items/ItemManager.cs (3)
  • Save (94-98)
  • Item (38-41)
  • Item (48-51)
Maple2.Server.Game/Manager/ShopManager.cs (3)
  • Save (628-651)
  • Shop (130-176)
  • Shop (178-197)
Maple2.Server.Game/Manager/GameEventManager.cs (1)
  • Save (162-171)
Maple2.Server.Game/Manager/ConfigManager.cs (1)
  • Save (572-590)
Maple2.Server.Game/Manager/SurvivalManager.cs (1)
  • Save (173-176)
Maple2.Server.Game/Manager/UgcMarketManager.cs (1)
  • Save (152-155)
Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Manager/DungeonManager.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (1)
Maple2.Model/Metadata/Constants.cs (1)
  • Constant (10-961)
Maple2.Server.Game/Commands/WarpCommand.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build
  • GitHub Check: format
  • GitHub Check: build
🔇 Additional comments (4)
Maple2.Model/Metadata/Constants.cs (1)

154-155: LGTM – consistent disposal constant

Mirrors the existing field timing constants and exposes the new dungeon window cleanly. Also keeps the value localized for any future tuning.

Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (2)

198-199: Appreciate the added dungeon init instrumentation.
The extra lobby context in the debug line will make dungeon spin-up investigations much easier.


286-310: Grace-based disposal flow looks solid.
Aggregating lobby + sub-room player counts before starting the empty timer—and using the new DungeonDisposeEmptyTime window—should prevent premature tears when parties are split across rooms.

Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1)

37-37: Pre-migration save placement looks solid.

Calling MigrationSave() immediately before the migrate-out request mirrors the other migration paths and keeps the flow idempotent thanks to the internal guard. Looks good.

Comment threadMaple2.Server.Game/Session/GameSession.cs
@AngeloTadeucci
AngeloTadeucci merged commit 17beeb1 into masterSep 28, 2025
3 checks passed
@AngeloTadeucci
AngeloTadeucci deleted the feat-session-save branch October 13, 2025 00:38
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@AngeloTadeucci@Zintixx
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

feat(session): add pre-migration session save - #575

Merged
AngeloTadeucci merged 3 commits into
masterfrom
feat-session-save
Sep 28, 2025
Merged

feat(session): add pre-migration session save#575
AngeloTadeucci merged 3 commits into
masterfrom
feat-session-save

Conversation

@AngeloTadeucci

@AngeloTadeucciAngeloTadeucci commented Sep 28, 2025

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features
    • Added a 5-minute grace period before closing empty dungeon lobbies, preventing abrupt shutdowns while players transition.
  • Bug Fixes
    • Improved reliability during migrations (channel changes, global portals, dungeon transfers, and quit-to-menu) by saving player state prior to transfer, reducing risk of progress loss.
  • Refactor
    • Streamlined internal save flow during migrations and session cleanup for greater consistency.
  • Style
    • Enhanced logging around dungeon lobby creation and disposal timing for clearer operational visibility.

@coderabbitai

coderabbitaiBot commented Sep 28, 2025

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

Walkthrough

Adds a pre-migration persistence step via GameSession.MigrationSave() across multiple migration paths, switches EnterServer locking to account-based, introduces a dungeon lobby empty-grace disposal flow with logging, and defines a new DungeonDisposeEmptyTime constant. Minor formatting/logging tweaks and non-functional collection initializer change included.

Changes

Cohort / File(s)Summary
Session persistence and locking
Maple2.Server.Game/Session/GameSession.cs
Adds preMigrationSaved flag and public MigrationSave() that saves player and component states; invokes MigrationSave() in migration and dispose paths; changes EnterServer locking from CharacterId to AccountId (AcquireLock/ReleaseLock).
Early migration save integration
Maple2.Server.Game/Commands/WarpCommand.cs, Maple2.Server.Game/Manager/DungeonManager.cs, Maple2.Server.Game/PacketHandlers/ChannelHandler.cs, Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs, Maple2.Server.Game/PacketHandlers/QuitHandler.cs
Inserts session.MigrationSave() before initiating various migration flows (warp, dungeon migrate, channel change, global portal, quit non-exit). DungeonManager also formats a collection initializer.
Dungeon disposal timing/logging
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs, Maple2.Model/Metadata/Constants.cs
Replaces immediate dungeon lobby disposal with grace-timer flow that totals players across lobby and sub-rooms, logs state, and disposes after elapsed grace; adds DungeonDisposeEmptyTime = TimeSpan.FromMinutes(5).

Sequence Diagram(s)

sequenceDiagram
autonumber
participant C as Client
participant H as Handler/Manager
participant S as GameSession
participant DB as Persistence
participant P as Planner/Login
C->>H: Action triggers migration (warp/channel/portal/quit)
H->>S: MigrationSave()
alt first-call
S->>DB: Save Player and components (UgcMarket, Config, Shop, Item, Survival, Housing, GameEvent, Achievement, Quest, Dungeon)
DB-->>S: Save results (per-component guarded)
else already-saved
S-->>H: No-op (idempotent)
end
H->>P: MigrateOut request
P-->>H: Migration ticket/ack
H->>C: Instruct disconnect/reconnect (migration)
Loading
sequenceDiagram
autonumber
participant FM as FieldManager
participant DL as Dungeon Lobby Field
participant SR as Sub-Rooms (all)
participant T as Timer
loop DisposeLoop tick
FM->>DL: Check players
FM->>SR: Sum players across rooms
alt Any players > 0
FM->>DL: Reset fieldEmptySince
note right of DL: Skip disposal this tick
else None and no timer set
DL->>T: Start grace timer (DungeonDisposeEmptyTime)
else None and timer elapsed
FM->>DL: DisposeDungeon()
end
end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • Zintixx
  • mettaursp

Poem

I thump my paw—save first, then hop,
Pack carrots, state, and make a stop.
Through portals wide, I glide with grace,
A timed-out dungeon leaves no trace.
Locks on accounts, I twitch and grin—
Migrate, persist… then burrow in! 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 6.67% which is insufficient. The required threshold is 80.00%.You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title Check✅ PassedThe title “feat(session): add pre-migration session save” accurately reflects the primary change in the pull request, which is the introduction of a MigrationSave method in GameSession and its invocation before migration flows. It is concise, clear, and highlights the feature context (session) without extraneous details or noise. This phrasing gives a teammate scanning the repository history a direct understanding that the changeset implements a new pre-migration save capability.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7de323d and d25aa11.

📒 Files selected for processing (1)
  • Maple2.Server.Game/Session/GameSession.cs (6 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1)

26-50: Reset migration persistence when the channel switch fails

When the channel swap RPC throws, we return an error to the client and keep the session alive. Because MigrationSave() ran just before the try/catch, preMigrationSaved remains stuck at true, so every later migration (and even the final dispose) skips saving. Any progress the player makes after the failed channel change is now lost. Please undo or reset the guard in this failure path (e.g., call a ResetMigrationSave() helper or only mark the save as finalized once we know we’re disconnecting).

🧹 Nitpick comments (1)
Maple2.Database/Storage/Game/GameStorage.User.cs (1)

283-407: Remove the dead commented SavePlayer rework

This 120+ line commented block makes the method noisy, obscures the true code path, and risks drifting from reality. If you plan to revisit this logic, keep it in a ticket or design note; otherwise, drop it so the file stays readable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8143d28 and 7de323d.

📒 Files selected for processing (9)
  • Maple2.Database/Storage/Game/GameStorage.User.cs (1 hunks)
  • Maple2.Model/Metadata/Constants.cs (1 hunks)
  • Maple2.Server.Game/Commands/WarpCommand.cs (1 hunks)
  • Maple2.Server.Game/Manager/DungeonManager.cs (2 hunks)
  • Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (2 hunks)
  • Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1 hunks)
  • Maple2.Server.Game/Session/GameSession.cs (6 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Session/GameSession.cs (16)
Maple2.Server.Login/Session/LoginSession.cs (2)
  • AcquireLock (55-73)
  • ReleaseLock (75-86)
Maple2.Database/Storage/Game/DatabaseRequest.cs (3)
  • Commit (20-28)
  • BeginTransaction (16-18)
  • SaveChanges (30-32)
Maple2.Database/Storage/Game/GameStorage.User.cs (4)
  • GameStorage (22-661)
  • Request (23-659)
  • SavePlayer (280-445)
  • Player (211-278)
Maple2.Database/Storage/Game/GameStorage.cs (5)
  • GameStorage (11-80)
  • GameStorage (22-40)
  • Request (42-49)
  • Request (51-79)
  • Request (53-55)
Maple2.Server.Game/Manager/DungeonManager.cs (1)
  • Save (617-619)
Maple2.Server.Game/Manager/AchievementManager.cs (1)
  • Save (302-305)
Maple2.Server.Game/Manager/Items/InventoryManager.cs (2)
  • Save (729-736)
  • Item (569-577)
Maple2.Server.Game/Manager/Items/EquipManager.cs (4)
  • Save (361-368)
  • Item (58-62)
  • Item (64-68)
  • Item (70-72)
Maple2.Server.Game/Manager/HousingManager.cs (1)
  • Save (342-345)
Maple2.Server.Game/Manager/QuestManager.cs (1)
  • Save (669-672)
Maple2.Server.Game/Manager/Items/ItemManager.cs (3)
  • Save (94-98)
  • Item (38-41)
  • Item (48-51)
Maple2.Server.Game/Manager/ShopManager.cs (3)
  • Save (628-651)
  • Shop (130-176)
  • Shop (178-197)
Maple2.Server.Game/Manager/GameEventManager.cs (1)
  • Save (162-171)
Maple2.Server.Game/Manager/ConfigManager.cs (1)
  • Save (572-590)
Maple2.Server.Game/Manager/SurvivalManager.cs (1)
  • Save (173-176)
Maple2.Server.Game/Manager/UgcMarketManager.cs (1)
  • Save (152-155)
Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Manager/DungeonManager.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (1)
Maple2.Model/Metadata/Constants.cs (1)
  • Constant (10-961)
Maple2.Server.Game/Commands/WarpCommand.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build
  • GitHub Check: format
  • GitHub Check: build
🔇 Additional comments (4)
Maple2.Model/Metadata/Constants.cs (1)

154-155: LGTM – consistent disposal constant

Mirrors the existing field timing constants and exposes the new dungeon window cleanly. Also keeps the value localized for any future tuning.

Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (2)

198-199: Appreciate the added dungeon init instrumentation.
The extra lobby context in the debug line will make dungeon spin-up investigations much easier.


286-310: Grace-based disposal flow looks solid.
Aggregating lobby + sub-room player counts before starting the empty timer—and using the new DungeonDisposeEmptyTime window—should prevent premature tears when parties are split across rooms.

Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1)

37-37: Pre-migration save placement looks solid.

Calling MigrationSave() immediately before the migrate-out request mirrors the other migration paths and keeps the flow idempotent thanks to the internal guard. Looks good.

Comment threadMaple2.Server.Game/Session/GameSession.cs
@AngeloTadeucci
AngeloTadeucci merged commit 17beeb1 into masterSep 28, 2025
3 checks passed
@AngeloTadeucci
AngeloTadeucci deleted the feat-session-save branch October 13, 2025 00:38
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@AngeloTadeucci@Zintixx
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(session): add pre-migration session save - #575

Merged
AngeloTadeucci merged 3 commits into
masterfrom
feat-session-save
Sep 28, 2025
Merged

feat(session): add pre-migration session save#575
AngeloTadeucci merged 3 commits into
masterfrom
feat-session-save

Conversation

@AngeloTadeucci

@AngeloTadeucciAngeloTadeucci commented Sep 28, 2025

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features
    • Added a 5-minute grace period before closing empty dungeon lobbies, preventing abrupt shutdowns while players transition.
  • Bug Fixes
    • Improved reliability during migrations (channel changes, global portals, dungeon transfers, and quit-to-menu) by saving player state prior to transfer, reducing risk of progress loss.
  • Refactor
    • Streamlined internal save flow during migrations and session cleanup for greater consistency.
  • Style
    • Enhanced logging around dungeon lobby creation and disposal timing for clearer operational visibility.

@coderabbitai

coderabbitaiBot commented Sep 28, 2025

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

Walkthrough

Adds a pre-migration persistence step via GameSession.MigrationSave() across multiple migration paths, switches EnterServer locking to account-based, introduces a dungeon lobby empty-grace disposal flow with logging, and defines a new DungeonDisposeEmptyTime constant. Minor formatting/logging tweaks and non-functional collection initializer change included.

Changes

Cohort / File(s)Summary
Session persistence and locking
Maple2.Server.Game/Session/GameSession.cs
Adds preMigrationSaved flag and public MigrationSave() that saves player and component states; invokes MigrationSave() in migration and dispose paths; changes EnterServer locking from CharacterId to AccountId (AcquireLock/ReleaseLock).
Early migration save integration
Maple2.Server.Game/Commands/WarpCommand.cs, Maple2.Server.Game/Manager/DungeonManager.cs, Maple2.Server.Game/PacketHandlers/ChannelHandler.cs, Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs, Maple2.Server.Game/PacketHandlers/QuitHandler.cs
Inserts session.MigrationSave() before initiating various migration flows (warp, dungeon migrate, channel change, global portal, quit non-exit). DungeonManager also formats a collection initializer.
Dungeon disposal timing/logging
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs, Maple2.Model/Metadata/Constants.cs
Replaces immediate dungeon lobby disposal with grace-timer flow that totals players across lobby and sub-rooms, logs state, and disposes after elapsed grace; adds DungeonDisposeEmptyTime = TimeSpan.FromMinutes(5).

Sequence Diagram(s)

sequenceDiagram
autonumber
participant C as Client
participant H as Handler/Manager
participant S as GameSession
participant DB as Persistence
participant P as Planner/Login
C->>H: Action triggers migration (warp/channel/portal/quit)
H->>S: MigrationSave()
alt first-call
S->>DB: Save Player and components (UgcMarket, Config, Shop, Item, Survival, Housing, GameEvent, Achievement, Quest, Dungeon)
DB-->>S: Save results (per-component guarded)
else already-saved
S-->>H: No-op (idempotent)
end
H->>P: MigrateOut request
P-->>H: Migration ticket/ack
H->>C: Instruct disconnect/reconnect (migration)
Loading
sequenceDiagram
autonumber
participant FM as FieldManager
participant DL as Dungeon Lobby Field
participant SR as Sub-Rooms (all)
participant T as Timer
loop DisposeLoop tick
FM->>DL: Check players
FM->>SR: Sum players across rooms
alt Any players > 0
FM->>DL: Reset fieldEmptySince
note right of DL: Skip disposal this tick
else None and no timer set
DL->>T: Start grace timer (DungeonDisposeEmptyTime)
else None and timer elapsed
FM->>DL: DisposeDungeon()
end
end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • Zintixx
  • mettaursp

Poem

I thump my paw—save first, then hop,
Pack carrots, state, and make a stop.
Through portals wide, I glide with grace,
A timed-out dungeon leaves no trace.
Locks on accounts, I twitch and grin—
Migrate, persist… then burrow in! 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 6.67% which is insufficient. The required threshold is 80.00%.You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title Check✅ PassedThe title “feat(session): add pre-migration session save” accurately reflects the primary change in the pull request, which is the introduction of a MigrationSave method in GameSession and its invocation before migration flows. It is concise, clear, and highlights the feature context (session) without extraneous details or noise. This phrasing gives a teammate scanning the repository history a direct understanding that the changeset implements a new pre-migration save capability.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7de323d and d25aa11.

📒 Files selected for processing (1)
  • Maple2.Server.Game/Session/GameSession.cs (6 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1)

26-50: Reset migration persistence when the channel switch fails

When the channel swap RPC throws, we return an error to the client and keep the session alive. Because MigrationSave() ran just before the try/catch, preMigrationSaved remains stuck at true, so every later migration (and even the final dispose) skips saving. Any progress the player makes after the failed channel change is now lost. Please undo or reset the guard in this failure path (e.g., call a ResetMigrationSave() helper or only mark the save as finalized once we know we’re disconnecting).

🧹 Nitpick comments (1)
Maple2.Database/Storage/Game/GameStorage.User.cs (1)

283-407: Remove the dead commented SavePlayer rework

This 120+ line commented block makes the method noisy, obscures the true code path, and risks drifting from reality. If you plan to revisit this logic, keep it in a ticket or design note; otherwise, drop it so the file stays readable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8143d28 and 7de323d.

📒 Files selected for processing (9)
  • Maple2.Database/Storage/Game/GameStorage.User.cs (1 hunks)
  • Maple2.Model/Metadata/Constants.cs (1 hunks)
  • Maple2.Server.Game/Commands/WarpCommand.cs (1 hunks)
  • Maple2.Server.Game/Manager/DungeonManager.cs (2 hunks)
  • Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (2 hunks)
  • Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1 hunks)
  • Maple2.Server.Game/Session/GameSession.cs (6 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Session/GameSession.cs (16)
Maple2.Server.Login/Session/LoginSession.cs (2)
  • AcquireLock (55-73)
  • ReleaseLock (75-86)
Maple2.Database/Storage/Game/DatabaseRequest.cs (3)
  • Commit (20-28)
  • BeginTransaction (16-18)
  • SaveChanges (30-32)
Maple2.Database/Storage/Game/GameStorage.User.cs (4)
  • GameStorage (22-661)
  • Request (23-659)
  • SavePlayer (280-445)
  • Player (211-278)
Maple2.Database/Storage/Game/GameStorage.cs (5)
  • GameStorage (11-80)
  • GameStorage (22-40)
  • Request (42-49)
  • Request (51-79)
  • Request (53-55)
Maple2.Server.Game/Manager/DungeonManager.cs (1)
  • Save (617-619)
Maple2.Server.Game/Manager/AchievementManager.cs (1)
  • Save (302-305)
Maple2.Server.Game/Manager/Items/InventoryManager.cs (2)
  • Save (729-736)
  • Item (569-577)
Maple2.Server.Game/Manager/Items/EquipManager.cs (4)
  • Save (361-368)
  • Item (58-62)
  • Item (64-68)
  • Item (70-72)
Maple2.Server.Game/Manager/HousingManager.cs (1)
  • Save (342-345)
Maple2.Server.Game/Manager/QuestManager.cs (1)
  • Save (669-672)
Maple2.Server.Game/Manager/Items/ItemManager.cs (3)
  • Save (94-98)
  • Item (38-41)
  • Item (48-51)
Maple2.Server.Game/Manager/ShopManager.cs (3)
  • Save (628-651)
  • Shop (130-176)
  • Shop (178-197)
Maple2.Server.Game/Manager/GameEventManager.cs (1)
  • Save (162-171)
Maple2.Server.Game/Manager/ConfigManager.cs (1)
  • Save (572-590)
Maple2.Server.Game/Manager/SurvivalManager.cs (1)
  • Save (173-176)
Maple2.Server.Game/Manager/UgcMarketManager.cs (1)
  • Save (152-155)
Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Manager/DungeonManager.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (1)
Maple2.Model/Metadata/Constants.cs (1)
  • Constant (10-961)
Maple2.Server.Game/Commands/WarpCommand.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build
  • GitHub Check: format
  • GitHub Check: build
🔇 Additional comments (4)
Maple2.Model/Metadata/Constants.cs (1)

154-155: LGTM – consistent disposal constant

Mirrors the existing field timing constants and exposes the new dungeon window cleanly. Also keeps the value localized for any future tuning.

Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (2)

198-199: Appreciate the added dungeon init instrumentation.
The extra lobby context in the debug line will make dungeon spin-up investigations much easier.


286-310: Grace-based disposal flow looks solid.
Aggregating lobby + sub-room player counts before starting the empty timer—and using the new DungeonDisposeEmptyTime window—should prevent premature tears when parties are split across rooms.

Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1)

37-37: Pre-migration save placement looks solid.

Calling MigrationSave() immediately before the migrate-out request mirrors the other migration paths and keeps the flow idempotent thanks to the internal guard. Looks good.

Comment threadMaple2.Server.Game/Session/GameSession.cs
@AngeloTadeucci
AngeloTadeucci merged commit 17beeb1 into masterSep 28, 2025
3 checks passed
@AngeloTadeucci
AngeloTadeucci deleted the feat-session-save branch October 13, 2025 00:38
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@AngeloTadeucci@Zintixx
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(session): add pre-migration session save - #575

Merged
AngeloTadeucci merged 3 commits into
masterfrom
feat-session-save
Sep 28, 2025
Merged

feat(session): add pre-migration session save#575
AngeloTadeucci merged 3 commits into
masterfrom
feat-session-save

Conversation

@AngeloTadeucci

@AngeloTadeucciAngeloTadeucci commented Sep 28, 2025

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features
    • Added a 5-minute grace period before closing empty dungeon lobbies, preventing abrupt shutdowns while players transition.
  • Bug Fixes
    • Improved reliability during migrations (channel changes, global portals, dungeon transfers, and quit-to-menu) by saving player state prior to transfer, reducing risk of progress loss.
  • Refactor
    • Streamlined internal save flow during migrations and session cleanup for greater consistency.
  • Style
    • Enhanced logging around dungeon lobby creation and disposal timing for clearer operational visibility.

@coderabbitai

coderabbitaiBot commented Sep 28, 2025

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

Walkthrough

Adds a pre-migration persistence step via GameSession.MigrationSave() across multiple migration paths, switches EnterServer locking to account-based, introduces a dungeon lobby empty-grace disposal flow with logging, and defines a new DungeonDisposeEmptyTime constant. Minor formatting/logging tweaks and non-functional collection initializer change included.

Changes

Cohort / File(s)Summary
Session persistence and locking
Maple2.Server.Game/Session/GameSession.cs
Adds preMigrationSaved flag and public MigrationSave() that saves player and component states; invokes MigrationSave() in migration and dispose paths; changes EnterServer locking from CharacterId to AccountId (AcquireLock/ReleaseLock).
Early migration save integration
Maple2.Server.Game/Commands/WarpCommand.cs, Maple2.Server.Game/Manager/DungeonManager.cs, Maple2.Server.Game/PacketHandlers/ChannelHandler.cs, Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs, Maple2.Server.Game/PacketHandlers/QuitHandler.cs
Inserts session.MigrationSave() before initiating various migration flows (warp, dungeon migrate, channel change, global portal, quit non-exit). DungeonManager also formats a collection initializer.
Dungeon disposal timing/logging
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs, Maple2.Model/Metadata/Constants.cs
Replaces immediate dungeon lobby disposal with grace-timer flow that totals players across lobby and sub-rooms, logs state, and disposes after elapsed grace; adds DungeonDisposeEmptyTime = TimeSpan.FromMinutes(5).

Sequence Diagram(s)

sequenceDiagram
autonumber
participant C as Client
participant H as Handler/Manager
participant S as GameSession
participant DB as Persistence
participant P as Planner/Login
C->>H: Action triggers migration (warp/channel/portal/quit)
H->>S: MigrationSave()
alt first-call
S->>DB: Save Player and components (UgcMarket, Config, Shop, Item, Survival, Housing, GameEvent, Achievement, Quest, Dungeon)
DB-->>S: Save results (per-component guarded)
else already-saved
S-->>H: No-op (idempotent)
end
H->>P: MigrateOut request
P-->>H: Migration ticket/ack
H->>C: Instruct disconnect/reconnect (migration)
Loading
sequenceDiagram
autonumber
participant FM as FieldManager
participant DL as Dungeon Lobby Field
participant SR as Sub-Rooms (all)
participant T as Timer
loop DisposeLoop tick
FM->>DL: Check players
FM->>SR: Sum players across rooms
alt Any players > 0
FM->>DL: Reset fieldEmptySince
note right of DL: Skip disposal this tick
else None and no timer set
DL->>T: Start grace timer (DungeonDisposeEmptyTime)
else None and timer elapsed
FM->>DL: DisposeDungeon()
end
end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • Zintixx
  • mettaursp

Poem

I thump my paw—save first, then hop,
Pack carrots, state, and make a stop.
Through portals wide, I glide with grace,
A timed-out dungeon leaves no trace.
Locks on accounts, I twitch and grin—
Migrate, persist… then burrow in! 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 6.67% which is insufficient. The required threshold is 80.00%.You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title Check✅ PassedThe title “feat(session): add pre-migration session save” accurately reflects the primary change in the pull request, which is the introduction of a MigrationSave method in GameSession and its invocation before migration flows. It is concise, clear, and highlights the feature context (session) without extraneous details or noise. This phrasing gives a teammate scanning the repository history a direct understanding that the changeset implements a new pre-migration save capability.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7de323d and d25aa11.

📒 Files selected for processing (1)
  • Maple2.Server.Game/Session/GameSession.cs (6 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1)

26-50: Reset migration persistence when the channel switch fails

When the channel swap RPC throws, we return an error to the client and keep the session alive. Because MigrationSave() ran just before the try/catch, preMigrationSaved remains stuck at true, so every later migration (and even the final dispose) skips saving. Any progress the player makes after the failed channel change is now lost. Please undo or reset the guard in this failure path (e.g., call a ResetMigrationSave() helper or only mark the save as finalized once we know we’re disconnecting).

🧹 Nitpick comments (1)
Maple2.Database/Storage/Game/GameStorage.User.cs (1)

283-407: Remove the dead commented SavePlayer rework

This 120+ line commented block makes the method noisy, obscures the true code path, and risks drifting from reality. If you plan to revisit this logic, keep it in a ticket or design note; otherwise, drop it so the file stays readable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8143d28 and 7de323d.

📒 Files selected for processing (9)
  • Maple2.Database/Storage/Game/GameStorage.User.cs (1 hunks)
  • Maple2.Model/Metadata/Constants.cs (1 hunks)
  • Maple2.Server.Game/Commands/WarpCommand.cs (1 hunks)
  • Maple2.Server.Game/Manager/DungeonManager.cs (2 hunks)
  • Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (2 hunks)
  • Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1 hunks)
  • Maple2.Server.Game/Session/GameSession.cs (6 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Session/GameSession.cs (16)
Maple2.Server.Login/Session/LoginSession.cs (2)
  • AcquireLock (55-73)
  • ReleaseLock (75-86)
Maple2.Database/Storage/Game/DatabaseRequest.cs (3)
  • Commit (20-28)
  • BeginTransaction (16-18)
  • SaveChanges (30-32)
Maple2.Database/Storage/Game/GameStorage.User.cs (4)
  • GameStorage (22-661)
  • Request (23-659)
  • SavePlayer (280-445)
  • Player (211-278)
Maple2.Database/Storage/Game/GameStorage.cs (5)
  • GameStorage (11-80)
  • GameStorage (22-40)
  • Request (42-49)
  • Request (51-79)
  • Request (53-55)
Maple2.Server.Game/Manager/DungeonManager.cs (1)
  • Save (617-619)
Maple2.Server.Game/Manager/AchievementManager.cs (1)
  • Save (302-305)
Maple2.Server.Game/Manager/Items/InventoryManager.cs (2)
  • Save (729-736)
  • Item (569-577)
Maple2.Server.Game/Manager/Items/EquipManager.cs (4)
  • Save (361-368)
  • Item (58-62)
  • Item (64-68)
  • Item (70-72)
Maple2.Server.Game/Manager/HousingManager.cs (1)
  • Save (342-345)
Maple2.Server.Game/Manager/QuestManager.cs (1)
  • Save (669-672)
Maple2.Server.Game/Manager/Items/ItemManager.cs (3)
  • Save (94-98)
  • Item (38-41)
  • Item (48-51)
Maple2.Server.Game/Manager/ShopManager.cs (3)
  • Save (628-651)
  • Shop (130-176)
  • Shop (178-197)
Maple2.Server.Game/Manager/GameEventManager.cs (1)
  • Save (162-171)
Maple2.Server.Game/Manager/ConfigManager.cs (1)
  • Save (572-590)
Maple2.Server.Game/Manager/SurvivalManager.cs (1)
  • Save (173-176)
Maple2.Server.Game/Manager/UgcMarketManager.cs (1)
  • Save (152-155)
Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Manager/DungeonManager.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (1)
Maple2.Model/Metadata/Constants.cs (1)
  • Constant (10-961)
Maple2.Server.Game/Commands/WarpCommand.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build
  • GitHub Check: format
  • GitHub Check: build
🔇 Additional comments (4)
Maple2.Model/Metadata/Constants.cs (1)

154-155: LGTM – consistent disposal constant

Mirrors the existing field timing constants and exposes the new dungeon window cleanly. Also keeps the value localized for any future tuning.

Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (2)

198-199: Appreciate the added dungeon init instrumentation.
The extra lobby context in the debug line will make dungeon spin-up investigations much easier.


286-310: Grace-based disposal flow looks solid.
Aggregating lobby + sub-room player counts before starting the empty timer—and using the new DungeonDisposeEmptyTime window—should prevent premature tears when parties are split across rooms.

Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1)

37-37: Pre-migration save placement looks solid.

Calling MigrationSave() immediately before the migrate-out request mirrors the other migration paths and keeps the flow idempotent thanks to the internal guard. Looks good.

Comment threadMaple2.Server.Game/Session/GameSession.cs
@AngeloTadeucci
AngeloTadeucci merged commit 17beeb1 into masterSep 28, 2025
3 checks passed
@AngeloTadeucci
AngeloTadeucci deleted the feat-session-save branch October 13, 2025 00:38
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@AngeloTadeucci@Zintixx
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

feat(session): add pre-migration session save - #575

Merged
AngeloTadeucci merged 3 commits into
masterfrom
feat-session-save
Sep 28, 2025
Merged

feat(session): add pre-migration session save#575
AngeloTadeucci merged 3 commits into
masterfrom
feat-session-save

Conversation

@AngeloTadeucci

@AngeloTadeucciAngeloTadeucci commented Sep 28, 2025

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features
    • Added a 5-minute grace period before closing empty dungeon lobbies, preventing abrupt shutdowns while players transition.
  • Bug Fixes
    • Improved reliability during migrations (channel changes, global portals, dungeon transfers, and quit-to-menu) by saving player state prior to transfer, reducing risk of progress loss.
  • Refactor
    • Streamlined internal save flow during migrations and session cleanup for greater consistency.
  • Style
    • Enhanced logging around dungeon lobby creation and disposal timing for clearer operational visibility.

@coderabbitai

coderabbitaiBot commented Sep 28, 2025

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

Walkthrough

Adds a pre-migration persistence step via GameSession.MigrationSave() across multiple migration paths, switches EnterServer locking to account-based, introduces a dungeon lobby empty-grace disposal flow with logging, and defines a new DungeonDisposeEmptyTime constant. Minor formatting/logging tweaks and non-functional collection initializer change included.

Changes

Cohort / File(s)Summary
Session persistence and locking
Maple2.Server.Game/Session/GameSession.cs
Adds preMigrationSaved flag and public MigrationSave() that saves player and component states; invokes MigrationSave() in migration and dispose paths; changes EnterServer locking from CharacterId to AccountId (AcquireLock/ReleaseLock).
Early migration save integration
Maple2.Server.Game/Commands/WarpCommand.cs, Maple2.Server.Game/Manager/DungeonManager.cs, Maple2.Server.Game/PacketHandlers/ChannelHandler.cs, Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs, Maple2.Server.Game/PacketHandlers/QuitHandler.cs
Inserts session.MigrationSave() before initiating various migration flows (warp, dungeon migrate, channel change, global portal, quit non-exit). DungeonManager also formats a collection initializer.
Dungeon disposal timing/logging
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs, Maple2.Model/Metadata/Constants.cs
Replaces immediate dungeon lobby disposal with grace-timer flow that totals players across lobby and sub-rooms, logs state, and disposes after elapsed grace; adds DungeonDisposeEmptyTime = TimeSpan.FromMinutes(5).

Sequence Diagram(s)

sequenceDiagram
autonumber
participant C as Client
participant H as Handler/Manager
participant S as GameSession
participant DB as Persistence
participant P as Planner/Login
C->>H: Action triggers migration (warp/channel/portal/quit)
H->>S: MigrationSave()
alt first-call
S->>DB: Save Player and components (UgcMarket, Config, Shop, Item, Survival, Housing, GameEvent, Achievement, Quest, Dungeon)
DB-->>S: Save results (per-component guarded)
else already-saved
S-->>H: No-op (idempotent)
end
H->>P: MigrateOut request
P-->>H: Migration ticket/ack
H->>C: Instruct disconnect/reconnect (migration)
Loading
sequenceDiagram
autonumber
participant FM as FieldManager
participant DL as Dungeon Lobby Field
participant SR as Sub-Rooms (all)
participant T as Timer
loop DisposeLoop tick
FM->>DL: Check players
FM->>SR: Sum players across rooms
alt Any players > 0
FM->>DL: Reset fieldEmptySince
note right of DL: Skip disposal this tick
else None and no timer set
DL->>T: Start grace timer (DungeonDisposeEmptyTime)
else None and timer elapsed
FM->>DL: DisposeDungeon()
end
end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • Zintixx
  • mettaursp

Poem

I thump my paw—save first, then hop,
Pack carrots, state, and make a stop.
Through portals wide, I glide with grace,
A timed-out dungeon leaves no trace.
Locks on accounts, I twitch and grin—
Migrate, persist… then burrow in! 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 6.67% which is insufficient. The required threshold is 80.00%.You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title Check✅ PassedThe title “feat(session): add pre-migration session save” accurately reflects the primary change in the pull request, which is the introduction of a MigrationSave method in GameSession and its invocation before migration flows. It is concise, clear, and highlights the feature context (session) without extraneous details or noise. This phrasing gives a teammate scanning the repository history a direct understanding that the changeset implements a new pre-migration save capability.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7de323d and d25aa11.

📒 Files selected for processing (1)
  • Maple2.Server.Game/Session/GameSession.cs (6 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1)

26-50: Reset migration persistence when the channel switch fails

When the channel swap RPC throws, we return an error to the client and keep the session alive. Because MigrationSave() ran just before the try/catch, preMigrationSaved remains stuck at true, so every later migration (and even the final dispose) skips saving. Any progress the player makes after the failed channel change is now lost. Please undo or reset the guard in this failure path (e.g., call a ResetMigrationSave() helper or only mark the save as finalized once we know we’re disconnecting).

🧹 Nitpick comments (1)
Maple2.Database/Storage/Game/GameStorage.User.cs (1)

283-407: Remove the dead commented SavePlayer rework

This 120+ line commented block makes the method noisy, obscures the true code path, and risks drifting from reality. If you plan to revisit this logic, keep it in a ticket or design note; otherwise, drop it so the file stays readable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8143d28 and 7de323d.

📒 Files selected for processing (9)
  • Maple2.Database/Storage/Game/GameStorage.User.cs (1 hunks)
  • Maple2.Model/Metadata/Constants.cs (1 hunks)
  • Maple2.Server.Game/Commands/WarpCommand.cs (1 hunks)
  • Maple2.Server.Game/Manager/DungeonManager.cs (2 hunks)
  • Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (2 hunks)
  • Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1 hunks)
  • Maple2.Server.Game/Session/GameSession.cs (6 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Session/GameSession.cs (16)
Maple2.Server.Login/Session/LoginSession.cs (2)
  • AcquireLock (55-73)
  • ReleaseLock (75-86)
Maple2.Database/Storage/Game/DatabaseRequest.cs (3)
  • Commit (20-28)
  • BeginTransaction (16-18)
  • SaveChanges (30-32)
Maple2.Database/Storage/Game/GameStorage.User.cs (4)
  • GameStorage (22-661)
  • Request (23-659)
  • SavePlayer (280-445)
  • Player (211-278)
Maple2.Database/Storage/Game/GameStorage.cs (5)
  • GameStorage (11-80)
  • GameStorage (22-40)
  • Request (42-49)
  • Request (51-79)
  • Request (53-55)
Maple2.Server.Game/Manager/DungeonManager.cs (1)
  • Save (617-619)
Maple2.Server.Game/Manager/AchievementManager.cs (1)
  • Save (302-305)
Maple2.Server.Game/Manager/Items/InventoryManager.cs (2)
  • Save (729-736)
  • Item (569-577)
Maple2.Server.Game/Manager/Items/EquipManager.cs (4)
  • Save (361-368)
  • Item (58-62)
  • Item (64-68)
  • Item (70-72)
Maple2.Server.Game/Manager/HousingManager.cs (1)
  • Save (342-345)
Maple2.Server.Game/Manager/QuestManager.cs (1)
  • Save (669-672)
Maple2.Server.Game/Manager/Items/ItemManager.cs (3)
  • Save (94-98)
  • Item (38-41)
  • Item (48-51)
Maple2.Server.Game/Manager/ShopManager.cs (3)
  • Save (628-651)
  • Shop (130-176)
  • Shop (178-197)
Maple2.Server.Game/Manager/GameEventManager.cs (1)
  • Save (162-171)
Maple2.Server.Game/Manager/ConfigManager.cs (1)
  • Save (572-590)
Maple2.Server.Game/Manager/SurvivalManager.cs (1)
  • Save (173-176)
Maple2.Server.Game/Manager/UgcMarketManager.cs (1)
  • Save (152-155)
Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Manager/DungeonManager.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (1)
Maple2.Model/Metadata/Constants.cs (1)
  • Constant (10-961)
Maple2.Server.Game/Commands/WarpCommand.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build
  • GitHub Check: format
  • GitHub Check: build
🔇 Additional comments (4)
Maple2.Model/Metadata/Constants.cs (1)

154-155: LGTM – consistent disposal constant

Mirrors the existing field timing constants and exposes the new dungeon window cleanly. Also keeps the value localized for any future tuning.

Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (2)

198-199: Appreciate the added dungeon init instrumentation.
The extra lobby context in the debug line will make dungeon spin-up investigations much easier.


286-310: Grace-based disposal flow looks solid.
Aggregating lobby + sub-room player counts before starting the empty timer—and using the new DungeonDisposeEmptyTime window—should prevent premature tears when parties are split across rooms.

Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1)

37-37: Pre-migration save placement looks solid.

Calling MigrationSave() immediately before the migrate-out request mirrors the other migration paths and keeps the flow idempotent thanks to the internal guard. Looks good.

Comment threadMaple2.Server.Game/Session/GameSession.cs
@AngeloTadeucci
AngeloTadeucci merged commit 17beeb1 into masterSep 28, 2025
3 checks passed
@AngeloTadeucci
AngeloTadeucci deleted the feat-session-save branch October 13, 2025 00:38
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@AngeloTadeucci@Zintixx
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(session): add pre-migration session save - #575

Merged
AngeloTadeucci merged 3 commits into
masterfrom
feat-session-save
Sep 28, 2025
Merged

feat(session): add pre-migration session save#575
AngeloTadeucci merged 3 commits into
masterfrom
feat-session-save

Conversation

@AngeloTadeucci

@AngeloTadeucciAngeloTadeucci commented Sep 28, 2025

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features
    • Added a 5-minute grace period before closing empty dungeon lobbies, preventing abrupt shutdowns while players transition.
  • Bug Fixes
    • Improved reliability during migrations (channel changes, global portals, dungeon transfers, and quit-to-menu) by saving player state prior to transfer, reducing risk of progress loss.
  • Refactor
    • Streamlined internal save flow during migrations and session cleanup for greater consistency.
  • Style
    • Enhanced logging around dungeon lobby creation and disposal timing for clearer operational visibility.

@coderabbitai

coderabbitaiBot commented Sep 28, 2025

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

Walkthrough

Adds a pre-migration persistence step via GameSession.MigrationSave() across multiple migration paths, switches EnterServer locking to account-based, introduces a dungeon lobby empty-grace disposal flow with logging, and defines a new DungeonDisposeEmptyTime constant. Minor formatting/logging tweaks and non-functional collection initializer change included.

Changes

Cohort / File(s)Summary
Session persistence and locking
Maple2.Server.Game/Session/GameSession.cs
Adds preMigrationSaved flag and public MigrationSave() that saves player and component states; invokes MigrationSave() in migration and dispose paths; changes EnterServer locking from CharacterId to AccountId (AcquireLock/ReleaseLock).
Early migration save integration
Maple2.Server.Game/Commands/WarpCommand.cs, Maple2.Server.Game/Manager/DungeonManager.cs, Maple2.Server.Game/PacketHandlers/ChannelHandler.cs, Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs, Maple2.Server.Game/PacketHandlers/QuitHandler.cs
Inserts session.MigrationSave() before initiating various migration flows (warp, dungeon migrate, channel change, global portal, quit non-exit). DungeonManager also formats a collection initializer.
Dungeon disposal timing/logging
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs, Maple2.Model/Metadata/Constants.cs
Replaces immediate dungeon lobby disposal with grace-timer flow that totals players across lobby and sub-rooms, logs state, and disposes after elapsed grace; adds DungeonDisposeEmptyTime = TimeSpan.FromMinutes(5).

Sequence Diagram(s)

sequenceDiagram
autonumber
participant C as Client
participant H as Handler/Manager
participant S as GameSession
participant DB as Persistence
participant P as Planner/Login
C->>H: Action triggers migration (warp/channel/portal/quit)
H->>S: MigrationSave()
alt first-call
S->>DB: Save Player and components (UgcMarket, Config, Shop, Item, Survival, Housing, GameEvent, Achievement, Quest, Dungeon)
DB-->>S: Save results (per-component guarded)
else already-saved
S-->>H: No-op (idempotent)
end
H->>P: MigrateOut request
P-->>H: Migration ticket/ack
H->>C: Instruct disconnect/reconnect (migration)
Loading
sequenceDiagram
autonumber
participant FM as FieldManager
participant DL as Dungeon Lobby Field
participant SR as Sub-Rooms (all)
participant T as Timer
loop DisposeLoop tick
FM->>DL: Check players
FM->>SR: Sum players across rooms
alt Any players > 0
FM->>DL: Reset fieldEmptySince
note right of DL: Skip disposal this tick
else None and no timer set
DL->>T: Start grace timer (DungeonDisposeEmptyTime)
else None and timer elapsed
FM->>DL: DisposeDungeon()
end
end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • Zintixx
  • mettaursp

Poem

I thump my paw—save first, then hop,
Pack carrots, state, and make a stop.
Through portals wide, I glide with grace,
A timed-out dungeon leaves no trace.
Locks on accounts, I twitch and grin—
Migrate, persist… then burrow in! 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 6.67% which is insufficient. The required threshold is 80.00%.You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title Check✅ PassedThe title “feat(session): add pre-migration session save” accurately reflects the primary change in the pull request, which is the introduction of a MigrationSave method in GameSession and its invocation before migration flows. It is concise, clear, and highlights the feature context (session) without extraneous details or noise. This phrasing gives a teammate scanning the repository history a direct understanding that the changeset implements a new pre-migration save capability.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7de323d and d25aa11.

📒 Files selected for processing (1)
  • Maple2.Server.Game/Session/GameSession.cs (6 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1)

26-50: Reset migration persistence when the channel switch fails

When the channel swap RPC throws, we return an error to the client and keep the session alive. Because MigrationSave() ran just before the try/catch, preMigrationSaved remains stuck at true, so every later migration (and even the final dispose) skips saving. Any progress the player makes after the failed channel change is now lost. Please undo or reset the guard in this failure path (e.g., call a ResetMigrationSave() helper or only mark the save as finalized once we know we’re disconnecting).

🧹 Nitpick comments (1)
Maple2.Database/Storage/Game/GameStorage.User.cs (1)

283-407: Remove the dead commented SavePlayer rework

This 120+ line commented block makes the method noisy, obscures the true code path, and risks drifting from reality. If you plan to revisit this logic, keep it in a ticket or design note; otherwise, drop it so the file stays readable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8143d28 and 7de323d.

📒 Files selected for processing (9)
  • Maple2.Database/Storage/Game/GameStorage.User.cs (1 hunks)
  • Maple2.Model/Metadata/Constants.cs (1 hunks)
  • Maple2.Server.Game/Commands/WarpCommand.cs (1 hunks)
  • Maple2.Server.Game/Manager/DungeonManager.cs (2 hunks)
  • Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (2 hunks)
  • Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1 hunks)
  • Maple2.Server.Game/Session/GameSession.cs (6 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Session/GameSession.cs (16)
Maple2.Server.Login/Session/LoginSession.cs (2)
  • AcquireLock (55-73)
  • ReleaseLock (75-86)
Maple2.Database/Storage/Game/DatabaseRequest.cs (3)
  • Commit (20-28)
  • BeginTransaction (16-18)
  • SaveChanges (30-32)
Maple2.Database/Storage/Game/GameStorage.User.cs (4)
  • GameStorage (22-661)
  • Request (23-659)
  • SavePlayer (280-445)
  • Player (211-278)
Maple2.Database/Storage/Game/GameStorage.cs (5)
  • GameStorage (11-80)
  • GameStorage (22-40)
  • Request (42-49)
  • Request (51-79)
  • Request (53-55)
Maple2.Server.Game/Manager/DungeonManager.cs (1)
  • Save (617-619)
Maple2.Server.Game/Manager/AchievementManager.cs (1)
  • Save (302-305)
Maple2.Server.Game/Manager/Items/InventoryManager.cs (2)
  • Save (729-736)
  • Item (569-577)
Maple2.Server.Game/Manager/Items/EquipManager.cs (4)
  • Save (361-368)
  • Item (58-62)
  • Item (64-68)
  • Item (70-72)
Maple2.Server.Game/Manager/HousingManager.cs (1)
  • Save (342-345)
Maple2.Server.Game/Manager/QuestManager.cs (1)
  • Save (669-672)
Maple2.Server.Game/Manager/Items/ItemManager.cs (3)
  • Save (94-98)
  • Item (38-41)
  • Item (48-51)
Maple2.Server.Game/Manager/ShopManager.cs (3)
  • Save (628-651)
  • Shop (130-176)
  • Shop (178-197)
Maple2.Server.Game/Manager/GameEventManager.cs (1)
  • Save (162-171)
Maple2.Server.Game/Manager/ConfigManager.cs (1)
  • Save (572-590)
Maple2.Server.Game/Manager/SurvivalManager.cs (1)
  • Save (173-176)
Maple2.Server.Game/Manager/UgcMarketManager.cs (1)
  • Save (152-155)
Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Manager/DungeonManager.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (1)
Maple2.Model/Metadata/Constants.cs (1)
  • Constant (10-961)
Maple2.Server.Game/Commands/WarpCommand.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build
  • GitHub Check: format
  • GitHub Check: build
🔇 Additional comments (4)
Maple2.Model/Metadata/Constants.cs (1)

154-155: LGTM – consistent disposal constant

Mirrors the existing field timing constants and exposes the new dungeon window cleanly. Also keeps the value localized for any future tuning.

Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (2)

198-199: Appreciate the added dungeon init instrumentation.
The extra lobby context in the debug line will make dungeon spin-up investigations much easier.


286-310: Grace-based disposal flow looks solid.
Aggregating lobby + sub-room player counts before starting the empty timer—and using the new DungeonDisposeEmptyTime window—should prevent premature tears when parties are split across rooms.

Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1)

37-37: Pre-migration save placement looks solid.

Calling MigrationSave() immediately before the migrate-out request mirrors the other migration paths and keeps the flow idempotent thanks to the internal guard. Looks good.

Comment threadMaple2.Server.Game/Session/GameSession.cs
@AngeloTadeucci
AngeloTadeucci merged commit 17beeb1 into masterSep 28, 2025
3 checks passed
@AngeloTadeucci
AngeloTadeucci deleted the feat-session-save branch October 13, 2025 00:38
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@AngeloTadeucci@Zintixx
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(session): add pre-migration session save - #575

Merged
AngeloTadeucci merged 3 commits into
masterfrom
feat-session-save
Sep 28, 2025
Merged

feat(session): add pre-migration session save#575
AngeloTadeucci merged 3 commits into
masterfrom
feat-session-save

Conversation

@AngeloTadeucci

@AngeloTadeucciAngeloTadeucci commented Sep 28, 2025

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features
    • Added a 5-minute grace period before closing empty dungeon lobbies, preventing abrupt shutdowns while players transition.
  • Bug Fixes
    • Improved reliability during migrations (channel changes, global portals, dungeon transfers, and quit-to-menu) by saving player state prior to transfer, reducing risk of progress loss.
  • Refactor
    • Streamlined internal save flow during migrations and session cleanup for greater consistency.
  • Style
    • Enhanced logging around dungeon lobby creation and disposal timing for clearer operational visibility.

@coderabbitai

coderabbitaiBot commented Sep 28, 2025

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

Walkthrough

Adds a pre-migration persistence step via GameSession.MigrationSave() across multiple migration paths, switches EnterServer locking to account-based, introduces a dungeon lobby empty-grace disposal flow with logging, and defines a new DungeonDisposeEmptyTime constant. Minor formatting/logging tweaks and non-functional collection initializer change included.

Changes

Cohort / File(s)Summary
Session persistence and locking
Maple2.Server.Game/Session/GameSession.cs
Adds preMigrationSaved flag and public MigrationSave() that saves player and component states; invokes MigrationSave() in migration and dispose paths; changes EnterServer locking from CharacterId to AccountId (AcquireLock/ReleaseLock).
Early migration save integration
Maple2.Server.Game/Commands/WarpCommand.cs, Maple2.Server.Game/Manager/DungeonManager.cs, Maple2.Server.Game/PacketHandlers/ChannelHandler.cs, Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs, Maple2.Server.Game/PacketHandlers/QuitHandler.cs
Inserts session.MigrationSave() before initiating various migration flows (warp, dungeon migrate, channel change, global portal, quit non-exit). DungeonManager also formats a collection initializer.
Dungeon disposal timing/logging
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs, Maple2.Model/Metadata/Constants.cs
Replaces immediate dungeon lobby disposal with grace-timer flow that totals players across lobby and sub-rooms, logs state, and disposes after elapsed grace; adds DungeonDisposeEmptyTime = TimeSpan.FromMinutes(5).

Sequence Diagram(s)

sequenceDiagram
autonumber
participant C as Client
participant H as Handler/Manager
participant S as GameSession
participant DB as Persistence
participant P as Planner/Login
C->>H: Action triggers migration (warp/channel/portal/quit)
H->>S: MigrationSave()
alt first-call
S->>DB: Save Player and components (UgcMarket, Config, Shop, Item, Survival, Housing, GameEvent, Achievement, Quest, Dungeon)
DB-->>S: Save results (per-component guarded)
else already-saved
S-->>H: No-op (idempotent)
end
H->>P: MigrateOut request
P-->>H: Migration ticket/ack
H->>C: Instruct disconnect/reconnect (migration)
Loading
sequenceDiagram
autonumber
participant FM as FieldManager
participant DL as Dungeon Lobby Field
participant SR as Sub-Rooms (all)
participant T as Timer
loop DisposeLoop tick
FM->>DL: Check players
FM->>SR: Sum players across rooms
alt Any players > 0
FM->>DL: Reset fieldEmptySince
note right of DL: Skip disposal this tick
else None and no timer set
DL->>T: Start grace timer (DungeonDisposeEmptyTime)
else None and timer elapsed
FM->>DL: DisposeDungeon()
end
end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • Zintixx
  • mettaursp

Poem

I thump my paw—save first, then hop,
Pack carrots, state, and make a stop.
Through portals wide, I glide with grace,
A timed-out dungeon leaves no trace.
Locks on accounts, I twitch and grin—
Migrate, persist… then burrow in! 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 6.67% which is insufficient. The required threshold is 80.00%.You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title Check✅ PassedThe title “feat(session): add pre-migration session save” accurately reflects the primary change in the pull request, which is the introduction of a MigrationSave method in GameSession and its invocation before migration flows. It is concise, clear, and highlights the feature context (session) without extraneous details or noise. This phrasing gives a teammate scanning the repository history a direct understanding that the changeset implements a new pre-migration save capability.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7de323d and d25aa11.

📒 Files selected for processing (1)
  • Maple2.Server.Game/Session/GameSession.cs (6 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1)

26-50: Reset migration persistence when the channel switch fails

When the channel swap RPC throws, we return an error to the client and keep the session alive. Because MigrationSave() ran just before the try/catch, preMigrationSaved remains stuck at true, so every later migration (and even the final dispose) skips saving. Any progress the player makes after the failed channel change is now lost. Please undo or reset the guard in this failure path (e.g., call a ResetMigrationSave() helper or only mark the save as finalized once we know we’re disconnecting).

🧹 Nitpick comments (1)
Maple2.Database/Storage/Game/GameStorage.User.cs (1)

283-407: Remove the dead commented SavePlayer rework

This 120+ line commented block makes the method noisy, obscures the true code path, and risks drifting from reality. If you plan to revisit this logic, keep it in a ticket or design note; otherwise, drop it so the file stays readable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8143d28 and 7de323d.

📒 Files selected for processing (9)
  • Maple2.Database/Storage/Game/GameStorage.User.cs (1 hunks)
  • Maple2.Model/Metadata/Constants.cs (1 hunks)
  • Maple2.Server.Game/Commands/WarpCommand.cs (1 hunks)
  • Maple2.Server.Game/Manager/DungeonManager.cs (2 hunks)
  • Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (2 hunks)
  • Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1 hunks)
  • Maple2.Server.Game/Session/GameSession.cs (6 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Session/GameSession.cs (16)
Maple2.Server.Login/Session/LoginSession.cs (2)
  • AcquireLock (55-73)
  • ReleaseLock (75-86)
Maple2.Database/Storage/Game/DatabaseRequest.cs (3)
  • Commit (20-28)
  • BeginTransaction (16-18)
  • SaveChanges (30-32)
Maple2.Database/Storage/Game/GameStorage.User.cs (4)
  • GameStorage (22-661)
  • Request (23-659)
  • SavePlayer (280-445)
  • Player (211-278)
Maple2.Database/Storage/Game/GameStorage.cs (5)
  • GameStorage (11-80)
  • GameStorage (22-40)
  • Request (42-49)
  • Request (51-79)
  • Request (53-55)
Maple2.Server.Game/Manager/DungeonManager.cs (1)
  • Save (617-619)
Maple2.Server.Game/Manager/AchievementManager.cs (1)
  • Save (302-305)
Maple2.Server.Game/Manager/Items/InventoryManager.cs (2)
  • Save (729-736)
  • Item (569-577)
Maple2.Server.Game/Manager/Items/EquipManager.cs (4)
  • Save (361-368)
  • Item (58-62)
  • Item (64-68)
  • Item (70-72)
Maple2.Server.Game/Manager/HousingManager.cs (1)
  • Save (342-345)
Maple2.Server.Game/Manager/QuestManager.cs (1)
  • Save (669-672)
Maple2.Server.Game/Manager/Items/ItemManager.cs (3)
  • Save (94-98)
  • Item (38-41)
  • Item (48-51)
Maple2.Server.Game/Manager/ShopManager.cs (3)
  • Save (628-651)
  • Shop (130-176)
  • Shop (178-197)
Maple2.Server.Game/Manager/GameEventManager.cs (1)
  • Save (162-171)
Maple2.Server.Game/Manager/ConfigManager.cs (1)
  • Save (572-590)
Maple2.Server.Game/Manager/SurvivalManager.cs (1)
  • Save (173-176)
Maple2.Server.Game/Manager/UgcMarketManager.cs (1)
  • Save (152-155)
Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Manager/DungeonManager.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (1)
Maple2.Model/Metadata/Constants.cs (1)
  • Constant (10-961)
Maple2.Server.Game/Commands/WarpCommand.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build
  • GitHub Check: format
  • GitHub Check: build
🔇 Additional comments (4)
Maple2.Model/Metadata/Constants.cs (1)

154-155: LGTM – consistent disposal constant

Mirrors the existing field timing constants and exposes the new dungeon window cleanly. Also keeps the value localized for any future tuning.

Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (2)

198-199: Appreciate the added dungeon init instrumentation.
The extra lobby context in the debug line will make dungeon spin-up investigations much easier.


286-310: Grace-based disposal flow looks solid.
Aggregating lobby + sub-room player counts before starting the empty timer—and using the new DungeonDisposeEmptyTime window—should prevent premature tears when parties are split across rooms.

Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1)

37-37: Pre-migration save placement looks solid.

Calling MigrationSave() immediately before the migrate-out request mirrors the other migration paths and keeps the flow idempotent thanks to the internal guard. Looks good.

Comment threadMaple2.Server.Game/Session/GameSession.cs
@AngeloTadeucci
AngeloTadeucci merged commit 17beeb1 into masterSep 28, 2025
3 checks passed
@AngeloTadeucci
AngeloTadeucci deleted the feat-session-save branch October 13, 2025 00:38
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@AngeloTadeucci@Zintixx
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

feat(session): add pre-migration session save - #575

Merged
AngeloTadeucci merged 3 commits into
masterfrom
feat-session-save
Sep 28, 2025
Merged

feat(session): add pre-migration session save#575
AngeloTadeucci merged 3 commits into
masterfrom
feat-session-save

Conversation

@AngeloTadeucci

@AngeloTadeucciAngeloTadeucci commented Sep 28, 2025

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features
    • Added a 5-minute grace period before closing empty dungeon lobbies, preventing abrupt shutdowns while players transition.
  • Bug Fixes
    • Improved reliability during migrations (channel changes, global portals, dungeon transfers, and quit-to-menu) by saving player state prior to transfer, reducing risk of progress loss.
  • Refactor
    • Streamlined internal save flow during migrations and session cleanup for greater consistency.
  • Style
    • Enhanced logging around dungeon lobby creation and disposal timing for clearer operational visibility.

@coderabbitai

coderabbitaiBot commented Sep 28, 2025

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

Walkthrough

Adds a pre-migration persistence step via GameSession.MigrationSave() across multiple migration paths, switches EnterServer locking to account-based, introduces a dungeon lobby empty-grace disposal flow with logging, and defines a new DungeonDisposeEmptyTime constant. Minor formatting/logging tweaks and non-functional collection initializer change included.

Changes

Cohort / File(s)Summary
Session persistence and locking
Maple2.Server.Game/Session/GameSession.cs
Adds preMigrationSaved flag and public MigrationSave() that saves player and component states; invokes MigrationSave() in migration and dispose paths; changes EnterServer locking from CharacterId to AccountId (AcquireLock/ReleaseLock).
Early migration save integration
Maple2.Server.Game/Commands/WarpCommand.cs, Maple2.Server.Game/Manager/DungeonManager.cs, Maple2.Server.Game/PacketHandlers/ChannelHandler.cs, Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs, Maple2.Server.Game/PacketHandlers/QuitHandler.cs
Inserts session.MigrationSave() before initiating various migration flows (warp, dungeon migrate, channel change, global portal, quit non-exit). DungeonManager also formats a collection initializer.
Dungeon disposal timing/logging
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs, Maple2.Model/Metadata/Constants.cs
Replaces immediate dungeon lobby disposal with grace-timer flow that totals players across lobby and sub-rooms, logs state, and disposes after elapsed grace; adds DungeonDisposeEmptyTime = TimeSpan.FromMinutes(5).

Sequence Diagram(s)

sequenceDiagram
autonumber
participant C as Client
participant H as Handler/Manager
participant S as GameSession
participant DB as Persistence
participant P as Planner/Login
C->>H: Action triggers migration (warp/channel/portal/quit)
H->>S: MigrationSave()
alt first-call
S->>DB: Save Player and components (UgcMarket, Config, Shop, Item, Survival, Housing, GameEvent, Achievement, Quest, Dungeon)
DB-->>S: Save results (per-component guarded)
else already-saved
S-->>H: No-op (idempotent)
end
H->>P: MigrateOut request
P-->>H: Migration ticket/ack
H->>C: Instruct disconnect/reconnect (migration)
Loading
sequenceDiagram
autonumber
participant FM as FieldManager
participant DL as Dungeon Lobby Field
participant SR as Sub-Rooms (all)
participant T as Timer
loop DisposeLoop tick
FM->>DL: Check players
FM->>SR: Sum players across rooms
alt Any players > 0
FM->>DL: Reset fieldEmptySince
note right of DL: Skip disposal this tick
else None and no timer set
DL->>T: Start grace timer (DungeonDisposeEmptyTime)
else None and timer elapsed
FM->>DL: DisposeDungeon()
end
end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • Zintixx
  • mettaursp

Poem

I thump my paw—save first, then hop,
Pack carrots, state, and make a stop.
Through portals wide, I glide with grace,
A timed-out dungeon leaves no trace.
Locks on accounts, I twitch and grin—
Migrate, persist… then burrow in! 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 6.67% which is insufficient. The required threshold is 80.00%.You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title Check✅ PassedThe title “feat(session): add pre-migration session save” accurately reflects the primary change in the pull request, which is the introduction of a MigrationSave method in GameSession and its invocation before migration flows. It is concise, clear, and highlights the feature context (session) without extraneous details or noise. This phrasing gives a teammate scanning the repository history a direct understanding that the changeset implements a new pre-migration save capability.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7de323d and d25aa11.

📒 Files selected for processing (1)
  • Maple2.Server.Game/Session/GameSession.cs (6 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1)

26-50: Reset migration persistence when the channel switch fails

When the channel swap RPC throws, we return an error to the client and keep the session alive. Because MigrationSave() ran just before the try/catch, preMigrationSaved remains stuck at true, so every later migration (and even the final dispose) skips saving. Any progress the player makes after the failed channel change is now lost. Please undo or reset the guard in this failure path (e.g., call a ResetMigrationSave() helper or only mark the save as finalized once we know we’re disconnecting).

🧹 Nitpick comments (1)
Maple2.Database/Storage/Game/GameStorage.User.cs (1)

283-407: Remove the dead commented SavePlayer rework

This 120+ line commented block makes the method noisy, obscures the true code path, and risks drifting from reality. If you plan to revisit this logic, keep it in a ticket or design note; otherwise, drop it so the file stays readable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8143d28 and 7de323d.

📒 Files selected for processing (9)
  • Maple2.Database/Storage/Game/GameStorage.User.cs (1 hunks)
  • Maple2.Model/Metadata/Constants.cs (1 hunks)
  • Maple2.Server.Game/Commands/WarpCommand.cs (1 hunks)
  • Maple2.Server.Game/Manager/DungeonManager.cs (2 hunks)
  • Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (2 hunks)
  • Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1 hunks)
  • Maple2.Server.Game/Session/GameSession.cs (6 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Session/GameSession.cs (16)
Maple2.Server.Login/Session/LoginSession.cs (2)
  • AcquireLock (55-73)
  • ReleaseLock (75-86)
Maple2.Database/Storage/Game/DatabaseRequest.cs (3)
  • Commit (20-28)
  • BeginTransaction (16-18)
  • SaveChanges (30-32)
Maple2.Database/Storage/Game/GameStorage.User.cs (4)
  • GameStorage (22-661)
  • Request (23-659)
  • SavePlayer (280-445)
  • Player (211-278)
Maple2.Database/Storage/Game/GameStorage.cs (5)
  • GameStorage (11-80)
  • GameStorage (22-40)
  • Request (42-49)
  • Request (51-79)
  • Request (53-55)
Maple2.Server.Game/Manager/DungeonManager.cs (1)
  • Save (617-619)
Maple2.Server.Game/Manager/AchievementManager.cs (1)
  • Save (302-305)
Maple2.Server.Game/Manager/Items/InventoryManager.cs (2)
  • Save (729-736)
  • Item (569-577)
Maple2.Server.Game/Manager/Items/EquipManager.cs (4)
  • Save (361-368)
  • Item (58-62)
  • Item (64-68)
  • Item (70-72)
Maple2.Server.Game/Manager/HousingManager.cs (1)
  • Save (342-345)
Maple2.Server.Game/Manager/QuestManager.cs (1)
  • Save (669-672)
Maple2.Server.Game/Manager/Items/ItemManager.cs (3)
  • Save (94-98)
  • Item (38-41)
  • Item (48-51)
Maple2.Server.Game/Manager/ShopManager.cs (3)
  • Save (628-651)
  • Shop (130-176)
  • Shop (178-197)
Maple2.Server.Game/Manager/GameEventManager.cs (1)
  • Save (162-171)
Maple2.Server.Game/Manager/ConfigManager.cs (1)
  • Save (572-590)
Maple2.Server.Game/Manager/SurvivalManager.cs (1)
  • Save (173-176)
Maple2.Server.Game/Manager/UgcMarketManager.cs (1)
  • Save (152-155)
Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/PacketHandlers/GlobalPortalHandler.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Manager/DungeonManager.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (1)
Maple2.Model/Metadata/Constants.cs (1)
  • Constant (10-961)
Maple2.Server.Game/Commands/WarpCommand.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1)
  • MigrationSave (848-877)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build
  • GitHub Check: format
  • GitHub Check: build
🔇 Additional comments (4)
Maple2.Model/Metadata/Constants.cs (1)

154-155: LGTM – consistent disposal constant

Mirrors the existing field timing constants and exposes the new dungeon window cleanly. Also keeps the value localized for any future tuning.

Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (2)

198-199: Appreciate the added dungeon init instrumentation.
The extra lobby context in the debug line will make dungeon spin-up investigations much easier.


286-310: Grace-based disposal flow looks solid.
Aggregating lobby + sub-room player counts before starting the empty timer—and using the new DungeonDisposeEmptyTime window—should prevent premature tears when parties are split across rooms.

Maple2.Server.Game/PacketHandlers/QuitHandler.cs (1)

37-37: Pre-migration save placement looks solid.

Calling MigrationSave() immediately before the migrate-out request mirrors the other migration paths and keeps the flow idempotent thanks to the internal guard. Looks good.

Comment threadMaple2.Server.Game/Session/GameSession.cs
@AngeloTadeucci
AngeloTadeucci merged commit 17beeb1 into masterSep 28, 2025
3 checks passed
@AngeloTadeucci
AngeloTadeucci deleted the feat-session-save branch October 13, 2025 00:38
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@AngeloTadeucci@Zintixx