Dungeon Creation Adjustments, Dungeon Rewards WIP, and Mini Game Rewards - #375
Conversation
WalkthroughThis pull request introduces a broad range of enhancements across the system. The changes extend the player information logic by incorporating guild details and update metadata storage with lazy-loaded tables for prestige and reward content. New parsing methods and enhanced error handling were added, along with numerous updates to dungeon and mini-game models and enums. The RPC and packet definitions now support field requests for creating and destroying dungeons, and dungeon management logic has been restructured throughout the server. Party, experience, and trigger systems have also been refined to accommodate the enhanced dungeon, field, and guild functionalities. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ChannelService
participant FieldManagerFactory as FieldManager.Factory
participant DungeonManager
Client->>ChannelService: Field(FieldRequest with CreateDungeon)
ChannelService->>ChannelService: Validate request type
ChannelService->>FieldManagerFactory: CreateDungeon(dungeonMetadata, ownerId, size, partyId)
FieldManagerFactory-->>ChannelService: Return DungeonFieldManager or error
ChannelService->>DungeonManager: Initialize dungeon lobby & set properties
DungeonManager->>ChannelService: Return roomId in FieldResponse
ChannelService->>Client: FieldResponse (roomId, error code)
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 7
🧹 Nitpick comments (17)
Maple2.Model/Enum/FieldType.cs (1)
4-6: Consider adding comments for all enum values for consistency.While
Randomhas a helpful explanatory comment, consider adding similar comments forDefaultandDungeonvalues to maintain consistency and improve clarity for developers. Additionally, XML documentation for the enum itself would further enhance code documentation.public enum FieldType : byte { - Default = 0, + Default = 0, // Standard field type Random = 1, // Random Rooms like Pocket Realms - Dungeon = 2 + Dungeon = 2 // Dungeon instance fields }Maple2.Server.Game/Packets/RoomStageDungeonPacket.cs (2)
16-17: Consider documenting the purpose of the empty parameter writes.The
WriteInt()andWriteShort()methods are called without parameters, which differs from most other packet writers in the codebase. If this is intentional to write default values (0), consider adding a comment explaining why these fields are empty or what they represent in the packet structure.pWriter.WriteInt(dungeonId); - pWriter.WriteInt(); - pWriter.WriteShort(); + pWriter.WriteInt(); // Reserved field or unused parameter + pWriter.WriteShort(); // Reserved field or unused parameter
12-20: Consider adding method documentation.It would be helpful to add a documentation comment that describes what this method does, what the
dungeonIdparameter represents, and when this packet is expected to be sent.+ /// <summary> + /// Creates a packet to set the dungeon stage room information. + /// </summary> + /// <param name="dungeonId">The ID of the dungeon to set.</param> + /// <returns>A ByteWriter containing the packet data.</returns> public static ByteWriter Set (int dungeonId) { var pWriter = Packet.Of(SendOp.RoomStageDungeon); pWriter.Write<Command>(Command.Set); pWriter.WriteInt(dungeonId); pWriter.WriteInt(); pWriter.WriteShort(); return pWriter; }Maple2.Server.Game/Model/Field/Widget/OxQuizWidget.cs (1)
54-54: Enhancement: Added debug formatting to quiz answers.The change adds a newline and "[DebugMode]" prefix to quiz answers in development mode, making it more visually clear when debug information is being displayed during testing.
Consider using string interpolation for better readability:
-answer = question.IsTrue ? "\n[DebugMode] TRUE" : "\n[DebugMode] FALSE"; +answer = $"\n[DebugMode] {(question.IsTrue ? "TRUE" : "FALSE")}";Maple2.Server.Game/Manager/Items/ItemDropManager.cs (1)
105-105: Using new C# 12 collection expression syntaxGood use of the modern C# 12 collection expression syntax to create an array literal
[entry]instead of explicitly creating a new List. However, theGetAllGroupsmethod signature on line 139 expects aList<IndividualDropItemTable.Entry>, which could potentially cause issues if the method internally uses List-specific operations.Consider updating the
GetAllGroupsmethod signature to accept a more generic interface likeIReadOnlyCollection<IndividualDropItemTable.Entry>orIEnumerable<IndividualDropItemTable.Entry>if it doesn't need to modify the collection:-private IEnumerable<Item> GetAllGroups(GameSession session, int level, List<IndividualDropItemTable.Entry> entry, int index = -1) { +private IEnumerable<Item> GetAllGroups(GameSession session, int level, IReadOnlyCollection<IndividualDropItemTable.Entry> entry, int index = -1) {Maple2.Server.Game/Packets/DungeonWaitingPacket.cs (1)
1-15:Details
❓ Verification inconclusive
Well-structured packet implementation
The
DungeonWaitingPacketclass follows the established pattern in the codebase for creating packets. The implementation is clean and focused on a single responsibility - creating a packet for dungeon waiting information.One observation: Unlike other packet methods in the codebase (such as those in
DungeonRoomPacket), this implementation doesn't include a Command enum after the operation code. If this is intentional based on the protocol specification, then it's fine. Otherwise, consider adding the appropriate command enum for consistency with other packet implementations.
🏁 Script executed:
#!/bin/bash # Check if other simple packet classes follow similar pattern without Command enum rg -A 10 "public static ByteWriter Set" --type cs | grep -v "Write<Command>"Length of output: 14027
Action Required: Verify Consistency with Protocol Specification
After reviewing similar packet implementations in the codebase, the grep output indicates that many simple packet methods (including those in the
RoomStageDungeonPacketand others) follow a pattern that does not include a command enum following the operation code. However, note that the original comment mentionedDungeonRoomPacket, which doesn’t seem to appear in our current repository context. Please confirm whether the omission of a Command enum inDungeonWaitingPacketis intentional per the protocol specification.
- Verify if the protocol requires a Command enum that is missing.
- Double-check if
DungeonRoomPacketis still in use or has been renamed/replaced.If the omission is by design, no changes are needed. Otherwise, consider adding the appropriate Command enum for consistency.
Maple2.Server.World/Service/WorldService.Migrate.cs (1)
114-132: Consider refactoring dungeon operation methods to reduce duplication.Both
CreateDungeonandDestroyDungeonmethods contain nearly identical code for retrieving a client and forwarding the request. This could be refactored into a common helper method to improve maintainability.- private FieldResponse CreateDungeon(FieldRequest request) { - if (!channelClients.TryGetClient(channelClients.FirstChannel(), out Channel.Service.Channel.ChannelClient? client)) { - return new FieldResponse { - Error = (int) MigrationError.s_move_err_no_server, - }; - } - - return client.Field(request); - } - - private FieldResponse DestroyDungeon(FieldRequest request) { - if (!channelClients.TryGetClient(channelClients.FirstChannel(), out Channel.Service.Channel.ChannelClient? client)) { - return new FieldResponse { - Error = (int) MigrationError.s_move_err_no_server, - }; - } - - return client.Field(request); - } + private FieldResponse ProcessFieldRequest(FieldRequest request) { + if (!channelClients.TryGetClient(channelClients.FirstChannel(), out Channel.Service.Channel.ChannelClient? client)) { + return new FieldResponse { + Error = (int) MigrationError.s_move_err_no_server, + }; + } + + return client.Field(request); + } + + private FieldResponse CreateDungeon(FieldRequest request) { + return ProcessFieldRequest(request); + } + + private FieldResponse DestroyDungeon(FieldRequest request) { + return ProcessFieldRequest(request); + }Additionally, consider whether using the first channel is always appropriate for dungeon operations, or if there are scenarios where a specific channel should be selected.
Maple2.Server.Game/Service/ChannelService.Field.cs (3)
9-18: Return a more descriptive error for unknown FieldRequest types.
Currently, the default case returns a blankFieldResponse. Consider returning an explicit error code or logging a warning to aid in troubleshooting unrecognized request types.
20-38: Consider logging successful dungeon creation.
After successfully creating a dungeon, generating a log entry or metric can help diagnose creation flow or track usage.
40-47: Add optional log message for destroyed dungeons.
Returning theMigrationErroris correct. Including a debug log for successful or failed destruction could aid visibility into dungeon lifecycle operations.Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (1)
280-280: Consider repetitive disposal attempts
CallingDisposeDungeon(dungeonField)each iteration when players remain could lead to frequent logs or overhead. You may wish to add a cooldown or check to limit repeated attempts.Maple2.Server.Game/Manager/DungeonManager.cs (4)
28-31: Use consistent encapsulation for new fields
Lines 28, 30, 31 introduce new public fields/properties (Lobby,DungeonRoomRecord,UserRecord). Where possible, prefer private fields with public getters/setters or strongly consider using auto-properties to better control where the values can be mutated. This helps prevent unintentional modifications from external classes.
64-78: Clarify method naming or expand logic
The naming ofLoad()andLoadField()is slightly ambiguous, asLoadField()only sends a packet if it’s a dungeon lobby. Consider renaming them (e.g.,InitializeDungeons()andCheckDungeonLobby()) or consolidating the logic if they are typically invoked together for initialization.
320-354: Ensure partition-based concurrency on dungeon completions
When multiple users finish at nearly the same time, concurrency might occur while awarding rewards or adding items. Ensure that awarding items and adding currency is thread-safe or that only one user triggers “completion” at a time. Otherwise, you risk race conditions or double awarding.
356-390: Check concurrency on dungeon reset
Similar to dungeon completion, resets triggered by multiple commands in quick succession could lead to concurrency issues (e.g., multiple attempts to destroy or reconfigure the same dungeon room). You may wish to guard against double resets or stale state to prevent undesired outcomes.Maple2.Server.Game/Trigger/TriggerContext.MiniGame.cs (2)
14-40: Allow custom logic upon mini-game end
TheEndMiniGamemethod simply sends rewards (if applicable) and sets records to null. If you plan further expansions (e.g., a summary UI, stats tracking, logs, or secondary achievements), consider introducing event hooks or extension points here.
89-99: Check for external constraints during EXP awarding
MiniGameGiveExpadds mini-game EXP to every player in the box, ignoring potential level caps or EXP-block scenarios. If your game design includes maximum level or special conditions, ensure there are checks to avoid awarding unintended EXP.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (58)
Maple2.Database/Storage/Game/GameStorage.User.cs(1 hunks)Maple2.Database/Storage/Game/GameStorage.cs(2 hunks)Maple2.Database/Storage/Metadata/ServerTableMetadataStorage.cs(3 hunks)Maple2.Database/Storage/Metadata/TableMetadataStorage.cs(3 hunks)Maple2.File.Ingest/Mapper/ServerTableMapper.cs(3 hunks)Maple2.File.Ingest/Mapper/TableMapper.cs(2 hunks)Maple2.File.Ingest/Utils/PyParameter.cs(1 hunks)Maple2.File.Ingest/Utils/TriggerDefinitionOverride.cs(1 hunks)Maple2.Model/Enum/Dungeon.cs(1 hunks)Maple2.Model/Enum/FieldType.cs(1 hunks)Maple2.Model/Enum/InstanceType.cs(1 hunks)Maple2.Model/Error/DungeonRoomError.cs(1 hunks)Maple2.Model/Game/Dungeon/DungeonRoomRecord.cs(1 hunks)Maple2.Model/Game/Dungeon/DungeonUserRecord.cs(1 hunks)Maple2.Model/Game/Dungeon/DungeonUserResult.cs(1 hunks)Maple2.Model/Game/Dungeon/IUserContentRecord.cs(1 hunks)Maple2.Model/Game/Dungeon/MiniGameUserRecord.cs(1 hunks)Maple2.Model/Game/FieldInstance.cs(1 hunks)Maple2.Model/Game/RewardItem.cs(2 hunks)Maple2.Model/Game/User/PlotInfo.cs(0 hunks)Maple2.Model/Metadata/Constants.cs(2 hunks)Maple2.Model/Metadata/ServerTable/PrestigeIdExpTable.cs(1 hunks)Maple2.Model/Metadata/ServerTableMetadata.cs(1 hunks)Maple2.Model/Metadata/Table/RewardContentTable.cs(1 hunks)Maple2.Model/Metadata/TableMetadata.cs(1 hunks)Maple2.Server.Core/proto/channel/channel.proto(1 hunks)Maple2.Server.Core/proto/common.proto(1 hunks)Maple2.Server.Core/proto/world/world.proto(1 hunks)Maple2.Server.Game/GameServer.cs(2 hunks)Maple2.Server.Game/Manager/DungeonManager.cs(6 hunks)Maple2.Server.Game/Manager/ExperienceManager.cs(3 hunks)Maple2.Server.Game/Manager/Field/FieldManager/DungeonFieldManager.cs(1 hunks)Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs(7 hunks)Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.cs(4 hunks)Maple2.Server.Game/Manager/Field/FieldManager/IField.cs(2 hunks)Maple2.Server.Game/Manager/Items/ItemDropManager.cs(1 hunks)Maple2.Server.Game/Manager/PartyManager.cs(4 hunks)Maple2.Server.Game/Model/Field/Widget/OxQuizWidget.cs(1 hunks)Maple2.Server.Game/PacketHandlers/DungeonRoomHandler.cs(1 hunks)Maple2.Server.Game/Packets/DungeonRewardPacket.cs(1 hunks)Maple2.Server.Game/Packets/DungeonRoomPacket.cs(2 hunks)Maple2.Server.Game/Packets/DungeonWaitingPacket.cs(1 hunks)Maple2.Server.Game/Packets/FieldEnterPacket.cs(2 hunks)Maple2.Server.Game/Packets/PartyPacket.cs(2 hunks)Maple2.Server.Game/Packets/RoomStageDungeonPacket.cs(1 hunks)Maple2.Server.Game/Scripting/Scripts(1 hunks)Maple2.Server.Game/Service/ChannelService.Field.cs(1 hunks)Maple2.Server.Game/Service/ChannelService.cs(1 hunks)Maple2.Server.Game/Session/GameSession.State.cs(2 hunks)Maple2.Server.Game/Session/GameSession.cs(6 hunks)Maple2.Server.Game/Trigger/TriggerContext.Dungeon.cs(3 hunks)Maple2.Server.Game/Trigger/TriggerContext.Interface.cs(1 hunks)Maple2.Server.Game/Trigger/TriggerContext.MiniGame.cs(1 hunks)Maple2.Server.Game/Util/Sync/PlayerInfoStorage.cs(1 hunks)Maple2.Server.World/Containers/PartyManager.cs(5 hunks)Maple2.Server.World/Service/WorldService.Migrate.cs(2 hunks)Maple2.Server.World/Service/WorldService.Party.cs(2 hunks)Maple2.Server.World/Service/WorldService.Sync.cs(1 hunks)
💤 Files with no reviewable changes (1)
- Maple2.Model/Game/User/PlotInfo.cs
🧰 Additional context used
🧬 Code Definitions (27)
Maple2.Model/Metadata/ServerTableMetadata.cs (1)
Maple2.File.Ingest/Mapper/ServerTableMapper.cs (1) (1)
PrestigeIdExpTable(646-656)
Maple2.Server.Game/PacketHandlers/DungeonRoomHandler.cs (2)
Maple2.Server.Game/Manager/DungeonManager.cs (1) (1)
Reset(356-390)Maple2.Server.Game/Manager/FishingManager.cs (1) (1)
Reset(39-52)
Maple2.Model/Game/RewardItem.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1) (1)
RewardRecord(524-579)
Maple2.Server.Game/Packets/RoomStageDungeonPacket.cs (2)
Maple2.Server.Game/Packets/DungeonWaitingPacket.cs (1) (1)
ByteWriter(8-14)Maple2.Server.Game/Packets/DungeonRoomPacket.cs (6) (6)
ByteWriter(21-31)ByteWriter(33-39)ByteWriter(41-48)ByteWriter(50-63)ByteWriter(65-72)ByteWriter(74-84)
Maple2.Server.Game/Packets/FieldEnterPacket.cs (1)
Maple2.Model/Game/FieldInstance.cs (2) (2)
FieldInstance(7-22)FieldInstance(13-16)
Maple2.Server.Game/Service/ChannelService.cs (5)
Maple2.Database/Storage/Metadata/TableMetadataStorage.cs (2) (2)
TableMetadataStorage(7-208)TableMetadataStorage(130-188)Maple2.Database/Storage/Metadata/ServerTableMetadataStorage.cs (2) (2)
ServerTableMetadataStorage(9-103)ServerTableMetadataStorage(52-73)Maple2.Server.Game/Service/ChannelService.Field.cs (1) (1)
ChannelService(8-47)Maple2.Database/Storage/Game/GameStorage.User.cs (1) (1)
GameStorage(23-529)Maple2.Database/Storage/Game/GameStorage.cs (2) (2)
GameStorage(10-71)GameStorage(21-32)
Maple2.Database/Storage/Metadata/TableMetadataStorage.cs (2)
Maple2.Database/Storage/Metadata/ServerTableMetadataStorage.cs (1) (1)
Lazy(85-102)Maple2.File.Ingest/Mapper/TableMapper.cs (1) (1)
RewardContentTable(1634-1686)
Maple2.Database/Storage/Game/GameStorage.cs (1)
Maple2.Database/Storage/Game/GameStorage.User.cs (4) (4)
PlayerInfo(100-140)Character(74-92)Character(455-465)IList(312-358)
Maple2.Server.Game/GameServer.cs (1)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (3) (3)
DungeonFieldManager(146-201)MigrationError(203-209)MigrationError(292-308)
Maple2.Model/Metadata/ServerTable/PrestigeIdExpTable.cs (2)
Maple2.File.Ingest/Mapper/ServerTableMapper.cs (2) (2)
PrestigeIdExpTable(646-656)ExpType(658-711)Maple2.File.Ingest/Mapper/TableMapper.cs (1) (1)
ExpType(1235-1240)
Maple2.Server.Game/Service/ChannelService.Field.cs (4)
Maple2.Server.Game/Service/ChannelService.cs (2) (2)
ChannelService(7-23)ChannelService(16-22)Maple2.Server.Game/GameServer.cs (2) (2)
MigrationError(109-111)DungeonFieldManager(105-107)Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (3) (3)
MigrationError(203-209)MigrationError(292-308)DungeonFieldManager(146-201)Maple2.Server.Game/Manager/Field/FieldManager/DungeonFieldManager.cs (2) (2)
DungeonFieldManager(11-69)DungeonFieldManager(18-27)
Maple2.Server.Game/Session/GameSession.cs (3)
Maple2.Server.Game/Manager/PartyManager.cs (3) (3)
PartyManager(17-383)PartyManager(28-39)CheckDisband(203-214)Maple2.Server.Game/Manager/Field/FieldManager/DungeonFieldManager.cs (2) (2)
DungeonFieldManager(11-69)DungeonFieldManager(18-27)Maple2.Server.Game/Manager/DungeonManager.cs (1) (1)
LoadField(70-78)
Maple2.Server.Game/Manager/PartyManager.cs (4)
Maple2.Server.Game/Manager/DungeonManager.cs (2) (2)
SetDungeon(207-214)SetDungeon(305-318)Maple2.Model/Game/Party/Party.cs (1) (1)
Party(11-63)Maple2.Server.World/Containers/PartyManager.cs (1) (1)
Disband(241-250)Maple2.Server.Game/Packets/PartyPacket.cs (1) (1)
PartyPacket(13-339)
Maple2.Server.World/Service/WorldService.Migrate.cs (2)
Maple2.Server.World/Containers/ChannelClientLookup.cs (4) (4)
TryGetClient(104-112)FirstChannel(82-88)Channel(25-48)Channel(38-47)Maple2.Server.Game/Service/ChannelService.Field.cs (2) (2)
FieldResponse(20-38)FieldResponse(40-46)
Maple2.Server.World/Service/WorldService.Party.cs (2)
Maple2.Server.World/Containers/PartyManager.cs (12) (12)
Disband(241-250)PartyManager(13-433)PartyManager(19-22)PartyError(94-130)PartyError(132-156)PartyError(158-185)PartyError(187-209)PartyError(211-239)PartyError(269-305)PartyError(307-338)PartyError(372-412)PartyError(414-432)Maple2.Server.World/Containers/PartyLookup.cs (3) (3)
TryGet(36-38)PartyError(51-70)PartyError(72-84)
Maple2.Server.Game/Packets/DungeonRewardPacket.cs (2)
Maple2.Model/Game/Dungeon/DungeonUserRecord.cs (2) (2)
DungeonUserRecord(7-91)DungeonUserRecord(23-35)Maple2.Model/Game/Dungeon/MiniGameUserRecord.cs (2) (2)
MiniGameUserRecord(6-55)MiniGameUserRecord(15-21)
Maple2.Model/Metadata/Table/RewardContentTable.cs (1)
Maple2.File.Ingest/Mapper/TableMapper.cs (3) (3)
RewardContentTable(1634-1686)Dictionary(313-366)Dictionary(1044-1091)
Maple2.Server.Game/Packets/DungeonWaitingPacket.cs (2)
Maple2.Server.Game/Packets/DungeonRoomPacket.cs (6) (6)
ByteWriter(21-31)ByteWriter(33-39)ByteWriter(41-48)ByteWriter(50-63)ByteWriter(65-72)ByteWriter(74-84)Maple2.Server.Game/Packets/RoomStageDungeonPacket.cs (1) (1)
ByteWriter(12-20)
Maple2.File.Ingest/Mapper/TableMapper.cs (4)
Maple2.Model/Metadata/TableMetadata.cs (1) (1)
TableMetadata(6-25)Maple2.File.Ingest/MapperExtensions.cs (3) (3)
Dictionary(15-25)Dictionary(27-37)Dictionary(387-446)Maple2.File.Ingest/Mapper/ServerTableMapper.cs (5) (5)
Dictionary(93-153)Dictionary(155-215)Dictionary(247-304)Dictionary(306-363)Dictionary(1459-1519)Maple2.Model/Game/Item/Item.cs (4) (4)
Item(11-271)Item(52-92)Item(94-119)Item(121-146)
Maple2.Server.Game/Manager/ExperienceManager.cs (3)
Maple2.Model/ModelExtensions.cs (2) (2)
ExpMessageCode(248-261)ExpType(263-276)Maple2.Server.Game/Packets/ExperienceUpPacket.cs (1) (1)
ExperienceUpPacket(8-34)Maple2.Server.Game/Session/GameSession.cs (1) (1)
ConditionUpdate(510-513)
Maple2.Model/Game/Dungeon/MiniGameUserRecord.cs (1)
Maple2.Model/Game/RewardItem.cs (2) (2)
RewardRecord(34-39)RewardRecord(41-46)
Maple2.Server.Game/Packets/PartyPacket.cs (1)
Maple2.Model/Game/Party/Party.cs (1) (1)
Party(11-63)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (3)
Maple2.Server.Game/Manager/Field/FieldManager/DungeonFieldManager.cs (2) (2)
DungeonFieldManager(11-69)DungeonFieldManager(18-27)Maple2.Model/Game/Dungeon/DungeonRoomRecord.cs (2) (2)
DungeonRoomRecord(7-17)DungeonRoomRecord(14-16)Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.cs (2) (2)
Init(109-231)Dispose(584-603)
Maple2.Server.Game/Manager/Field/FieldManager/DungeonFieldManager.cs (5)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.cs (4) (4)
FieldManager(30-604)FieldManager(81-106)TryGetPlayerById(295-305)Broadcast(573-582)Maple2.Model/Game/Dungeon/DungeonRoomRecord.cs (2) (2)
DungeonRoomRecord(7-17)DungeonRoomRecord(14-16)Maple2.Model/Game/Dungeon/DungeonUserResult.cs (1) (1)
DungeonUserResult(13-17)Maple2.Server.Game/Manager/Field/FieldManager/IField.cs (2) (2)
TryGetPlayerById(80-80)Broadcast(68-68)Maple2.Server.Game/Packets/DungeonRoomPacket.cs (1) (1)
DungeonRoomPacket(11-85)
Maple2.Server.Game/Manager/DungeonManager.cs (4)
Maple2.Model/Game/Dungeon/DungeonRoomRecord.cs (2) (2)
DungeonRoomRecord(7-17)DungeonRoomRecord(14-16)Maple2.Model/Game/Dungeon/DungeonUserRecord.cs (3) (3)
DungeonUserRecord(7-91)DungeonUserRecord(23-35)Add(37-45)Maple2.Server.Game/Packets/DungeonRoomPacket.cs (1) (1)
DungeonRoomPacket(11-85)Maple2.Server.Game/Packets/DungeonRewardPacket.cs (1) (1)
DungeonRewardPacket(9-32)
Maple2.Server.Game/Trigger/TriggerContext.MiniGame.cs (4)
Maple2.Server.Game/Trigger/TriggerContext.cs (2) (2)
TriggerContext(15-232)TriggerContext(31-39)Maple2.Server.Game/Packets/DungeonRewardPacket.cs (1) (1)
DungeonRewardPacket(9-32)Maple2.Model/Game/Dungeon/MiniGameUserRecord.cs (3) (3)
MiniGameUserRecord(6-55)MiniGameUserRecord(15-21)Add(23-33)Maple2.Server.Game/Session/GameSession.cs (1) (1)
RewardRecord(524-579)
Maple2.Model/Game/Dungeon/DungeonUserRecord.cs (2)
Maple2.Model/Game/Dungeon/MiniGameUserRecord.cs (2) (2)
Add(23-33)WriteTo(35-54)Maple2.Model/Game/RewardItem.cs (2) (2)
RewardRecord(34-39)RewardRecord(41-46)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: format
🔇 Additional comments (114)
Maple2.Server.Game/Scripting/Scripts (1)
1-1: Subproject Commit Update VerificationThe updated subproject commit hash (
a5694aea90d5ed1ee4c5f6e9ee06dcf6aab6810d) appears to be the only change in this file. Please verify that this new commit incorporates all the necessary changes for dungeon creation, rewards, and mini-game adjustments as described in the PR objectives. This update should have been tested against the overall integration to ensure that no unintended issues are introduced.Maple2.Model/Metadata/Constants.cs (1)
114-119: Well-structured implementation of Content Rewards constants!The addition of these reward constants is well-aligned with the PR objectives for implementing dungeon and mini-game rewards. The use of a read-only dictionary provides immutable access to these values, and the included comments clearly explain the purpose of each constant.
Maple2.Model/Enum/FieldType.cs (1)
3-7: Looks good! The enum is well-structured for the field type categorization.The enum is correctly defined as a byte type for memory optimization and follows proper C# naming conventions. It provides a clear categorization for different field types which aligns with the PR objective of adjusting dungeon creation processes.
Maple2.File.Ingest/Utils/PyParameter.cs (1)
147-147: Banner type mapping updated correctlyThe change from "BannerType.Fail" to "BannerType.GameOver" for value "5" appears to be a deliberate correction that aligns with the PR objectives of updating mini-game reward triggers. This mapping change will ensure the proper banner type is displayed when appropriate in-game events occur.
Maple2.Server.Game/Packets/RoomStageDungeonPacket.cs (1)
1-21: LGTM on the overall structure of this new packet handler.The implementation follows the packet construction pattern used throughout the project. This new class correctly handles the RoomStageDungeon packet format, which appears to be part of the dungeon management enhancements mentioned in the PR objectives.
Maple2.File.Ingest/Mapper/TableMapper.cs (2)
82-82: Good addition of the reward content mappingThis line properly adds the mapping for reward content XML files, following the same pattern as other similar mappings in the
Map()method.
1634-1686: Well-structured implementation of reward content parsingThe
ParseRewardContentTable()method follows the same pattern as other parsing methods in the class, with clear separation between the different types of reward content data. The method handles base reward content, item rewards, meso rewards (both static and level-based), and experience rewards.I particularly like the use of null-coalescing operators in lines 1667 and 1682 to handle potential null data.
Maple2.Server.Game/Packets/PartyPacket.cs (2)
129-129: Documentation improvementAdding a clear comment to indicate that this boolean value is related to "Party Search" improves code readability and maintenance.
202-207: Enhanced parameter usage and clarityRenaming the parameter from
message2tosystemSoundKeymakes its purpose much clearer. Additionally, you're now correctly using this parameter value in the packet (line 207) instead of an empty string, allowing sound effects to be sent with party notifications. The comment example "Field_Enterance_Reset_Dungeon" provides helpful context for developers.Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.cs (4)
74-76: New properties to support dungeon functionalityThese new properties with
initaccessors enhance theFieldManagerclass to properly support dungeon management.DungeonIdidentifies associated dungeons,Sizelikely controls dungeon capacity, andFieldTypecategorizes the field type. These additions align with the PR objective of implementing dungeon creation adjustments.
105-105: Default initialization for FieldType propertyThe
FieldTypeproperty is correctly initialized toFieldType.Defaultin the constructor, establishing proper default behavior for new field instances.
123-123: Simplified FieldInstance initializationThe constructor call for
FieldInstancehas been simplified by removing theblockChangeChannel: trueparameter. This change supports the PR objective of allowing dungeon creation on the first instance channel.
134-139: Fixed default plot initializationReplaced
Plot.Defaultwith a properly initializedPlotinstance using aUgcMapGroupwith appropriate nested objects. This addresses the PR objective of "fixing the default plot on fields."Maple2.Server.Game/PacketHandlers/DungeonRoomHandler.cs (1)
45-47: Implementation of dungeon reset functionality looks good.The added line now properly delegates the reset functionality to the Dungeon manager, aligning with the PR objectives of allowing dungeon resets. The DungeonManager.Reset() method contains appropriate validation and error handling, including checking if the session has a party and if the current character is the party leader.
Maple2.Server.Game/Session/GameSession.State.cs (2)
2-2: New import added for Dungeon functionality.This import aligns with the PR objectives of implementing dungeon creation adjustments and rewards. The import is needed for the Dungeon-related models and types.
48-48:Details
✅ Verification successful
New MiniGameRecord field added for tracking mini-game state.
This nullable field enables the GameSession to track information about mini-games for the current player session, which aligns with the PR objectives of implementing "mini-game reward triggers". As a nullable reference type, it properly indicates that a player may not always be participating in a mini-game.
Make sure that any code that uses this field properly checks for null before accessing it to avoid null reference exceptions. Also, confirm that this field is properly initialized when a player starts a mini-game and cleared when they exit one.
🏁 Script executed:
#!/bin/bash # Search for places where MiniGameRecord is assigned or accessed rg "MiniGameRecord\s*=" --type cs rg "MiniGameRecord\s*\?" --type cs rg "MiniGameRecord\s*\." --type csLength of output: 2222
Attention: Verified MiniGameRecord Implementation and Usage
The newly added nullable
MiniGameRecordfield inMaple2.Server.Game/Session/GameSession.State.csis used consistently throughout the codebase. The shell script results confirm that:
- The field is explicitly set to
nullwhen appropriate (e.g., inGameSession.cs).- Nullable checks (e.g.,
if (player.Session.MiniGameRecord == null)) are in place inTriggerContext.MiniGame.cs.- The field is correctly initialized—assigning a new
MiniGameUserRecordwhen needed—and subsequently used safely (accessing properties likeShowResult, calling.Add, and manipulatingRewards).There are no further concerns regarding potential null reference exceptions as long as these checks remain consistent. Please continue to ensure that any additional modifications uphold this pattern.
Maple2.File.Ingest/Utils/TriggerDefinitionOverride.cs (1)
264-264:Details
❓ Verification inconclusive
Updated default parameter handling for
dungeon_clearaction.The default value for the
uiTypeparameter has been changed from a string value"None"tonull. This change aligns with the dungeon creation adjustments mentioned in the PR objectives, likely affecting how UI behaves during dungeon clear events.Since this change could affect trigger script behavior, verify that existing scripts using this action continue to work properly.
🏁 Script executed:
#!/bin/bash # Search for any scripts using the dungeon_clear action to ensure they work with the new null default rg -A 2 -B 2 "dungeon_clear" --type xmlLength of output: 188
Below is a revised review comment. Since the initial ripgrep command (using the XML file type filter) did not actually search any files—likely due to an overly restrictive filter—we need manual verification (or a broader search) to ensure that no trigger scripts using the
dungeon_clearaction are impacted by the updated default parameter. You might consider running the following broader command to search the entire repository:#!/bin/bash # Search the entire repository for references to "dungeon_clear" to verify that none of the associated scripts are adversely affected. rg -A 2 -B 2 "dungeon_clear" .Manual Verification Required – Verify
dungeon_clearScript Behavior
- The
uiTypedefault now being set tonull(instead of"None") is consistent with the recent dungeon creation changes.- Because the previous search command (filtered to XML files) returned no results, please ensure that any scripts (regardless of file type) that might be using the
dungeon_clearaction are checked for compatibility with the new default.Once you confirm that the trigger scripts behave as expected, we can consider this aspect of the PR as verified.
Maple2.Model/Metadata/ServerTableMetadata.cs (1)
37-37: Added support for prestige ID experience table serialization.This addition registers the
PrestigeIdExpTableclass with the JSON serialization system, allowing it to be properly deserialized from JSON data with the discriminator "prestigeIdExpTable". This supports the parsing functionality seen inServerTableMapper.ParsePrestigeIdExpTable().Maple2.Model/Error/DungeonRoomError.cs (1)
33-33: Improved error message clarity.The description for
s_room_dungeon_AlreadyLapentaHallhas been updated from a placeholder with a map ID to a more user-friendly message. This improves the player experience by providing a clearer explanation when this error occurs.Maple2.Model/Metadata/TableMetadata.cs (1)
82-82: Added reward content table for JSON serialization.This change registers the
RewardContentTableclass with the JSON serialization system, which supports the dungeon reward system implementation mentioned in the PR objectives. This allows for proper serialization and deserialization of reward content data.Maple2.Server.Game/Manager/Field/FieldManager/IField.cs (2)
23-23: Clean addition of a DungeonId property.The property is well-placed in the interface and aligns with the PR's goal of enhancing dungeon creation and management capabilities.
56-57: Good addition of FieldType and Size properties.These properties will be valuable for distinguishing between different field types (especially dungeons) and defining their dimensions. The use of
initaccessor for Size ensures its immutability after initialization, which is a good practice.Maple2.Database/Storage/Metadata/TableMetadataStorage.cs (3)
48-48: Clean addition of reward content table field.The private field follows the established pattern used throughout this class, using lazy loading for efficient resource management.
109-109: Good implementation of RewardContentTable property.The property provides a clean interface to access the lazy-loaded table value, consistent with other properties in this class.
171-171: Proper initialization of rewardContentTable.The initialization follows the same pattern as other tables, using the
Retrievemethod with the appropriate XML pattern. This ensures consistency with the rest of the codebase.Maple2.Server.Game/Packets/FieldEnterPacket.cs (3)
2-2: Appropriate addition of the FieldType enum namespace.This import is necessary for the changes made to write the field type in the packet.
19-19: Good implementation of field type serialization.The code now writes the field type to the packet, allowing clients to distinguish between different field types (like dungeons, default fields, etc.).
21-21: Proper serialization of dungeon ID.This change ensures the dungeon ID is properly communicated to the client, which is essential for dungeon-related functionality.
Maple2.Model/Game/Dungeon/DungeonRoomRecord.cs (3)
1-6: Well-structured namespace and imports.The imports and namespace are appropriately organized. The
ConcurrentDictionaryis a good choice for managing concurrent access to user results.
7-13: Clean implementation of DungeonRoomRecord properties.The class design is appropriate for tracking dungeon room data:
- Using a concurrent dictionary for thread-safe access to user results
- Read-only metadata reference ensures it can't be changed after initialization
- Start/end tick properties for timing
- State property with a reasonable default value
14-17: Simple and effective constructor.The constructor properly initializes the essential metadata property while leaving other properties to be set elsewhere as needed.
Maple2.Model/Enum/InstanceType.cs (1)
9-17: Enum values reordered for better organization and additional context addedThe enum values have been reordered to keep a consistent numbering scheme, with GuildEvent now at 6, GuildPvp at 7, and DungeonLobby at 8. Comments have been added to indicate which values are not confirmed, providing clarity for future development.
This change supports the dungeon creation adjustments mentioned in the PR objectives by properly defining the DungeonLobby instance type.
Maple2.Model/Metadata/Table/RewardContentTable.cs (1)
1-32: Well-structured data model for reward content implementedThis new file creates a comprehensive data model for storing various types of reward content, including base entries, items, meso, and experience points. The structure uses immutable records with clear property names and appropriate data types.
The implementation aligns perfectly with the PR objectives of "ingesting reward data" and establishing "rudimentary dungeon rewards." The model supports all the necessary data types mentioned in the related reward processing code.
Maple2.Server.Game/Packets/DungeonRewardPacket.cs (1)
9-32: Clean implementation of dungeon and mini-game reward packetsThe new packet definitions follow a consistent pattern for both dungeon and mini-game rewards. The Command enum clearly defines the different packet types, and the methods properly serialize the corresponding user record classes.
This implementation directly supports the PR objectives related to implementing dungeon rewards and mini-game reward triggers.
Maple2.Model/Game/FieldInstance.cs (2)
7-9: Simplified FieldInstance initializationThe static Default instance has been updated to use the new constructor signature that no longer includes the BlockChangeChannel parameter.
13-16: Simplified FieldInstance constructorThe constructor has been streamlined by removing the BlockChangeChannel parameter, simplifying the API and reducing potential confusion.
This change aligns with the PR objectives of adjusting the dungeon creation process, suggesting that blocking channel changes is now handled differently or is no longer needed for the enhanced dungeon functionality.
Maple2.Model/Enum/Dungeon.cs (6)
79-100: Great implementation of DungeonAccumulationRecordType enumThe enum is well-defined with descriptive attributes that will be useful for UI display. The naming is consistent and the values cover a comprehensive range of metrics to track in dungeon gameplay.
102-108: LGTM: DungeonState enum implementationThe enum values are clearly defined with appropriate description attributes for user-facing messages.
110-118: LGTM: DungeonGrade enum implementationGood implementation with a logical progression from F to SPlus grades, with None as a default state.
120-124: LGTM: DungeonRewardType enum implementationSimple and concise definition of reward types that aligns well with the PR objectives for implementing dungeon rewards.
126-136: LGTM: DungeonBonusFlag enum implementationGood use of [Flags] attribute for bit flags that can be combined. The values follow the binary pattern as expected for flags (0, 2, 4, 8, 16, etc.).
138-143: LGTM: DungeonBonusFlag2 enum implementationProperly implemented as flags enum with appropriate naming for the specific bonus flags.
Maple2.Model/Game/Dungeon/MiniGameUserRecord.cs (4)
6-13: Well-structured MiniGameUserRecord class with appropriate propertiesThe class has all the necessary properties to track user records in mini-games, implemented with proper initialization of collections.
15-21: LGTM: Constructor properly initializes rewards dictionaryThe constructor correctly initializes the character ID and populates the Rewards dictionary with all enum values, setting their initial values to 0.
23-33: LGTM: Add method with appropriate null checkThe Add method correctly updates reward values and includes a proper null check before iterating through items.
35-54: LGTM: Complete WriteTo implementationThe WriteTo method correctly serializes all record data to the byte writer in a logical order.
Maple2.Model/Game/Dungeon/DungeonUserRecord.cs (4)
7-22: Well-structured DungeonUserRecord class with appropriate propertiesThe class has a comprehensive set of properties to track user performance and rewards in dungeons. The use of ConcurrentDictionary for AccumulationRecords is a good choice for thread safety.
23-35: LGTM: Constructor properly initializes collectionsThe constructor correctly sets up the necessary dictionaries for accumulation records and rewards.
47-64: LGTM: First part of WriteTo implementationThis section properly serializes the dungeon success state, IDs, and other metadata.
66-90: LGTM: Item and bonus reward serialization in WriteToThese sections correctly serialize the reward items and bonus rewards.
Maple2.Server.Core/proto/world/world.proto (1)
15-16: LGTM: New Field RPC methodThis addition aligns with the PR objective of implementing dungeon creation adjustments. It follows the existing pattern in the file and is well-commented.
Maple2.Server.Core/proto/channel/channel.proto (1)
10-11: Well-implemented RPC addition for field management!The new
FieldRPC method is a clean addition to theChannelservice, following the established pattern of other RPC methods in this service. This aligns with the PR objectives for implementing dungeon creation adjustments.Maple2.Server.Game/Trigger/TriggerContext.Interface.cs (1)
78-80: Good optimization to prevent unnecessary broadcastsThis conditional check prevents broadcasting round events when minimum and maximum round values are identical, which is a sensible optimization.
Maple2.Model/Game/Dungeon/IUserContentRecord.cs (1)
6-10: Well-designed interface for user content recordsThis interface establishes a clean contract for classes that handle dungeon and mini-game rewards, aligning perfectly with the PR objectives. The properties are clearly defined and the extension of
IByteSerializableensures implementations will handle serialization correctly.Maple2.Model/Game/Dungeon/DungeonUserResult.cs (1)
6-18: Well-structured result object for dungeon performanceThe struct is well-designed with appropriate memory layout attributes and a clear separation between immutable and mutable fields. This is perfect for efficiently representing user performance in dungeons and will work well with the reward system being implemented.
Maple2.Server.Game/Util/Sync/PlayerInfoStorage.cs (1)
46-47: Guild information enhancement looks good.The addition of GuildName and GuildId properties to the CharacterInfo object completes the player information by including guild-related data, which aligns with the PR objective of fetching GuildId and GuildName during player info construction.
Maple2.Model/Metadata/ServerTable/PrestigeIdExpTable.cs (1)
5-12: Well-structured record definition for prestige experience data.This new PrestigeIdExpTable record provides a clean structure for storing prestige ID experience mapping data, supporting the dungeon rewards implementation mentioned in the PR objectives. The record pattern is appropriate for this type of immutable metadata.
Maple2.Server.World/Service/WorldService.Sync.cs (1)
39-40: Properly extends PlayerInfoResponse with guild information.The addition of GuildName and GuildId to the PlayerInfoResponse ensures that the guild data is properly propagated from the World service to the Game service, consistent with the earlier changes in PlayerInfoStorage.
Maple2.Server.Game/Packets/DungeonRoomPacket.cs (2)
16-16: New command enum for dungeon results.Adding the DungeonResult command allows for packet communication of dungeon completion results, supporting the "Dungeon Rewards WIP" objective mentioned in the PR.
50-63: Comprehensive packet creation for dungeon results.The implementation follows the established pattern in the class and properly serializes all necessary information for dungeon results, including player statistics and reward information. This supports the dungeon rewards implementation mentioned in the PR objectives.
A few observations:
- The method handles a variable number of player results appropriately
- All required fields (character ID, grade, record type, and value) are included in the packet
- The implementation is consistent with other packet construction methods in the class
Maple2.Server.Game/GameServer.cs (3)
7-7: Added necessary import for error handling.The addition of
Maple2.Model.Errorimport supports the new dungeon management functionality while maintaining clean code organization.
105-107: Well-structured facade method for dungeon creation.This method provides a clean interface to the dungeon creation functionality by delegating to the field factory. Good separation of concerns.
109-111: Well-structured facade method for dungeon destruction.The method provides a clear interface for destroying dungeons while properly returning MigrationError to inform callers about the outcome of the operation.
Maple2.Server.Core/proto/common.proto (2)
5-19: Well-structured protocol definition for field operations.The
FieldRequestmessage with its nested messages provides a clean way to handle different field operations (creating and destroying dungeons) while maintaining a single entry point. Theoneofconstruct ensures type safety and clear request structure.
21-24: Clear response structure for field operations.The
FieldResponseincludes both an error code and room ID, which provides sufficient information for the client to handle the response appropriately.Maple2.Database/Storage/Metadata/ServerTableMetadataStorage.cs (1)
19-19: Added support for new prestige ID experience table.The additions properly follow the established pattern of lazy-loaded metadata tables in the class. This supports the new reward functionality while maintaining consistency with the existing code structure.
Also applies to: 40-40, 62-62
Maple2.Server.Game/Session/GameSession.cs (5)
303-303: Moved PartyManager initialization to EnterField.Moving the PartyManager initialization from EnterServer to EnterField ensures that party-related functionality is only available after the player has successfully entered a field, which can prevent potential issues with party management during the initialization phase.
332-332: Reset MiniGameRecord when leaving field.Adding
MiniGameRecord = nullensures proper cleanup of mini-game state when leaving a field, preventing potential state leakage between different game sessions.
354-363: Improved dungeon field navigation logic.The enhanced logic properly checks if the target map is either a room in the current dungeon or the dungeon lobby, with a fallback to migrate out of the instance if neither condition is met. This provides clearer handling of dungeon navigation.
558-579: Well-implemented item reward handling with appropriate fallback.The implementation properly handles item creation, level restrictions, and inventory management. The fallback to mail items when inventory is full is a good design decision that ensures players receive all rewards.
698-698: Added party cleanup on session disconnect.The call to
Party.CheckDisband()ensures that party resources are properly cleaned up when a player disconnects, preventing orphaned party instances and helping to maintain system integrity.Maple2.Server.Game/Trigger/TriggerContext.Dungeon.cs (5)
8-15: Good defensive coding added to prevent errors.The addition of early return checks ensures that the dungeon clear logic only executes under appropriate conditions. First validating that the field is a dungeon field manager, then checking if the UI should be displayed based on the uiType parameter helps prevent potential errors and improves control flow.
17-17: Improved dungeon state management.Using the
ChangeStatemethod on the dungeon field manager provides a cleaner way to handle state transitions, making the code more maintainable and easier to understand than direct state assignments.
120-120: Helpful debug logging added.Adding debug logging at the beginning of this check provides valuable information for troubleshooting dungeon lobby user count issues.
125-125: Simplified size check logic.The player count check has been simplified to directly compare against the dungeon field's size property, making the code more readable and maintainable.
135-135: Simplified dungeon room check.The
IsDungeonRoommethod has been simplified to use type checking rather than property validation, which is cleaner and more efficient.Maple2.Model/Game/RewardItem.cs (4)
7-7: Good usage of record struct.Converting RewardItem to a record struct provides value-based equality and immutability features, which is ideal for data transfer objects like this reward representation.
23-25: Useful implicit conversion operator.Adding an implicit conversion from Item to RewardItem simplifies code that needs to work with both types, reducing the need for manual conversion and making the API more user-friendly.
28-39: Well-designed RewardRecord structure.The new RewardRecord struct provides a clean way to encapsulate multiple reward items along with currency and experience values, which aligns perfectly with the PR objective of implementing dungeon rewards.
41-46: Convenient constructor overload.This constructor overload that accepts Item objects and converts them to RewardItems demonstrates good API design by allowing flexibility in how rewards are created.
Maple2.Server.Game/Service/ChannelService.cs (2)
11-11: Adding TableMetadataStorage dependency.Adding the TableMetadataStorage field will allow access to dungeon and reward metadata, which is necessary for the dungeon creation adjustments mentioned in the PR objectives.
16-22: Updated constructor with new dependency.The constructor has been properly updated to accept and initialize the new TableMetadataStorage dependency, maintaining proper dependency injection practices.
Maple2.Database/Storage/Game/GameStorage.User.cs (2)
131-136: Added guild information retrieval.The new code effectively retrieves guild information by joining the GuildMember and Guild tables. The use of a Tuple to store both the guild ID and name is a clean approach, and the null coalescing operator handles the case when a player is not in a guild.
139-139: Updated BuildPlayerInfo call with guild data.The BuildPlayerInfo method call now includes guild information as parameters, fulfilling the PR objective of "fetching of GuildId and GuildName during player info construction."
Maple2.Database/Storage/Game/GameStorage.cs (1)
49-68: Guild information is now properly incorporated into PlayerInfo.The method signature has been updated to include guild-related parameters, and these parameters are now used in both branches of the conditional logic to ensure guild information is consistently available in the
PlayerInfoobject.Maple2.Server.Game/Manager/PartyManager.cs (3)
93-95: Properly initializing dungeon properties from PartyInfo.These property initializations ensure that when a party is set, the dungeon-related properties are correctly maintained, supporting the dungeon creation functionality mentioned in the PR objectives.
203-214: Simplified party disbanding logic.The
CheckDisbandmethod has been refactored to always send a disband request to the world service without first checking for online members. This moves the responsibility of checking online members to the world service, centralizing the party-related logic.
299-301: Added notification for dungeon reset.This enhancement provides better user feedback when dungeons are reset by sending a party notice when
Party.DungeonIdis set to zero.Maple2.File.Ingest/Mapper/ServerTableMapper.cs (3)
46-46: Added support for processing adventureIdExpTable.xml.This addition enables the system to parse and utilize data from the
adventureIdExpTable.xmlfile, enhancing the mapping capabilities. This aligns with the PR objectives related to ingesting reward data.
67-67: Improved error handling in instance field type parsing.Replaced direct casting with
Enum.TryParseto safely convert instance types, preventing potential runtime exceptions if invalid values are encountered. The code now defaults toInstanceType.noneif parsing fails.
646-656: Added method to parse prestige ID experience table.The new
ParsePrestigeIdExpTablemethod properly constructs aPrestigeIdExpTablefrom parsed adventure ID experience data, following the established pattern of other parsing methods in the class.Maple2.Server.World/Service/WorldService.Migrate.cs (1)
103-112: Added Field request handling to support dungeon operations.The new
Fieldmethod routes requests to appropriate handlers based on theFieldCase, enabling support for dungeon creation and destruction operations. This aligns with the PR objectives related to dungeon creation adjustments.Maple2.Server.World/Service/WorldService.Party.cs (2)
25-25: Confirm all DisbandParty method invocations follow new signature.
The updated call, now passingrequest.RequestorIdandrequest.Disband, properly aligns with the revisedDisbandPartysignature.
63-70: Ensure robust handling of requestor privileges within the disband workflow.
The updated method referencesmanager.Disband(requestorId), relying onPartyManagerto handle leadership or permission checks. This approach is valid so long asPartyManagerconsistently enforces authority requirements.Maple2.Server.Game/Manager/ExperienceManager.cs (2)
99-109: Good improvement returning gained experience.
Converting fromvoidtolongreturn type helps track how many points were actually awarded. The early return of 0 whenexpGained <= 0also promotes clarity.
111-151: Consistent return usage for experience addition by type.
This method properly returns0when the experience type is invalid or the table lookups fail, and delegates toAddExp(...)otherwise. The approach aligns with the firstAddExpmethod's new return conventions.Maple2.Server.World/Containers/PartyManager.cs (3)
86-87: Confirm disband conditions for edge cases
Disbanding if fewer than 2 members are online or the total member count is ≤ 2 may be intentional. However, if you plan to keep 2-member parties when both are online, you might need to revise this logic.
142-142: Validate direct assignment of player info
AssigningInfo = infostores the reference directly rather than creating a copy. Verify if this is intentional, as changes toinfooutside this scope could affect the storedPartyMember.
419-419: No immediate concerns
AssigningParty.DungeonSet = set;appears straightforward. Ensure other parts of the code consistently reference this property.Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (5)
8-8: Import changes
No issues with addingusing Maple2.Model.Error;to handle migration/error codes.
10-10: Import of dungeon-related classes
Includingusing Maple2.Model.Game.Dungeon;aligns with the new dungeon handling methods below.
146-168: Check concurrency in dungeon creation
Creating a lobby field and subfields is a multi-step process involving metadata loading, record creation, and initialization. Confirm thread-safety if multiple calls toCreateDungeonuse the same metadata concurrently.
203-209: Destroy method return flow
The method correctly returns an error if the dungeon is absent or proceeds to dispose it. Ensure concurrency safety if multiple actors request destruction at the same time.
292-308: Verify multi-threaded disposal correctness
Ensure that no players can join while the dungeon is being disposed. The concurrency checks onRoomFieldsanddungeonslook correct, but concurrency across multiple threads could still pose issues.Maple2.Server.Game/Manager/Field/FieldManager/DungeonFieldManager.cs (6)
3-4: New imports for enums and dungeon classes
The imports forMaple2.Model.EnumandMaple2.Model.Game.Dungeoncorrectly support the new dungeon features.
6-7: Additional references for model and packet logic
These imports (Maple2.Server.Game.ModelandMaple2.Server.Game.Packets) integrate with the methods handling dungeon state changes and packet broadcasting.
12-12: Dungeon metadata as a read-only field
DeclaringDungeonMetadataas readonly ensures consistency with other immutable definitions, preventing accidental reassignment.
15-16: Examining data flow forDungeonRoomRecordandPartyId
Exposing these properties is fine, but confirm that external code uses them correctly, especially for merging party and dungeon logic.
18-27: Constructor parameter alignment
SettingDungeonMetadata,DungeonId,Size, andPartyIdin one place is clean. However, ensureLobby = this;only applies when the metadata truly matches the currentmapMetadata.Id.
29-67: Ensure stability inChangeState
The state transition logic compiles user records, broadcasts results, and callsCompleteDungeonif cleared. Confirm that player data is updated or saved appropriately before disposal.Maple2.Server.Game/Manager/DungeonManager.cs (1)
218-255: Validate party size for large groups
When creating a dungeon with a party (lines 229–237), the number of members is used directly assize. Depending on the constraints of the dungeon, you might want to ensure the party size does not exceed the maximum allowable. Failing to do so could result in a mismatch between the requested dungeon size and actual capacity.Maple2.Server.Game/Trigger/TriggerContext.MiniGame.cs (3)
42-80: Handle partial round completions more clearly
EndMiniGameRoundincrementsClearedRoundsfor winners while awarding loser bonuses ifisGainLoserBonusis true. This logic is sound, but be mindful of partial round completions or tie scenarios. You may want to track a “no winners found” edge case or handle special draw conditions where no one is in the “winning” box.
82-87: Camera direction logic is straightforward
SendingCameraPacket.Local(...)to all players in the specified box is clear and minimal. Ensure thatcameraIdvalues correspond to valid cameras or handle invalid IDs gracefully to avoid confusion in the player's client.
117-127: Initialize MiniGameUserRecord carefully
TheStartMiniGamemethod sets eachMiniGameRecordwithMinRound = 1; if you need more nuanced behaviors (e.g., some mini-games start from round 0 or might skip rounds), ensure these fields are dynamically set or validated.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
Maple2.Server.Game/Trigger/TriggerContext.MiniGame.cs (1)
137-139:⚠️ Potential issueAvoid returning prematurely inside a multi-player loop.
If one player in the loop has reached the
TotalRounds == MinRound, the methodStartMiniGameRoundreturns immediately, bypassing the round start for other players. This may lead to inconsistent states among participants.Apply this diff to fix the issue:
-if (player.Session.MiniGameRecord.TotalRounds == player.Session.MiniGameRecord.MinRound) { - return; -} +if (player.Session.MiniGameRecord.TotalRounds == player.Session.MiniGameRecord.MinRound) { + continue; +}Maple2.Server.Game/Manager/DungeonManager.cs (1)
287-300:⚠️ Potential issueGuard for invalid indexes in field IDs
The code accesses
Metadata.FieldIds[0]without verifying thatFieldIdsis non-empty. This could lead to anIndexOutOfRangeExceptionif the array is empty.Apply this change to add a guard clause:
if (Lobby == null || Metadata == null || Lobby.RoomFields.IsEmpty) { logger.Error("Field is null, cannot enter dungeon"); return; } +if (Metadata.FieldIds.Length == 0) { + logger.Error("Dungeon has no field IDs, cannot enter dungeon"); + return; +} if (!Lobby.RoomFields.TryGetValue(Metadata.FieldIds[0], out DungeonFieldManager? firstField)) { logger.Error("First field is null, cannot enter dungeon"); return; }
🧹 Nitpick comments (2)
Maple2.Model/Game/RewardItem.cs (1)
28-46: Well-designed RewardRecord structure.The new RewardRecord struct effectively encapsulates all reward components (items, experience, prestige experience, and meso) in a single immutable package. The dual constructors provide flexibility, allowing creation from either RewardItem collections or Item collections. This aligns well with the PR objectives for implementing dungeon and mini-game rewards.
Two minor observations:
- The Items property initializes with
= []which uses C# 12 collection expressions - ensure your target framework supports this.- Consider adding a parameterless constructor to create empty rewards (similar to what's used in GetRewardContent when metadata isn't found).
Maple2.Model/Game/Dungeon/DungeonUserRecord.cs (1)
7-21: Consider encapsulating fields with properties for better maintainability.Exposing multiple public fields directly can quickly lead to maintenance issues and unintended modifications. Using properties or private fields with public getters and setters fosters better encapsulation and helps maintain consistent usage across the codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
Maple2.Model/Game/Dungeon/DungeonUserRecord.cs(1 hunks)Maple2.Model/Game/Dungeon/MiniGameUserRecord.cs(1 hunks)Maple2.Model/Game/RewardItem.cs(2 hunks)Maple2.Model/Metadata/Constants.cs(1 hunks)Maple2.Server.Game/Manager/DungeonManager.cs(6 hunks)Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.cs(4 hunks)Maple2.Server.Game/Packets/DungeonWaitingPacket.cs(1 hunks)Maple2.Server.Game/Packets/RoomStageDungeonPacket.cs(1 hunks)Maple2.Server.Game/Trigger/TriggerContext.Dungeon.cs(3 hunks)Maple2.Server.Game/Trigger/TriggerContext.MiniGame.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- Maple2.Server.Game/Packets/RoomStageDungeonPacket.cs
- Maple2.Server.Game/Packets/DungeonWaitingPacket.cs
- Maple2.Model/Metadata/Constants.cs
- Maple2.Server.Game/Trigger/TriggerContext.Dungeon.cs
- Maple2.Model/Game/Dungeon/MiniGameUserRecord.cs
🧰 Additional context used
🧬 Code Definitions (4)
Maple2.Model/Game/RewardItem.cs (1)
Maple2.Server.Game/Session/GameSession.cs (1) (1)
RewardRecord(524-579)
Maple2.Model/Game/Dungeon/DungeonUserRecord.cs (2)
Maple2.Model/Game/Dungeon/MiniGameUserRecord.cs (2) (2)
Add(23-31)WriteTo(33-52)Maple2.Model/Game/RewardItem.cs (2) (2)
RewardRecord(34-39)RewardRecord(41-46)
Maple2.Server.Game/Manager/DungeonManager.cs (5)
Maple2.Server.Game/Manager/Field/FieldManager/DungeonFieldManager.cs (2) (2)
DungeonFieldManager(11-69)DungeonFieldManager(18-27)Maple2.Model/Game/Dungeon/DungeonRoomRecord.cs (2) (2)
DungeonRoomRecord(7-17)DungeonRoomRecord(14-16)Maple2.Model/Game/Dungeon/DungeonUserRecord.cs (3) (3)
DungeonUserRecord(7-91)DungeonUserRecord(23-35)Add(37-45)Maple2.Server.Game/Packets/DungeonRoomPacket.cs (1) (1)
DungeonRoomPacket(11-85)Maple2.Server.Game/Packets/DungeonRewardPacket.cs (1) (1)
DungeonRewardPacket(9-32)
Maple2.Server.Game/Trigger/TriggerContext.MiniGame.cs (3)
Maple2.Server.Game/Trigger/TriggerContext.cs (2) (2)
TriggerContext(15-232)TriggerContext(31-39)Maple2.Server.Game/Packets/DungeonRewardPacket.cs (1) (1)
DungeonRewardPacket(9-32)Maple2.Model/Game/Dungeon/MiniGameUserRecord.cs (3) (3)
MiniGameUserRecord(6-53)MiniGameUserRecord(15-21)Add(23-31)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (22)
Maple2.Model/Game/RewardItem.cs (2)
7-7: Good change to record struct for RewardItem.Changing to a record struct provides value-based equality, better toString implementation, and reinforces the immutability semantics of this read-only data structure. This is a good practice for data objects, especially those used for reward representation.
23-25: Nice addition of implicit conversion operator.This operator elegantly simplifies the conversion from Item to RewardItem, making the codebase more readable by eliminating explicit conversions when working with rewards. The implementation correctly maps properties from the source to the target type.
Maple2.Server.Game/Trigger/TriggerContext.MiniGame.cs (6)
14-40: Good implementation of end-game rewards.The code properly handles both winners and losers, with appropriate null checks and conditional logic for showing results. Setting the MiniGameRecord to null after processing ensures clean state management.
42-80: Well-structured reward system with type differentiation.The implementation correctly differentiates between experience types based on game name, and handles rewards for both winners and losers appropriately. The meso reward is only added when the value is positive, and round tracking is properly maintained.
82-87: Camera direction function is correctly implemented.The method properly sends camera packets to all players in the specified box.
89-99: Experience reward handling is well-implemented.The code correctly checks for null records before adding experience, and properly updates the player's reward records.
101-114: Reward distribution includes proper error handling.The implementation now includes checking if the content type exists and logs warnings appropriately if it doesn't.
118-128: Clean initialization of mini-game records.The code correctly initializes the mini-game record for each player with appropriate default values.
Maple2.Model/Game/Dungeon/DungeonUserRecord.cs (3)
23-35: Constructor logic looks good.These initializations align well with the defined enumerations, ensuring default values and preventing missing dictionary keys.
37-45: Add a null check forrecord.Itemsto prevent potential NullReferenceException.This issue is identical to a past review comment in a similar method. If
record.Itemsis null, iterating over it will throw at runtime.
[flag_critical_issue, duplicate_comment]public void Add(RewardRecord record) { Rewards[DungeonRewardType.Exp] += (int) record.Exp; Rewards[DungeonRewardType.Meso] += (int) record.Meso; Rewards[DungeonRewardType.Prestige] += (int) record.PrestigeExp; - foreach (RewardItem item in record.Items) { - RewardItems.Add(item); - } + if (record.Items != null) { + foreach (RewardItem item in record.Items) { + RewardItems.Add(item); + } + } }
47-90: Serialization logic appears properly structured and consistent.The partitioned writing of rewards, items, and bonus items matches the enumerated types and avoids the common pitfalls of collection size mismatches.
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.cs (4)
74-76: Good addition of dungeon-related properties.Adding these properties with the
initaccessor is a good practice for immutability after initialization, providing better structure for dungeon field management.
105-105: Default initialization for FieldType is appropriate.Setting the default field type during initialization ensures consistent behavior for all fields.
134-139: Fixed the default plot initialization.Replacing the
Plot.Defaultwith a proper instance ofPlotincluding UgcMapGroup parameters fixes the issue with default plots mentioned in the PR objectives.
123-123:Details
✅ Verification successful
Verify the implications of removing blockChangeChannel parameter.
The
blockChangeChannel: trueparameter has been removed from theFieldInstanceconstructor. This change appears intentional to allow dungeon creation on the first instance channel as mentioned in the PR objectives, but ensure this doesn't unintentionally allow channel changes in other contexts.
🏁 Script executed:
#!/bin/bash # Check for other usages of blockChangeChannel in the codebase echo "Searching for other usages of blockChangeChannel parameter..." rg "blockChangeChannel" -A 5 -B 5Length of output: 166
Let's gather a bit more context on how the
FieldInstanceconstructor is defined to ensure that removing theblockChangeChannelparameter won't have unexpected side effects. It’s important to verify that the constructor no longer expects this parameter (or that it has a safe default), and that its removal here doesn’t leave any gaps elsewhere in the codebase.Please run the following shell script to search for the
FieldInstanceconstructor definition and check for any mentions ofblockChangeChannel:Once we have this output, we can determine if the removal was safe or if further adjustments are needed.
🏁 Script executed:
#!/bin/bash echo "Searching for FieldInstance constructor for usage of blockChangeChannel..." rg "public FieldInstance\s*\(" -A 20 -B 20Length of output: 1577
FieldInstance Constructor: Removal of blockChangeChannel Confirmed
I verified that the
FieldInstanceconstructor now accepts only theinstanceTypeandinstanceIdparameters (seeMaple2.Model/Game/FieldInstance.cs), and there are no remaining usages ofblockChangeChannelin the codebase. This aligns with the PR objectives, and no unintended side effects regarding channel changes appear to be present.Maple2.Server.Game/Manager/DungeonManager.cs (7)
28-31: Good renaming and property enhancements.Renaming
FieldtoLobbywith a private setter improves encapsulation, and the added properties provide convenient access to dungeon state and user records.
64-68: Well-structured Load method.This new method sends all necessary dungeon-related packets to the client in a clear and organized way.
70-78: Good field loading implementation.The
LoadFieldmethod properly checks for null values and sends the appropriate packet based on the instance type.
207-214: Enhanced SetDungeon method with user record handling.The updated method now properly initializes the user record and adds it to the dungeon room record, which is essential for tracking player progress in dungeons.
240-257: Improved dungeon creation with RPC and error handling.The implementation now uses proper RPC calls to the field service with good error handling, which is critical for a reliable dungeon creation process.
320-354: Well-implemented dungeon completion logic.The
CompleteDungeonmethod properly:
- Validates required objects are not null
- Calculates and awards experience and meso rewards
- Handles item rewards from drop boxes
- Adds items to inventory or mails them if inventory is full
- Sends appropriate packets to update the client
This implementation aligns well with the PR objective of implementing rudimentary dungeon rewards.
356-390: Good dungeon reset implementation.The
Resetmethod properly:
- Verifies the player is the party leader
- Makes a field request to destroy the dungeon
- Handles RPC errors
- Updates the party state via the party service
This effectively supports the PR objective of providing the ability to reset dungeons.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes