Plot expiration & misc home fixes - #525
Conversation
WalkthroughThis update implements extensive enhancements to plot and cube management for the game's housing and field systems. It introduces new gRPC methods and protobuf messages for plot updates, adds robust plot and cube lifecycle management in both server and database layers, refines packet serialization, and improves error handling, synchronization, and data consistency across services. Changes
Sequence Diagram(s)Plot Update Request Flow (High-Level)sequenceDiagram
participant Client
participant GameServer
participant ChannelService
participant WorldService
participant Database
Client->>GameServer: Request plot/cube update
GameServer->>ChannelService: UpdateFieldPlot(FieldPlotRequest)
ChannelService->>WorldService: UpdateFieldPlot(FieldPlotRequest)
WorldService->>ChannelService: Broadcast UpdateFieldPlot to channels
ChannelService->>Database: Update plot/cube data
ChannelService->>GameServer: Broadcast updated plot/cube info
GameServer->>Client: Send update confirmation/packets
Field Plot Expiry Check (WorldServer)sequenceDiagram
participant WorldServer
participant Database
participant ChannelClients
WorldServer->>Database: Get expired plots
Database-->>WorldServer: List of expired plots
WorldServer->>Database: Set plot pending/open, delete cubes as needed
WorldServer->>ChannelClients: UpdateFieldPlot (notify plot state changes)
WorldServer->>WorldServer: Schedule next expiry check
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsMaple2.Server.Game/Manager/HousingManager.cs (2)🧬 Code Graph Analysis (1)Maple2.Server.Game/Manager/HousingManager.cs (13)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (20)
✨ 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: 11
🔭 Outside diff range comments (4)
Maple2.Tools/Extensions/PacketExtensions.cs (1)
65-71:ReadArray<T>loop condition prevents any element from being read
for (int i = 0; i > size; i++)never executes because0 > sizeis false for every non-negativesize. This results in always returning an empty array.- for (int i = 0; i > size; i++) { + for (int i = 0; i < size; i++) {Without this fix, every call to
ReadArraysilently drops data.Maple2.Tools/Collision/IPolygon.cs (1)
19-20:[]collection expression may require C# 12 (preview)
[]for an empty array relies on the new collection-expression feature (C# 12).
If the project isn’t compiled with the latest language version / preview features, replace with the traditional form:- public override Vector2[] Points => []; + public override Vector2[] Points => Array.Empty<Vector2>();Maple2.Server.Game/Service/ChannelService.cs (1)
16-22: Missing field assignment for ItemMetadataStorage parameter.The
itemMetadataparameter is added to the constructor but not assigned to a field, making it unusable by service methods. Consider adding a private readonly field and assignment.public partial class ChannelService : Channel.Service.Channel.ChannelBase { private readonly GameServer server; private readonly PlayerInfoStorage playerInfos; private readonly GameStorage gameStorage; private readonly TableMetadataStorage tableMetadata; private readonly ServerTableMetadataStorage serverTableMetadata; + private readonly ItemMetadataStorage itemMetadata; private readonly ILogger logger = Log.Logger.ForContext<ChannelService>(); public ChannelService(GameServer server, PlayerInfoStorage playerInfos, GameStorage gameStorage, ServerTableMetadataStorage serverTableMetadata, TableMetadataStorage tableMetadata, ItemMetadataStorage itemMetadata) { this.server = server; this.playerInfos = playerInfos; this.gameStorage = gameStorage; this.serverTableMetadata = serverTableMetadata; this.tableMetadata = tableMetadata; + this.itemMetadata = itemMetadata; }Maple2.Server.Game/Manager/Items/FurnishingManager.cs (1)
85-111: Add null validation for the cube parameter.The method now accepts a
PlotCubeparameter but doesn't validate it's not null before use. Consider adding a null check at the beginning of the method.public bool TryAddCube(long uid, PlotCube cube) { + if (cube == null) { + logger.Error("Attempted to add null cube"); + return false; + } const int amount = 1; lock (session.Item) { if (session.Field == null) { return false; }
🧹 Nitpick comments (5)
Maple2.Tools/Extensions/PacketExtensions.cs (1)
95-106: Null-collection branch writes an implicit value – be explicit
writer.WriteInt();relies on the overload that defaults to0. Being explicit improves readability and avoids accidental misuse if the API changes.- writer.WriteInt(); // 0 items + writer.WriteInt(0); // 0 itemsMaple2.Tools/Collision/IPolygon.cs (1)
8-10: Inconsistent defaultepsilonbetween overloadsThe float-based overload defaults to
0.001f, while theVector2overload defaults to1e-5f.
Confirm this intentional; otherwise standardise the default to avoid surprising callers.Maple2.Server.World/Service/WorldService.UpdateFieldPlot.cs (1)
6-21: Consider adding error handling for channel broadcast failures.The current implementation doesn't handle potential failures when broadcasting to channel clients. Consider wrapping the broadcast calls in try-catch blocks to ensure one failed channel doesn't prevent updates to other channels.
public override Task<FieldPlotResponse> UpdateFieldPlot(FieldPlotRequest request, ServerCallContext context) { if (request.MapId == -1) { worldServer.FieldPlotExpiryCheck(); return Task.FromResult(new FieldPlotResponse()); } if (request.MapId <= 0) { throw new RpcException(new Status(StatusCode.InvalidArgument, "Invalid map ID")); } foreach ((int, Channel.Service.Channel.ChannelClient) channel in channelClients) { - channel.Item2.UpdateFieldPlot(request); + try { + channel.Item2.UpdateFieldPlot(request); + } catch (Exception ex) { + logger.Error(ex, "Failed to update field plot on channel {ChannelId}", channel.Item1); + } } return Task.FromResult(new FieldPlotResponse()); }Maple2.Database/Storage/Game/GameStorage.Map.cs (1)
147-155: Use consistent null check formatting.For consistency with the codebase style, add braces to the null check.
Apply this diff:
public void SetPlotOpen(long plotId) { UgcMap? model = Context.UgcMap.Find(plotId); - if (model == null) return; + if (model == null) { + return; + } model.ExpiryTime = DateTimeOffset.MinValue; Context.UgcMap.Update(model); Context.TrySaveChanges(); }Maple2.Server.Game/Manager/HousingManager.cs (1)
491-527: Consider using transactions for database consistency.The
CreateCubemethod performs multiple database operations (nurturing creation and cube creation) that should be atomic. If cube creation fails after nurturing is created, it could leave orphaned nurturing records.Consider wrapping the operations in a transaction to ensure consistency:
private PlotCube? CreateCube(Plot plot, ItemMetadata itemMetadata, FunctionCubeMetadata? functionCubeMetadata, UgcItemLook? template, Vector3 position, float rotation) { var result = new PlotCube(itemMetadata, 0, template) { Type = PlotCube.CubeType.Construction, Position = position, Rotation = rotation, }; using GameStorage.Request db = session.GameStorage.Context(); + // Consider using a transaction here + // using var transaction = db.BeginTransaction(); if (functionCubeMetadata is not null) { result.Interact = new InteractCube(position, functionCubeMetadata); if (result.Interact.Nurturing is not null && result.Interact.Metadata.Nurturing is not null) { Nurturing? nurturing = db.GetNurturing(session.AccountId, result.ItemId, result.Interact.Metadata.Nurturing); if (nurturing is null) { nurturing = db.CreateNurturing(session.AccountId, result.Interact.Metadata.Nurturing, result.Interact.Metadata.Id); if (nurturing is null) { logger.Error("Failed to create Nurturing for {AccountId}, ItemId {ItemId}", session.AccountId, result.ItemId); + // transaction?.Rollback(); return null; } } } } result = db.CreateCube(plot, result); if (result is null) { logger.Error("Failed to create cube for plot {PlotNumber} at position {Position}.", plot.Number, position); + // transaction?.Rollback(); return null; } + // transaction?.Commit(); return result; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
Maple2.Database/Storage/Game/GameStorage.Map.cs(4 hunks)Maple2.Database/Storage/Game/GameStorage.Nurturing.cs(1 hunks)Maple2.Model/Game/Cube/PlotCube.cs(2 hunks)Maple2.Model/Game/User/Home.cs(1 hunks)Maple2.Model/Game/User/PlotInfo.cs(2 hunks)Maple2.Model/Metadata/Constants.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/Commands/DebugCommand.cs(3 hunks)Maple2.Server.Game/GameServer.cs(1 hunks)Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs(1 hunks)Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs(1 hunks)Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Ugc.cs(3 hunks)Maple2.Server.Game/Manager/Field/FieldManager/HomeFieldManager.cs(2 hunks)Maple2.Server.Game/Manager/HousingManager.cs(14 hunks)Maple2.Server.Game/Manager/Items/FurnishingManager.cs(2 hunks)Maple2.Server.Game/Manager/Items/ItemManager.cs(2 hunks)Maple2.Server.Game/PacketHandlers/ChannelHandler.cs(2 hunks)Maple2.Server.Game/PacketHandlers/DungeonRoomHandler.cs(0 hunks)Maple2.Server.Game/PacketHandlers/LoadUgcMapHandler.cs(2 hunks)Maple2.Server.Game/PacketHandlers/RequestCubeHandler.cs(6 hunks)Maple2.Server.Game/PacketHandlers/TimeSyncHandler.cs(1 hunks)Maple2.Server.Game/Packets/LoadCubesPacket.cs(3 hunks)Maple2.Server.Game/Service/ChannelService.UpdateFieldPlot.cs(1 hunks)Maple2.Server.Game/Service/ChannelService.cs(1 hunks)Maple2.Server.Game/Session/GameSession.cs(3 hunks)Maple2.Server.World/Service/WorldService.UpdateFieldPlot.cs(1 hunks)Maple2.Server.World/WorldServer.cs(5 hunks)Maple2.Tools/Collision/IPolygon.cs(1 hunks)Maple2.Tools/Extensions/PacketExtensions.cs(1 hunks)
💤 Files with no reviewable changes (1)
- Maple2.Server.Game/PacketHandlers/DungeonRoomHandler.cs
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#236
File: Maple2.Server.Game/PacketHandlers/RequestCubeHandler.cs:375-422
Timestamp: 2024-09-16T07:50:30.281Z
Learning: In `RequestCubeHandler`, the `HandleLoadLayout` method is responsible for applying the layout to the plot after missing cubes are calculated in `HandleRequestLayout`.
Maple2.Tools/Extensions/PacketExtensions.cs (3)
Learnt from: Zintixx
PR: AngeloTadeucci/Maple2#279
File: Maple2.File.Ingest/Mapper/TableMapper.cs:1525-1551
Timestamp: 2024-10-12T20:08:58.356Z
Learning: When reviewing C# code, remember that initializing lists with `[]` is valid in C# 12 and later.
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#415
File: Maple2.Database/Storage/Metadata/QuestMetadataStorage.cs:12-12
Timestamp: 2025-04-13T23:57:40.599Z
Learning: C# 12 (released with .NET 8) supports collection expressions which allow using `[]` syntax to initialize collections including ConcurrentDictionary and other collection types.
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#415
File: Maple2.Database/Storage/Metadata/QuestMetadataStorage.cs:12-12
Timestamp: 2025-04-13T23:57:40.599Z
Learning: C# 12 (released with .NET 8) supports collection expressions which allow using `[]` syntax to initialize collections including ConcurrentDictionary and other collection types.
Maple2.Tools/Collision/IPolygon.cs (2)
Learnt from: mettaursp
PR: AngeloTadeucci/Maple2#270
File: Maple2.Tools/VectorMath/Transform.cs:92-94
Timestamp: 2024-10-09T04:13:28.776Z
Learning: In the `Transform.Scale` setter in `Maple2.Tools/VectorMath/Transform.cs`, the axes are normalized and directly scaled without considering the original scale, as it's no longer a factor after normalization.
Learnt from: mettaursp
PR: AngeloTadeucci/Maple2#270
File: Maple2.Tools/VectorMath/Transform.cs:92-94
Timestamp: 2024-09-30T04:20:44.252Z
Learning: In the `Transform.Scale` setter in `Maple2.Tools/VectorMath/Transform.cs`, the axes are normalized and directly scaled without considering the original scale, as it's no longer a factor after normalization.
Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (2)
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#199
File: Maple2.Server.Game/PacketHandlers/FileHandler.cs:0-0
Timestamp: 2024-10-09T04:13:28.776Z
Learning: Packets can't ever be null inside the `Handle` function of packet handlers in the repository.
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#199
File: Maple2.Server.Game/PacketHandlers/FileHandler.cs:0-0
Timestamp: 2024-07-12T04:31:25.376Z
Learning: Packets can't ever be null inside the `Handle` function of packet handlers in the repository.
Maple2.Server.Game/PacketHandlers/LoadUgcMapHandler.cs (4)
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#215
File: Maple2.Server.Core/Packets/UgcPacket.cs:111-127
Timestamp: 2024-08-22T01:51:54.865Z
Learning: The `counter1` variable and the associated loop in the `LoadBanners` method of `UgcPacket` are intended to document the packet structure rather than serve a functional purpose.
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#215
File: Maple2.Server.Core/Packets/UgcPacket.cs:111-127
Timestamp: 2024-10-09T04:13:28.776Z
Learning: The `counter1` variable and the associated loop in the `LoadBanners` method of `UgcPacket` are intended to document the packet structure rather than serve a functional purpose.
Learnt from: Zintixx
PR: AngeloTadeucci/Maple2#279
File: Maple2.Database/Storage/Game/GameStorage.Wedding.cs:147-159
Timestamp: 2024-10-13T18:10:08.059Z
Learning: In the `GameStorage` class in `Maple2.Database/Storage/Game/GameStorage.Wedding.cs`, the `GetMarriage` method is necessary to properly convert the `Marriage` object and initialize `Partner1` and `Partner2`.
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#236
File: Maple2.Server.Game/PacketHandlers/RequestCubeHandler.cs:375-422
Timestamp: 2024-09-16T07:50:30.281Z
Learning: In `RequestCubeHandler`, the `HandleLoadLayout` method is responsible for applying the layout to the plot after missing cubes are calculated in `HandleRequestLayout`.
Maple2.Server.Game/PacketHandlers/TimeSyncHandler.cs (2)
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#199
File: Maple2.Server.Game/PacketHandlers/FileHandler.cs:0-0
Timestamp: 2024-10-09T04:13:28.776Z
Learning: Packets can't ever be null inside the `Handle` function of packet handlers in the repository.
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#199
File: Maple2.Server.Game/PacketHandlers/FileHandler.cs:0-0
Timestamp: 2024-07-12T04:31:25.376Z
Learning: Packets can't ever be null inside the `Handle` function of packet handlers in the repository.
Maple2.Server.Game/GameServer.cs (2)
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#239
File: Maple2.Server.Game/Session/GameSession.State.cs:26-27
Timestamp: 2024-09-19T04:27:27.320Z
Learning: In `GameSession` class, fields are public. When adding new fields to this class, they should be public for consistency.
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#239
File: Maple2.Server.Game/Session/GameSession.State.cs:26-27
Timestamp: 2024-10-09T04:13:28.776Z
Learning: In `GameSession` class, fields are public. When adding new fields to this class, they should be public for consistency.
Maple2.Database/Storage/Game/GameStorage.Nurturing.cs (1)
Learnt from: Zintixx
PR: AngeloTadeucci/Maple2#279
File: Maple2.Database/Storage/Game/GameStorage.Wedding.cs:147-159
Timestamp: 2024-10-13T18:10:08.059Z
Learning: In the `GameStorage` class in `Maple2.Database/Storage/Game/GameStorage.Wedding.cs`, the `GetMarriage` method is necessary to properly convert the `Marriage` object and initialize `Partner1` and `Partner2`.
Maple2.Server.Game/Manager/Items/ItemManager.cs (1)
Learnt from: Zintixx
PR: AngeloTadeucci/Maple2#279
File: Maple2.Database/Storage/Game/GameStorage.Wedding.cs:147-159
Timestamp: 2024-10-13T18:10:08.059Z
Learning: In the `GameStorage` class in `Maple2.Database/Storage/Game/GameStorage.Wedding.cs`, the `GetMarriage` method is necessary to properly convert the `Marriage` object and initialize `Partner1` and `Partner2`.
Maple2.Server.Game/Session/GameSession.cs (3)
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#239
File: Maple2.Server.Game/Session/GameSession.State.cs:26-27
Timestamp: 2024-09-19T04:27:27.320Z
Learning: In `GameSession` class, fields are public. When adding new fields to this class, they should be public for consistency.
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#239
File: Maple2.Server.Game/Session/GameSession.State.cs:26-27
Timestamp: 2024-10-09T04:13:28.776Z
Learning: In `GameSession` class, fields are public. When adding new fields to this class, they should be public for consistency.
Learnt from: Zintixx
PR: AngeloTadeucci/Maple2#279
File: Maple2.Database/Storage/Game/GameStorage.Wedding.cs:147-159
Timestamp: 2024-10-13T18:10:08.059Z
Learning: In the `GameStorage` class in `Maple2.Database/Storage/Game/GameStorage.Wedding.cs`, the `GetMarriage` method is necessary to properly convert the `Marriage` object and initialize `Partner1` and `Partner2`.
Maple2.Server.Game/Manager/Field/FieldManager/HomeFieldManager.cs (2)
Learnt from: Zintixx
PR: AngeloTadeucci/Maple2#279
File: Maple2.Database/Storage/Game/GameStorage.Wedding.cs:147-159
Timestamp: 2024-10-13T18:10:08.059Z
Learning: In the `GameStorage` class in `Maple2.Database/Storage/Game/GameStorage.Wedding.cs`, the `GetMarriage` method is necessary to properly convert the `Marriage` object and initialize `Partner1` and `Partner2`.
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#236
File: Maple2.Server.Game/PacketHandlers/RequestCubeHandler.cs:375-422
Timestamp: 2024-09-16T07:50:30.281Z
Learning: In `RequestCubeHandler`, the `HandleLoadLayout` method is responsible for applying the layout to the plot after missing cubes are calculated in `HandleRequestLayout`.
Maple2.Server.Game/PacketHandlers/RequestCubeHandler.cs (3)
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#236
File: Maple2.Server.Game/PacketHandlers/RequestCubeHandler.cs:375-422
Timestamp: 2024-09-16T07:50:30.281Z
Learning: In `RequestCubeHandler`, the `HandleLoadLayout` method is responsible for applying the layout to the plot after missing cubes are calculated in `HandleRequestLayout`.
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#199
File: Maple2.Server.Game/PacketHandlers/FileHandler.cs:0-0
Timestamp: 2024-10-09T04:13:28.776Z
Learning: Packets can't ever be null inside the `Handle` function of packet handlers in the repository.
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#199
File: Maple2.Server.Game/PacketHandlers/FileHandler.cs:0-0
Timestamp: 2024-07-12T04:31:25.376Z
Learning: Packets can't ever be null inside the `Handle` function of packet handlers in the repository.
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Ugc.cs (1)
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#236
File: Maple2.Server.Game/PacketHandlers/RequestCubeHandler.cs:375-422
Timestamp: 2024-09-16T07:50:30.281Z
Learning: In `RequestCubeHandler`, the `HandleLoadLayout` method is responsible for applying the layout to the plot after missing cubes are calculated in `HandleRequestLayout`.
Maple2.Database/Storage/Game/GameStorage.Map.cs (2)
Learnt from: Zintixx
PR: AngeloTadeucci/Maple2#279
File: Maple2.Database/Storage/Game/GameStorage.Wedding.cs:147-159
Timestamp: 2024-10-13T18:10:08.059Z
Learning: In the `GameStorage` class in `Maple2.Database/Storage/Game/GameStorage.Wedding.cs`, the `GetMarriage` method is necessary to properly convert the `Marriage` object and initialize `Partner1` and `Partner2`.
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#236
File: Maple2.Server.Game/PacketHandlers/RequestCubeHandler.cs:375-422
Timestamp: 2024-09-16T07:50:30.281Z
Learning: In `RequestCubeHandler`, the `HandleLoadLayout` method is responsible for applying the layout to the plot after missing cubes are calculated in `HandleRequestLayout`.
Maple2.Server.Game/Manager/HousingManager.cs (2)
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#236
File: Maple2.Server.Game/PacketHandlers/RequestCubeHandler.cs:375-422
Timestamp: 2024-09-16T07:50:30.281Z
Learning: In `RequestCubeHandler`, the `HandleLoadLayout` method is responsible for applying the layout to the plot after missing cubes are calculated in `HandleRequestLayout`.
Learnt from: Zintixx
PR: AngeloTadeucci/Maple2#279
File: Maple2.Database/Storage/Game/GameStorage.Wedding.cs:147-159
Timestamp: 2024-10-13T18:10:08.059Z
Learning: In the `GameStorage` class in `Maple2.Database/Storage/Game/GameStorage.Wedding.cs`, the `GetMarriage` method is necessary to properly convert the `Marriage` object and initialize `Partner1` and `Partner2`.
🧬 Code Graph Analysis (6)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (1)
Maple2.Server.Game/Manager/Field/FieldManager/HomeFieldManager.cs (2)
HomeFieldManager(7-53)HomeFieldManager(15-19)
Maple2.Server.Game/Manager/Items/ItemManager.cs (3)
Maple2.Server.Game/Manager/Items/FurnishingManager.cs (3)
FurnishingManager(15-316)FurnishingManager(26-45)Load(47-64)Maple2.Database/Storage/Game/GameStorage.Map.cs (2)
GameStorage(16-450)Request(17-449)Maple2.Database/Storage/Game/GameStorage.Item.cs (2)
GameStorage(11-192)Request(12-191)
Maple2.Server.Game/Session/GameSession.cs (1)
Maple2.Server.World/Containers/ChannelClientLookup.cs (2)
Channel(36-59)Channel(49-58)
Maple2.Model/Game/User/PlotInfo.cs (4)
Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (1)
ToString(681-687)Maple2.Tools/Collision/BoundingBox.cs (1)
ToString(29-31)Maple2.Model/Game/TriggerObject.cs (1)
ToString(123-125)Maple2.Server.Game/Model/Field/Buff.cs (1)
ToString(333-335)
Maple2.Server.Game/Commands/DebugCommand.cs (6)
Maple2.Server.Game/Commands/CommandRouter.cs (2)
GameCommand(108-113)GameCommand(110-112)Maple2.Server.Game/Session/GameSession.cs (3)
GameSession(36-850)GameSession(116-126)GameSession(740-740)Maple2.Database/Storage/Metadata/MapMetadataStorage.cs (3)
MapMetadataStorage(10-93)MapMetadataStorage(17-24)TryGetUgc(49-64)Maple2.Database/Storage/Game/GameStorage.Map.cs (7)
GameStorage(16-450)Request(17-449)PlotInfo(50-70)PlotInfo(72-89)PlotInfo(91-111)PlotInfo(113-123)PlotInfo(360-382)Maple2.Model/Game/User/Home.cs (1)
Home(10-199)Maple2.Model/Game/User/PlotInfo.cs (2)
PlotInfo(7-48)PlotInfo(32-34)
Maple2.Database/Storage/Game/GameStorage.Map.cs (7)
Maple2.Model/Game/User/PlotInfo.cs (3)
PlotInfo(7-48)PlotInfo(32-34)Plot(50-60)Maple2.Server.Game/Manager/HousingManager.cs (5)
PlotInfo(215-258)Plot(118-135)Plot(137-146)PlotCube(491-521)DeleteCube(523-527)Maple2.Database/Model/Map/UgcMap.cs (1)
UgcMap(8-45)Maple2.Model/Metadata/Constants.cs (1)
Constant(10-959)Maple2.Database/Extensions/DbContextExtensions.cs (1)
TrySaveChanges(16-25)Maple2.Model/Game/Cube/PlotCube.cs (2)
PlotCube(6-37)PlotCube(22-28)Maple2.Database/Model/Map/UgcMapCube.cs (1)
UgcMapCube(11-49)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (42)
Maple2.Tools/Extensions/PacketExtensions.cs (1)
1-1: Unusedusing System;removal looks goodNice clean-up; the directive was genuinely unused and its removal reduces noise.
Maple2.Tools/Collision/IPolygon.cs (1)
14-14: Return type now implicitly depends onSystem.Range; confirm target framework versionEven after re-adding
using System, ensure the target framework isnetstandard2.1/net6.0+, otherwiseSystem.Rangeis unavailable.
Verify the libraries consumingIPolygonare built against a compatible TFM.Maple2.Server.Game/PacketHandlers/ChannelHandler.cs (1)
15-18: Re-verifying behavioral parity after swapping base classChanging the inheritance from
FieldPacketHandlertoPacketHandler<GameSession>alters:
- Lifecycle hooks (e.g. any virtual
OnConnected/OnDisconnectedlogic).- The guarantee that
session.Fieldwas non-null in previous implementations.While the current handler still calls
session.Field?.MapId, losing any field-specific pre-checks could surface NOREF bugs in edge cases (e.g. user changes to a channel from the login-lobby, whereFieldmight be null).Double-check:
- Other overrides (
OnEnterField, etc.) were not inadvertently dropped.- Upstream DI registrations bind
ChannelHandlerexactly once per session regardless of the new base type.- Unit/integration tests still cover “change channel while not loaded into a field”.
No code change required if these are already covered; just ensure parity.
Maple2.Server.Game/PacketHandlers/TimeSyncHandler.cs (1)
1-4: LGTM: Clean namespace reorganization.The namespace change properly moves the handler to the game server package while maintaining the inheritance relationship through the using directive. This aligns with the broader architectural changes mentioned in the PR objectives.
Maple2.Database/Storage/Game/GameStorage.Nurturing.cs (1)
10-11: LGTM: Parameter name change aligns with architectural shift.The change from
characterIdtoaccountIdproperly reflects the semantic shift toward account-based identification mentioned in the AI summary. The implementation is consistent with the parameter name change.Maple2.Server.Core/proto/channel/channel.proto (1)
51-52: LGTM: RPC addition supports plot management system.The new
UpdateFieldPlotRPC method is properly integrated into the Channel service and follows the existing pattern. This supports the broader plot lifecycle management and synchronization features mentioned in the PR objectives.Maple2.Server.Core/proto/world/world.proto (1)
65-66: LGTM: Consistent RPC addition enables cross-server plot updates.The
UpdateFieldPlotRPC method mirrors the Channel service implementation, enabling proper coordination between world and channel servers for plot state synchronization as described in the PR objectives.Maple2.Server.Game/PacketHandlers/LoadUgcMapHandler.cs (2)
47-47: LGTM: Helpful comment clarifies data source.The comment clearly explains why the home is fetched from the database instead of using the session's player value, which provides valuable context for future maintainers.
83-86: LGTM: Refined plot filtering logic.The filtering logic has been improved to be more specific:
PlotOwnersnow only includes plots with statePlotState.Taken(more precise than all non-open plots)PlotExpirymaintains the existing logic for all non-open plotsThis refinement aligns with the enhanced plot state management described in the AI summary and should provide more accurate plot information to clients.
Maple2.Model/Metadata/Constants.cs (1)
278-278: LGTM! Excellent type safety improvement.Converting the constant from raw seconds to a strongly-typed
TimeSpanimproves code clarity and prevents unit confusion. This aligns well with other time-related constants in the same region.Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Factory.cs (1)
138-138: LGTM! Constructor parameter alignment.The addition of the
roomIdparameter correctly aligns with the updatedHomeFieldManagerconstructor. This supports the new room-specific plot management functionality whereroomId == -1indicates planner plots that should start empty.Maple2.Server.Game/Packets/LoadCubesPacket.cs (3)
45-45: LGTM! Simplified plot state serialization.Writing a boolean for whether the plot is
Takensimplifies client-side logic compared to serializing the full enum value.
57-58: LGTM! Added missing plot data fields.The addition of
ApartmentNumberandNamefields aligns with the database schema updates and ensures clients receive complete plot ownership information.
72-72: LGTM! Consistent apartment number inclusion.Adding
ApartmentNumberto the plot expiry packet maintains consistency with the other packet methods.Maple2.Server.Game/GameServer.cs (1)
86-90: LGTM! Well-implemented session lookup by account ID.The new method follows the established pattern with proper thread safety using the existing mutex. This complements the character ID-based lookup and supports plot management features that operate at the account level.
Maple2.Model/Game/Cube/PlotCube.cs (2)
10-13: LGTM! Helpful clarification comment.The comment clearly distinguishes between PlotCube and HeldCube ID purposes, which will help prevent confusion during development.
30-36: Clockwise rotation logic is correct
In our coordinate system (e.g. Unity’s), positive angles rotate counter-clockwise. Adding 270° (equivalent to –90°) for a “clockwise” spin and +90° for counter-clockwise matches that convention—no changes needed.Maple2.Server.Game/Session/GameSession.cs (3)
56-56: LGTM! Property design follows good encapsulation practices.The
Channelproperty with private setter provides controlled access while maintaining the public accessibility pattern consistent with this class design.
145-145: Correct channel assignment from migration response.The assignment properly sets the session's channel for multi-channel coordination.
657-657: Well-coordinated conditional logic for planner modes.The conditional RoomId assignment correctly differentiates between normal and planner modes, aligning with the HomeFieldManager's cube clearing logic when roomId is -1.
Maple2.Server.Game/Manager/Items/ItemManager.cs (2)
16-16: Good refactor to enable dynamic furnishing management.Converting from readonly field to property with private setter properly enables reassignment while maintaining encapsulation.
27-31: Well-implemented furnishing reloading method.The method correctly uses
usingfor database context disposal, creates a fresh FurnishingManager instance, and calls Load() to update the client. Resource management and functionality are properly implemented.Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs (1)
670-670: Correct placement of function cube broadcast.The broadcast is properly positioned after adding the entity to the field collection, ensuring state consistency before client notification. This aligns well with the PR's plot state broadcasting objectives.
Maple2.Server.Game/Manager/Field/FieldManager/HomeFieldManager.cs (3)
13-13: Appropriate field addition for roomId storage.The readonly field properly stores the roomId parameter for use throughout the class lifecycle.
15-19: Well-implemented constructor enhancement.The roomId parameter is properly added and assigned to the field, enabling the planner mode functionality integration.
26-29: Correct implementation of planner mode cube clearing.The conditional logic properly clears cubes for planner modes (roomId == -1), providing a clean slate for planning activities. The comment clearly explains the intent.
Maple2.Model/Game/User/PlotInfo.cs (3)
57-59: LGTM!The
ToString()implementation follows the codebase patterns and provides useful debugging information.
36-48: Plot state logic verified — no changes required
- Maple2.Model/Game/User/PlotInfo.cs:
Statenow correctly treats anyExpiryTime > 0as active (Taken/Pending) and all non‐positive values as Open.- Maple2.Server.World/WorldServer.cs:
FieldPlotExpiryCheckcomparesExpiryTimetoDateTimeOffset.UtcNow.ToUnixTimeSeconds(), marking expired plots Pending/Open in the database.No further adjustments needed.
53-55: Preserving cubes on entering planner mode is intentionalThe
SetPlannerModechange inMaple2.Model/Game/User/PlotInfo.csnow only updatesPlotMode, preserving the existingCubescollection in the client model. Server-side logic still clears cubes where needed—e.g.:
- In
FieldManager.Ugc.cs(line 86):plot.Cubes.Clear()- In
HomeFieldManager.cs(line 28):plot.Cubes.Clear()- In
RequestCubeHandler.HandleLoadLayout(lines 603–606): enforces an empty cube list before loading layoutsThis behavior supports the PR’s planner/blueprint editing improvements by retaining cubes on the client while ensuring server-side layout loads still require a cleared state.
Maple2.Server.Game/Service/ChannelService.UpdateFieldPlot.cs (2)
73-150: Well-structured block update handling!The implementation properly validates inputs, manages cube lifecycle, handles interactions, and sends appropriate packets for each operation type. The error logging is comprehensive and will help with debugging.
65-70: Check session availability before sending forfeit confirmation.The
sessionvariable might be null if the player is offline (lines 40-48), but the forfeit confirmation is sent unconditionally at line 68.if (request.Forfeit) { if (fieldManager.Plots.TryGetValue(plotNumber, out Plot? plot)) { fieldManager.Broadcast(CubePacket.ForfeitPlot(plot)); - session?.Send(CubePacket.ConfirmForfeitPlot(plot)); + if (session != null) { + session.Send(CubePacket.ConfirmForfeitPlot(plot)); + } } }⛔ Skipped due to learnings
Learnt from: AngeloTadeucci PR: AngeloTadeucci/Maple2#236 File: Maple2.Server.Game/PacketHandlers/RequestCubeHandler.cs:375-422 Timestamp: 2024-09-16T07:50:30.281Z Learning: In `RequestCubeHandler`, the `HandleLoadLayout` method is responsible for applying the layout to the plot after missing cubes are calculated in `HandleRequestLayout`.Maple2.Model/Game/User/Home.cs (1)
31-41: LGTM!Moving field initializations inline is a good practice in modern C# and improves code readability.
Maple2.Server.Game/PacketHandlers/RequestCubeHandler.cs (2)
219-219: Good addition of null checksThe early return null checks for
session.Fieldprevent potential null reference exceptions.Also applies to: 253-253, 293-293
232-244: Consistent implementation of plot update notificationsThe addition of
FieldPlotRequestupdates for cube operations on outdoor plots ensures proper synchronization across channels. The pattern is consistently applied across remove, rotate, and replace operations.Also applies to: 269-283, 314-324
Maple2.Database/Storage/Game/GameStorage.Map.cs (2)
247-270: Well-implemented cube creation with proper error handling.The method correctly handles exceptions, logs errors, and manages nurturing metadata when present.
134-145: Add early return after null check.The method continues execution even when
ugcMapis null, which could lead to unexpected behavior.Apply this diff:
public void SetPlotPending(long plotId) { // Set expiry time to now UgcMap? ugcMap = Context.UgcMap.Find(plotId); - if (ugcMap == null) return; + if (ugcMap == null) { + return; + } ugcMap.ExpiryTime = DateTimeOffset.UtcNow; ugcMap.OwnerId = 0; ugcMap.Name = string.Empty; Context.UgcMap.Update(ugcMap); Context.TrySaveChanges(); }Likely an incorrect or invalid review comment.
Maple2.Server.Game/Manager/HousingManager.cs (4)
43-74: Good integration with world update notifications.The method now properly saves plot names to the database and notifies other channels of the update.
621-661: Excellent error handling and cleanup logic.The method properly cleans up created cubes from the database if subsequent operations fail, ensuring data consistency.
206-212: Consistent world update notifications across plot operations.All plot operations (buy, forfeit, extend) now properly notify the world service of changes, ensuring synchronization across channels.
Also applies to: 248-257, 307-312
722-738: Good resilience with null checks and warning logs.The methods now handle missing shop entries and item metadata gracefully by logging warnings and continuing, rather than failing the entire operation.
Also applies to: 782-786
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.Ugc.cs (1)
175-194: Well-implemented plot update methodThe
UpdateAllPlotsmethod properly handles concurrent access with appropriate locking and provides comprehensive plot state synchronization.Maple2.Server.World/WorldServer.cs (1)
38-55: Constructor properly integrates ItemMetadataStorage dependencyThe dependency injection and initialization of the plot expiry check are implemented correctly.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores