Conversation
WalkthroughThis update introduces new event types and related logic for handling Bingo and TimeRun events across the server, model, and packet layers. The event data model is extended with new record types for Bingo, TimeRun, MapleSurvivalOpenPeriod, ShutdownMapleSurvival, and Ontime events. Corresponding parsing and serialization logic is implemented in the server and model layers. The server now processes new event reward commands for Bingo and TimeRun, including user interactions such as opening a bingo board, crossing off numbers, and claiming rewards. Additionally, item use handling is enhanced with support for quest scrolls, and related packet construction methods are added. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant EventRewardHandler
participant Inventory
participant Mailbox
%% Bingo: Claim Reward
Client->>Server: RequestClaimBingoReward(rewardIndex)
Server->>EventRewardHandler: HandleClaimBingoReward
EventRewardHandler->>Inventory: TryAddRewardItem
alt Inventory full
EventRewardHandler->>Mailbox: MailRewardItem
end
EventRewardHandler-->>Client: ClaimBingoRewardPacket
EventRewardHandler-->>Client: UpdateBingoPacket
sequenceDiagram
participant Client
participant Server
participant ItemUseHandler
participant Inventory
%% Quest Scroll Usage
Client->>Server: UseItem(QuestScroll)
Server->>ItemUseHandler: HandleQuestScroll
ItemUseHandler->>Inventory: ConsumeItem(QuestScroll)
ItemUseHandler-->>Client: QuestScrollUsedPacket
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (13)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (12)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (7)
Maple2.Model/Enum/GameEventUserValueType.cs (1)
32-35: Address the TODO and consider documenting enum valuesThe added Bingo event types include a TODO comment indicating these are placeholder values. Before finalizing, determine the actual confirmed values. Additionally, consider adding documentation comments to explain what each enum value represents and how they're used in the Bingo event system.
- // Bingo - TODO: These are not the actual confirmed values. Just using it as a way to store this data for now. + // Bingo event tracking - Used to store user-specific bingo state BingoUid = 4000, // Unique identifier for the player's bingo card BingoRewardsClaimed = 4001, // Bitmap of rewards that have been claimed BingoNumbersChecked = 4002, // Bitmap of numbers that have been checked offMaple2.Server.Game/Packets/EventRewardPacket.cs (2)
20-48: Missingcountfor reward items
Unknown1,ClaimTimeRunFinalReward, andClaimTimeRunStepRewardonly writeitemIdandrarity, omittingcount.
Other reward-related packets (e.g., Gallery) includecountwhen applicable. Verify the client expects a quantity; if so, extend the payload:- pWriter.WriteInt(item.ItemId); - pWriter.WriteShort(item.Rarity); + pWriter.WriteInt(item.ItemId); + pWriter.WriteShort(item.Rarity); + pWriter.WriteInt(item.Amount);
70-91: Validate string length vs. protocol limits
checkedNumbersandrewardsClaimedcan grow over time. If the protocol caps these fields (commonly 255 or 1024 bytes), enforce a length check or truncate to prevent malformed packets and potential disconnects.Maple2.File.Ingest/Mapper/ServerTableMapper.cs (1)
1216-1269:TimeRunEvent: step-reward metadata ignoredStep rewards are initialised with an empty dictionary:
StepRewards: new Dictionary<int, RewardItem>(), // No step rewards were added …If step-reward information later appears in the XML (likely
value4), this implementation will silently drop it, forcing a server patch.
Consider parsingvalue4now (or at least logging a TODO) to future-proof the mapper.Maple2.Server.Game/PacketHandlers/EventRewardHandler.cs (1)
32-54: Missing default case logs / safeguardThe new switch arms look good, but the
switchstill lacks adefault:branch.
Without it, unsupported command IDs silently do nothing, which is painful when debugging malformed packets.+ default: + Logger.Warn($"Unhandled EventReward command: {(byte)command}"); + return;Maple2.Model/Metadata/ServerTable/GameEventTable.cs (2)
165-182: Consider validating Bingo array dimensions
int[][] Numbersleaves the board size implicit.
Downstream code will need to assume a fixed 5×5 grid (or similar). If any inner array is shorter, runtime errors ensue. Recommend adding a validation helper or switching toIReadOnlyList<int[/*5*/]>.
241-245: Attribute list is growing—watch the polymorphism limit
System.Text.Jsoncurrently allows up to 64 derived types; you are at 29+. Keep track to avoid hitting the runtime limit in future expansions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
Maple2.File.Ingest/Mapper/ServerTableMapper.cs(1 hunks)Maple2.Model/Enum/GameEventUserValueType.cs(1 hunks)Maple2.Model/Game/Event/GameEvent.cs(2 hunks)Maple2.Model/Metadata/ServerTable/GameEventTable.cs(2 hunks)Maple2.Server.Game/Manager/NpcScriptManager.cs(1 hunks)Maple2.Server.Game/PacketHandlers/EventRewardHandler.cs(2 hunks)Maple2.Server.Game/PacketHandlers/ItemUseHandler.cs(2 hunks)Maple2.Server.Game/Packets/EventRewardPacket.cs(2 hunks)Maple2.Server.Game/Packets/ItemUsePacket.cs(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
Maple2.Model/Metadata/ServerTable/GameEventTable.cs (1)
Maple2.File.Ingest/Mapper/ServerTableMapper.cs (1)
GameEventData(829-1277)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: format
🔇 Additional comments (10)
Maple2.Server.Game/Packets/ItemUsePacket.cs (2)
13-14: LGTM! The QuestScroll command is properly added to the Command enum.The new command follows the existing pattern and numbering scheme.
53-58: LGTM! The QuestScroll packet creation method is well-implemented.The implementation correctly follows the established pattern of other packet creation methods in this class.
Maple2.Server.Game/PacketHandlers/ItemUseHandler.cs (1)
123-125: LGTM! QuestScroll case properly added to the item function switch statement.The case is appropriately placed in the switch statement and correctly calls the new handler method.
Maple2.Server.Game/Manager/NpcScriptManager.cs (3)
369-373: LGTM! Improved packet ordering for NPC interactionsThe added comment clearly explains the reasoning for the packet ordering, and the code correctly sends the movement packet first.
374-376: LGTM! Moved OpenDialog packet to a more appropriate positionThe OpenDialog packet is now sent after the MovePlayer packet but before any portal movement, which aligns with the requirement that NpcTalk packets be sent first.
382-384: LGTM! Portal existence check improves reliabilityThe conditional sending of the MoveByPortal packet now correctly checks if the portal exists in the current field, preventing potential errors.
Maple2.Model/Game/Event/GameEvent.cs (1)
207-218: Potentially missing Bingo board numbers in serialization
BingoEventcurrently serializes reward data and pencil item IDs, but it never writes theNumbersgrid that was parsed intoBingoEvent.Numbers.
If clients reconstruct the board from the packet, omitting this information will prevent them from rendering the initial state.Please double-check the protocol spec: if the board’s numbers are expected on the wire, they must be written here (most likely as a 2-D array preceded by row/col counts).
Maple2.Server.Game/Packets/EventRewardPacket.cs (1)
10-18: Enum values may collide with future additionsNew enum members are inserted with hard-coded byte values (1, 3, 4, 20, 21, 23).
Please confirm these IDs are unique in the full protocol enum (including values defined elsewhere).
Consider adding a comment that references the official opcode table to avoid accidental reuse.Maple2.Server.Game/PacketHandlers/EventRewardHandler.cs (1)
19-27: Verify enum values do not collide with legacy commands
ClaimTimeRunFinalReward = 3andClaimTimeRunStepReward = 4are inserted into the existing enum.
Please double-check that values 3 and 4 are unused by earlier client builds; if they overlap, packets will be mis-decoded.Would you run a quick grep on
EventRewardPacket(and the client opcode table, if available) to confirm that0x03and0x04were previously unassigned?Maple2.Model/Metadata/ServerTable/GameEventTable.cs (1)
183-200:IReadOnlyDictionary<int, RewardItem>serialises with string keys
System.Text.Jsonwrites dictionary keys as strings; on deserialisation they must match exactly. If any tooling emits"1"instead of1, you’re fine, but be aware when editing data by hand.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
Maple2.Server.Game/PacketHandlers/EventRewardHandler.cs (4)
80-98:⚠️ Potential issueTimeRun final reward can be claimed multiple times
The final reward implementation doesn't prevent a player from claiming the reward repeatedly.
Apply this diff to prevent multiple claims:
private void HandleClaimTimeRunFinalReward(GameSession session) { GameEvent? gameEvent = session.FindEvent(GameEventType.TimeRunEvent).FirstOrDefault(); if (gameEvent?.Metadata.Data is not TimeRunEvent timeRunEvent) { return; } + // Check if final reward was already claimed + GameEventUserValue claimed = session.GameEvent.Get( + GameEventUserValueType.TimeRunFinalRewardClaimed, gameEvent.Id, gameEvent.EndTime); + if (claimed.Int() > 0) { + return; // Already claimed + } RewardItem rewardItem = timeRunEvent.FinalReward; Item? item = session.Field.ItemDrop.CreateItem(rewardItem.ItemId, rewardItem.Rarity, rewardItem.Amount); if (item == null) { return; } if (!session.Item.Inventory.Add(item, true)) { session.Item.MailItem(item); } session.Send(EventRewardPacket.ClaimTimeRunFinalReward(rewardItem)); + + // Record that the final reward has been claimed + session.GameEvent.Set(gameEvent.Id, GameEventUserValueType.TimeRunFinalRewardClaimed, 1); }Note: You'll need to add
TimeRunFinalRewardClaimedto theGameEventUserValueTypeenum.
200-240:⚠️ Potential issueBingo number crossing issues
There are several issues with this implementation:
- Line 222: Whitespace formatting error as indicated by pipeline failures
- Line 235:
checkedNumbers.Valueis used directly after adding to the list but before updating the string representation- No concurrency protection when updating the checkedNumbers
Apply these fixes:
private void HandleCrossOffBingoNumber(GameSession session, IByteReader packet) { GameEvent? gameEvent = session.FindEvent(GameEventType.BingoEvent).FirstOrDefault(); if (gameEvent?.Metadata.Data is not BingoEvent bingoEvent) { return; } int itemId = packet.ReadInt(); int bingoNumber = packet.ReadInt(); // Check if the item is a pencil if (itemId != bingoEvent.PencilItemId && itemId != bingoEvent.PencilPlusItemId) { return; } GameEventUserValue checkedNumbers = session.GameEvent.Get(GameEventUserValueType.BingoNumbersChecked, gameEvent.Id, gameEvent.EndTime); List<int> checkedNumbersList = string.IsNullOrEmpty(checkedNumbers.Value) ? [] : checkedNumbers.Value.Split(',').Select(int.Parse).ToList(); if (checkedNumbersList.Contains(bingoNumber)) { return; } if (itemId == bingoEvent.PencilItemId) { DateTime startTime = gameEvent.StartTime.FromEpochSeconds(); - int days = Math.Max(0, (int)(DateTime.UtcNow - startTime).TotalDays); + int days = Math.Max(0, (int) (DateTime.UtcNow - startTime).TotalDays); if (!bingoEvent.Numbers[days].Contains(bingoNumber)) { return; } } Item? item = session.Item.Inventory.Find(itemId).FirstOrDefault(); if (item == null) { return; } + // Lock to prevent concurrent modifications + lock (session) { checkedNumbersList.Add(bingoNumber); session.Item.Inventory.Consume(item.Uid, 1); - session.GameEvent.Set(gameEvent.Id, GameEventUserValueType.BingoNumbersChecked, checkedNumbers.Value); + session.GameEvent.Set(gameEvent.Id, GameEventUserValueType.BingoNumbersChecked, string.Join(',', checkedNumbersList)); + } GameEventUserValue rewardsClaimed = session.GameEvent.Get(GameEventUserValueType.BingoRewardsClaimed, gameEvent.Id, gameEvent.EndTime); session.Send(EventRewardPacket.UpdateBingo(checkedNumbers.Value, rewardsClaimed.Value)); }🧰 Tools
🪛 GitHub Actions: Format
[error] 222-222: Whitespace formatting error: Fix whitespace formatting. Insert '\s'.
242-275:⚠️ Potential issueBingo reward claim lacks validation
The implementation doesn't verify that the player has actually completed the required number of bingo lines before claiming rewards.
Apply these changes to add validation and simplify response packets:
private void HandleClaimBingoReward(GameSession session, IByteReader packet) { int index = packet.ReadInt(); GameEvent? gameEvent = session.FindEvent(GameEventType.BingoEvent).FirstOrDefault(); if (gameEvent?.Metadata.Data is not BingoEvent bingoEvent) { return; } if (index >= bingoEvent.Rewards.Length) { return; } GameEventUserValue rewardsClaimed = session.GameEvent.Get(GameEventUserValueType.BingoRewardsClaimed, gameEvent.Id, gameEvent.EndTime); List<int> rewardsClaimedList = string.IsNullOrEmpty(rewardsClaimed.Value) ? [] : rewardsClaimed.Value.Split(',').Select(int.Parse).ToList(); if (rewardsClaimedList.Contains(index)) { return; } + + // Validate that the player has completed enough bingo lines + GameEventUserValue checkedNumbers = session.GameEvent.Get(GameEventUserValueType.BingoNumbersChecked, gameEvent.Id, gameEvent.EndTime); + List<int> checkedNumbersList = string.IsNullOrEmpty(checkedNumbers.Value) ? [] : checkedNumbers.Value.Split(',').Select(int.Parse).ToList(); + + // TODO: Add validation logic here to verify the player has completed at least 'index+1' bingo lines + // int completedLines = CalculateCompletedBingoLines(checkedNumbersList); + // if (completedLines < index + 1) { + // return; // Not enough lines completed for this reward + // } rewardsClaimedList.Add(index); session.GameEvent.Set(gameEvent.Id, GameEventUserValueType.BingoRewardsClaimed, string.Join(',', rewardsClaimedList)); - GameEventUserValue checkedNumbers = session.GameEvent.Get(GameEventUserValueType.BingoNumbersChecked, gameEvent.Id, gameEvent.EndTime); foreach (RewardItem rewardItem in bingoEvent.Rewards[index].Items) { Item? item = session.Field.ItemDrop.CreateItem(rewardItem.ItemId, rewardItem.Rarity, rewardItem.Amount); if (item == null) { continue; } if (!session.Item.Inventory.Add(item, true)) { session.Item.MailItem(item); } } - session.Send(EventRewardPacket.ClaimBingoReward(bingoEvent.Rewards[index].Items)); - session.Send(EventRewardPacket.UpdateBingo(checkedNumbers.Value, rewardsClaimed.Value)); + + // Send a combined update to reduce packet overhead + session.Send(EventRewardPacket.ClaimBingoReward(bingoEvent.Rewards[index].Items)); + session.Send(EventRewardPacket.UpdateBingo(checkedNumbers.Value, string.Join(',', rewardsClaimedList))); }You need to implement a
CalculateCompletedBingoLinesfunction that counts how many bingo lines (horizontal, vertical, diagonal) the player has completed based on their checked numbers.
56-78:⚠️ Potential issueTimeRun step reward lacks validation and persistence
The implementation has two critical issues:
- There's no validation that the player has actually traveled the specified distance
- There's no mechanism to prevent claiming the same step reward multiple times
Apply this diff to fix the issues:
private void HandleClaimTimeRunStepReward(GameSession session, IByteReader packet) { int steps = packet.ReadInt(); GameEvent? gameEvent = session.FindEvent(GameEventType.TimeRunEvent).FirstOrDefault(); if (gameEvent?.Metadata.Data is not TimeRunEvent timeRunEvent) { return; } + // Check if this step reward was already claimed + GameEventUserValue claimed = session.GameEvent.Get( + GameEventUserValueType.TimeRunStepRewardsClaimed, gameEvent.Id, gameEvent.EndTime); + List<int> claimedSteps = string.IsNullOrEmpty(claimed.Value) + ? [] + : claimed.Value.Split(',').Select(int.Parse).ToList(); + + if (claimedSteps.Contains(steps)) { + return; // Already claimed this step reward + } if (!timeRunEvent.StepRewards.TryGetValue(steps, out RewardItem rewardItem)) { return; } + // TODO: Add validation that player has actually traveled this distance + // if (!session.TimeRun.HasReachedDistance(steps)) { + // return; + // } Item? item = session.Field.ItemDrop.CreateItem(rewardItem.ItemId, rewardItem.Rarity, rewardItem.Amount); if (item == null) { return; } if (!session.Item.Inventory.Add(item, true)) { session.Item.MailItem(item); } + // Record that this step reward has been claimed + claimedSteps.Add(steps); + session.GameEvent.Set(gameEvent.Id, + GameEventUserValueType.TimeRunStepRewardsClaimed, + string.Join(",", claimedSteps)); session.Send(EventRewardPacket.ClaimTimeRunStepReward(rewardItem)); }Note: You'll need to add
TimeRunStepRewardsClaimedto theGameEventUserValueTypeenum.
🧹 Nitpick comments (2)
Maple2.Server.Game/PacketHandlers/EventRewardHandler.cs (2)
182-182: Fix whitespace formatting error as reported by pipeline- int days = Math.Max(0, (int)(DateTime.UtcNow - startTime).TotalDays); + int days = Math.Max(0, (int) (DateTime.UtcNow - startTime).TotalDays);🧰 Tools
🪛 GitHub Actions: Format
[error] 182-182: Whitespace formatting error: Fix whitespace formatting. Insert '\s'.
222-222: Fix whitespace formatting error as reported by pipeline- int days = Math.Max(0, (int)(DateTime.UtcNow - startTime).TotalDays); + int days = Math.Max(0, (int) (DateTime.UtcNow - startTime).TotalDays);🧰 Tools
🪛 GitHub Actions: Format
[error] 222-222: Whitespace formatting error: Fix whitespace formatting. Insert '\s'.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Maple2.Database/Model/GameEventUserValue.cs(1 hunks)Maple2.Model/Game/User/GameEventUserValue.cs(2 hunks)Maple2.Server.Game/Manager/GameEventManager.cs(2 hunks)Maple2.Server.Game/PacketHandlers/EventRewardHandler.cs(2 hunks)Maple2.Server.Game/PacketHandlers/ItemUseHandler.cs(2 hunks)Maple2.Server.Game/Packets/GameEventUserValuePacket.cs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Maple2.Server.Game/PacketHandlers/ItemUseHandler.cs
🧰 Additional context used
🧬 Code Graph Analysis (1)
Maple2.Server.Game/Manager/GameEventManager.cs (1)
Maple2.Model/Game/User/GameEventUserValue.cs (2)
SetValue(24-26)Long(30-30)
🪛 GitHub Actions: Format
Maple2.Server.Game/PacketHandlers/EventRewardHandler.cs
[error] 182-182: Whitespace formatting error: Fix whitespace formatting. Insert '\s'.
[error] 222-222: Whitespace formatting error: Fix whitespace formatting. Insert '\s'.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (6)
Maple2.Database/Model/GameEventUserValue.cs (1)
27-27: Good refactoring to match constructor usageThis change properly adapts to the updated
GameEventUserValueclass in the Model layer, which now controls theValueproperty through a constructor rather than direct property assignment.Maple2.Server.Game/Manager/GameEventManager.cs (2)
133-134: Good adaptation to the new SetValue methodThis change correctly uses the new
SetValuemethod instead of direct property assignment, respecting the encapsulation introduced in theGameEventUserValueclass.
166-166: Good adaptation to the new SetValue methodThis change correctly uses the new
SetValuemethod instead of direct property assignment, maintaining consistency with the encapsulation pattern.Maple2.Model/Game/User/GameEventUserValue.cs (3)
9-9: Good encapsulation of the Value propertyConverting the public field to a property with a private setter improves encapsulation and helps prevent unexpected modifications.
13-15: Good constructor refactoringThis constructor properly initializes the Value property and provides a simpler way to create instances with an initial value.
24-26: Good addition of the SetValue methodThe SetValue method provides controlled access to modify the Value property, maintaining encapsulation while allowing necessary updates.
2 participants
Summary by CodeRabbit
New Features
Enhancements
Other Changes