Feat: Death & Revive - #410
Conversation
WalkthroughThis pull request overhauls the game’s death and revival mechanics across multiple modules. Legacy death metrics have been removed from character configurations and replaced with an instant revival counter. Updates span database models, file ingest, game storage, player state management, packet protocols, and service layers. New enumerations, properties, methods, and packet fields have been introduced to standardize death penalty calculations, tombstone handling, and revival procedures during gameplay. Changes
Sequence Diagram(s)sequenceDiagram
participant P as Player
participant FP as FieldPlayer
participant CM as ConfigManager
participant GS as GameSession
participant RP as RevivalPacket
participant TS as Tombstone
P->>FP: Triggers death event
FP->>TS: Create Tombstone and update death state
FP->>CM: Notify death and increment death count
CM->>GS: Update death penalty and revival info
GS->>P: Send updated DeathInfo via RevivalPacket
P->>GS: Request instant revival
GS->>CM: Process revival request (SetDeathPenalty/Revive)
CM-->>GS: Return updated revival count
GS->>FP: Revive player and reset tombstone
Possibly related PRs
Suggested labels
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: 4
🔭 Outside diff range comments (1)
Maple2.Database/Storage/Game/GameStorage.User.cs (1)
312-389: 💡 Verification agent🧩 Analysis chain
Verify all references to death metrics are updated
The changes correctly implement the shift from death tracking to revival tracking. However, ensure that all other references to the old death metrics throughout the codebase have been updated accordingly.
🏁 Script executed:
#!/bin/bash # Search for any remaining references to DeathTick or DeathCount in the codebase echo "Searching for remaining DeathTick references..." rg "DeathTick" --type csharp echo "Searching for remaining DeathCount references..." rg "DeathCount" --type csharp echo "Checking for any GameStorage method calls with old parameter names..." rg "SaveCharacterConfig\(.*death(Tick|Count)" --type csharpLength of output: 2117
Action Required: Update Remaining DeathCount References
The changes in the database storage module correctly implement the shift from death tracking to revival tracking. However, the grep results indicate that several parts of the codebase still reference the old death metric "DeathCount." In particular, please review and update the following files as needed:
- Maple2.Server.Game/PacketHandlers/HomeDoctorHandler.cs – References to
session.Config.DeathCount- Maple2.Server.Game/PacketHandlers/ResolveDeathPenaltyHandler.cs – References to
session.Config.DeathCount- Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs – Multiple references including
Session.Config.DeathCount- Maple2.Server.Game/Manager/Config/ConfigManager.cs – Various occurrences setting or incrementing
DeathCount- Maple2.Model/Enum/Dungeon.cs and Maple2.Server.Game/Session/GameSession.cs – Additional references to
DeathCountPlease update or confirm these references to align them with the new revival tracking approach.
🧹 Nitpick comments (20)
Maple2.File.Ingest/Program.cs (2)
190-221: Disable Legacy Mapper Updates for Death Penalties and Related Data
The block comment from lines 190 to 221 disables a series of legacyUpdateDatabasecalls (e.g., usingAiMapper,AdditionalEffectMapper,AnimationMapper, etc.). This appears to be an intentional removal aligned with the PR objective of removing death penalty-related processing and shifting toward the new death & revival mechanics.
• Action: Please confirm that the removal of these mappers is coordinated with changes in related modules (e.g., updates inCharacterConfig.csand adjustments in game session handling inGameStorage.User.cs). If these mappers are no longer needed, consider eventually removing the commented-out code to keep the codebase clean.
223-223: Review Commented-Out Reporting Call
The comment on line 223 (//mapDataMapper.ReportStats();) indicates that the statistical reporting for map data is now disabled. Make sure that this deactivation is intentional as part of the refactoring; if diagnostic or logging functionality for map data remains necessary under the revised death and revival handling, ensure that an appropriate alternative is implemented.Maple2.Model/Metadata/Constants.cs (3)
931-932: Consider using PascalCase for these constants to maintain consistency.The naming convention for these new constants uses camelCase (
maxDeadCount,hitPerDeadCount), which is inconsistent with the PascalCase naming convention used for other constants in this file (e.g.,UserRevivalPaneltyTick,UserRevivalPaneltyMinLevel).-public const int maxDeadCount = 3; -public const byte hitPerDeadCount = 5; +public const int MaxDeadCount = 3; +public const byte HitPerDeadCount = 5;
931-932: Consider renaming for clarity within the Death & Revival system.These constants are a key part of the new Death & Revival system implementation. The terms "Dead" vs "Death" should be consistent with the rest of the system for better understanding.
If the system generally refers to "Death" rather than "Dead" in other parts of the code, consider:
-public const int maxDeadCount = 3; -public const byte hitPerDeadCount = 5; +public const int MaxDeathCount = 3; +public const byte HitPerDeathCount = 5;
931-932: Document the purpose of these constants.These constants are critical to the new Death & Revival system but lack documentation explaining their purpose and usage.
Consider adding XML comments to explain what these constants represent:
+/// <summary> +/// Maximum number of times a player can die before maximum penalties are applied. +/// </summary> public const int maxDeadCount = 3; +/// <summary> +/// Number of hit points associated with each death count. +/// </summary> public const byte hitPerDeadCount = 5;Maple2.Server.Game/Model/Field/Tombstone.cs (2)
8-26: The Tombstone class looks well-structured, but could benefit from XML documentation.The class design looks good with clear responsibilities and encapsulation. It handles the state of a player's tombstone and broadcasts updates appropriately. A few suggestions:
- Consider adding XML documentation to explain the purpose of this class and its key properties.
- Rename
Unknown1andUnknown2to more descriptive names based on their purposes (from the comments in theWriteTomethod, they appear to beHitByUserandRevivedByPet).namespace Maple2.Server.Game.Model; +/// <summary> +/// Represents a player's tombstone after death, tracking the number of hits needed for revival. +/// </summary> public class Tombstone : IByteSerializable { public readonly FieldPlayer Owner; public int ObjectId => Owner.ObjectId; private byte hitsRemaining; + /// <summary> + /// Gets or sets the remaining hits needed before the tombstone is cleared. + /// Setting this value broadcasts the updated state to all players in the field. + /// </summary> public byte HitsRemaining { get => hitsRemaining; set { if (hitsRemaining == 0) { return; } hitsRemaining = Math.Clamp(value, (byte) 0, TotalHitCount); Owner.Field.Broadcast(RevivalPacket.Tombstone(this)); } } public byte TotalHitCount { get; } - public int Unknown1 { get; } = 1; - public bool Unknown2 { get; } + public int HitByUser { get; } = 1; // Indicates if hit by a user + public bool RevivedByPet { get; } // Indicates if revived by pet
32-38: The WriteTo method should reflect the renamed properties if you implement the suggested changes.The serialization looks correct, but the comments indicate the true purpose of the "Unknown" properties. Make sure to update the method if you rename the properties.
public void WriteTo(IByteWriter writer) { writer.WriteInt(ObjectId); writer.WriteByte(HitsRemaining); writer.WriteByte(TotalHitCount); - writer.WriteInt(Unknown1); // 1 if hit by a user? - writer.WriteBool(Unknown2); // true if revived by pet + writer.WriteInt(HitByUser); // 1 if hit by a user + writer.WriteBool(RevivedByPet); // true if revived by pet }Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (1)
300-347:Revivemethod is well-structured, with an optional instant revival.• Checking both
IsDeadandTombstonebefore reviving prevents miscalls.
• Restoring full health and clearing death flags are appropriately handled.
• Applying death penalty and updating conditions match the new game mechanics.
• Broadcasting revival packets is consistent with your networking flow.
• Optional suggestion: Consider settingTombstone = null;after line 323 to remove any stale references, though settingHitsRemaining = 0is functionally sufficient in your workflow.Maple2.Server.Game/Util/ConditionUtil.cs (2)
164-164: Fix typo in condition nameThere's a spelling error in the condition name - "panelty" should be "penalty".
- case ConditionType.resolve_panelty: + case ConditionType.resolve_penalty:
296-296: Match spelling correction in CheckTarget methodIf you correct the spelling in the CheckCode method, ensure you also update it here for consistency.
- case ConditionType.resolve_panelty: + case ConditionType.resolve_penalty:Maple2.Database/Storage/Game/GameStorage.ServerInfo.cs (1)
38-38: Consider updating the TODO comment about death counter.Since this PR implements death tracking with the new
InstantRevivalCountfield, the TODO comment on line 38 about "Death counter" might be outdated and could be removed or updated to reflect the current implementation.Maple2.Server.Game/PacketHandlers/HomeDoctorHandler.cs (1)
25-26: Fix spacing in cost calculation assignment.There's a missing space after the equals sign in the cost calculation.
-int cost =session.Field.Lua.CalcResolvePenaltyPrice((ushort) session.Player.Value.Character.Level, session.Config.DeathCount, 0); +int cost = session.Field.Lua.CalcResolvePenaltyPrice((ushort) session.Player.Value.Character.Level, session.Config.DeathCount, 0);Maple2.Server.Game/PacketHandlers/RevivalHandler.cs (1)
55-57: Add error notification for failed voucher consumption.Currently, the code silently returns when voucher consumption fails. Consider sending an error notification to the player to improve user experience.
if (!session.Item.Inventory.Consume([new IngredientInfo(ItemTag.FreeReviveCoupon, 1)])) { - // Send error packet? + session.Send(NoticePacket.Notice(NoticePacket.Flags.Alert | NoticePacket.Flags.Message, StringCode.s_item_not_enough)); return; }Maple2.Server.Game/PacketHandlers/ResolveDeathPenaltyHandler.cs (1)
29-32: Penalty resolution implementationWhen all conditions are met, the handler:
- Deducts the Meso cost from the player's currency
- Resets the death penalty to zero via UpdateDeathPenalty(0)
- Updates the game condition with type "resolve_panelty"
This completes the death penalty resolution flow, aligning with the PR objective of implementing a death and revival system.
Note: There appears to be a typo in the condition type "resolve_panelty" (should be "resolve_penalty").
-session.ConditionUpdate(ConditionType.resolve_panelty); +session.ConditionUpdate(ConditionType.resolve_penalty);Maple2.Server.Game/Session/GameSession.cs (1)
258-258: Consider validating negativeMsRemaining.When calling
Config.SetDeathPenalty(configResponse.DeathInfo, currentTick);, ensure thatMsRemainingis non-negative to avoid unintentionally setting an already-expired penalty. It may be helpful to clamp negative values or treat them as an immediate penalty expiration to prevent potential edge cases.Maple2.Server.Game/Manager/Config/ConfigManager.cs (3)
30-30: Consider clarifying tick-based naming.
public long DeathPenaltyEndTick;is descriptive, but adding “ExpireTick” or a comment indicating the tick/time unit can reduce confusion.
312-339: Ensure consistent death penalty expiration handling.
UpdateDeathPenaltychecks for expired penalties by comparing ticks and resetsDeathCountif expired. Consider unifying any partial-expiration checks with logic inLoadRevival()or the retrieval logic fromPlayerConfigLookUpfor consistency.
341-344: ValidatedeathInfo.MsRemainingwhen setting penalty.
SetDeathPenaltyunconditionally calculatesDeathPenaltyEndTick = currentTick + deathInfo.MsRemaining;. Consider verifying or clamping negativeMsRemainingto prevent nonsensical penalty end times.Maple2.Server.World/Containers/PlayerConfigLookUp.cs (2)
56-61: Overwrite logic for death info.When saving new
DeathInfo, this code overwrites any existing record. This is typically fine, but ensure overwriting is always desired rather than merging or ignoring older data.
68-69: Minor naming improvement.Retrieving
DeathInfoasdeathis fine, though consistently naming local variables asdeathInfocould improve readability in line with other code usage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
Maple2.Server.World/Migrations/20250406064924_DeathCount.Designer.csis excluded by!Maple2.Server.World/Migrations/*Maple2.Server.World/Migrations/20250406064924_DeathCount.csis excluded by!Maple2.Server.World/Migrations/*Maple2.Server.World/Migrations/Ms2ContextModelSnapshot.csis excluded by!Maple2.Server.World/Migrations/*
📒 Files selected for processing (36)
Maple2.Database/Model/CharacterConfig.cs(2 hunks)Maple2.Database/Storage/Game/GameStorage.ServerInfo.cs(1 hunks)Maple2.Database/Storage/Game/GameStorage.User.cs(4 hunks)Maple2.File.Ingest/Maple2.File.Ingest.csproj(1 hunks)Maple2.File.Ingest/Mapper/MapMapper.cs(1 hunks)Maple2.File.Ingest/Mapper/ServerTableMapper.cs(3 hunks)Maple2.File.Ingest/MapperExtensions.cs(1 hunks)Maple2.File.Ingest/Program.cs(2 hunks)Maple2.Model/Enum/Map.cs(1 hunks)Maple2.Model/Game/User/Character.cs(0 hunks)Maple2.Model/Metadata/BeginCondition.cs(1 hunks)Maple2.Model/Metadata/Constants.cs(1 hunks)Maple2.Model/Metadata/MapMetadata.cs(1 hunks)Maple2.Model/Metadata/ServerTable/JobConditionTable.cs(1 hunks)Maple2.Model/Metadata/ServerTable/ScriptConditionTable.cs(1 hunks)Maple2.Server.Core/proto/world/world.proto(3 hunks)Maple2.Server.Game/Commands/KillCommand.cs(2 hunks)Maple2.Server.Game/Manager/Config/BuffManager.cs(2 hunks)Maple2.Server.Game/Manager/Config/ConfigManager.cs(5 hunks)Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs(2 hunks)Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.cs(1 hunks)Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs(5 hunks)Maple2.Server.Game/Model/Field/Buff.cs(1 hunks)Maple2.Server.Game/Model/Field/Tombstone.cs(1 hunks)Maple2.Server.Game/PacketHandlers/HomeDoctorHandler.cs(2 hunks)Maple2.Server.Game/PacketHandlers/ResolveDeathPenaltyHandler.cs(1 hunks)Maple2.Server.Game/PacketHandlers/RevivalHandler.cs(2 hunks)Maple2.Server.Game/PacketHandlers/TombstoneHandler.cs(1 hunks)Maple2.Server.Game/Packets/DeadUserPacket.cs(1 hunks)Maple2.Server.Game/Packets/RevivalPacket.cs(1 hunks)Maple2.Server.Game/Session/GameSession.cs(4 hunks)Maple2.Server.Game/Util/ConditionUtil.cs(2 hunks)Maple2.Server.Game/Util/NpcTalkUtil.cs(2 hunks)Maple2.Server.Game/Util/SkillUtils.cs(1 hunks)Maple2.Server.World/Containers/PlayerConfigLookUp.cs(3 hunks)Maple2.Server.World/Service/WorldService.PlayerConfig.cs(2 hunks)
💤 Files with no reviewable changes (1)
- Maple2.Model/Game/User/Character.cs
🧰 Additional context used
🧬 Code Definitions (13)
Maple2.Server.Game/Packets/DeadUserPacket.cs (3)
Maple2.Server.Game/PacketHandlers/HomeDoctorHandler.cs (1)
ByteWriter(38-43)Maple2.Server.Game/Packets/RevivalPacket.cs (4)
ByteWriter(10-17)ByteWriter(19-24)ByteWriter(26-32)ByteWriter(34-39)Maple2.Server.Core/Packets/Packet.cs (1)
Packet(7-16)
Maple2.Server.Game/PacketHandlers/TombstoneHandler.cs (4)
Maple2.Server.Game/Session/GameSession.cs (3)
GameSession(37-802)GameSession(105-115)GameSession(695-695)Maple2.Server.Game/Model/Field/Tombstone.cs (2)
Tombstone(8-39)Tombstone(27-31)Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.cs (2)
TryGetPlayer(377-379)TryGetPlayer(381-384)Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (2)
FieldPlayer(15-486)FieldPlayer(67-80)
Maple2.Server.Game/PacketHandlers/RevivalHandler.cs (4)
Maple2.Server.Game/Session/GameSession.cs (3)
GameSession(37-802)GameSession(105-115)GameSession(695-695)Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (1)
Revive(304-347)Maple2.Server.Core/Packets/NoticePacket.cs (1)
NoticePacket(9-77)Maple2.Model/Game/User/Player.cs (1)
Currency(45-59)
Maple2.Server.Game/PacketHandlers/ResolveDeathPenaltyHandler.cs (4)
Maple2.Server.Game/Session/GameSession.cs (4)
GameSession(37-802)GameSession(105-115)GameSession(695-695)ConditionUpdate(524-527)Maple2.Server.Game/PacketHandlers/HomeDoctorHandler.cs (1)
Handle(15-36)Maple2.Model/Game/User/Player.cs (1)
Currency(45-59)Maple2.Server.Game/Manager/Config/ConfigManager.cs (1)
UpdateDeathPenalty(316-339)
Maple2.Server.Game/Packets/RevivalPacket.cs (1)
Maple2.Server.Game/Model/Field/Tombstone.cs (2)
Tombstone(8-39)Tombstone(27-31)
Maple2.Server.Game/Model/Field/Buff.cs (1)
Maple2.Server.Game/Manager/Config/BuffManager.cs (1)
UpdateEnabled(436-440)
Maple2.Server.Game/Commands/KillCommand.cs (3)
Maple2.Server.Game/Session/GameSession.cs (3)
GameSession(37-802)GameSession(105-115)GameSession(695-695)Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (3)
FieldPlayer(15-486)FieldPlayer(67-80)Update(121-170)Maple2.Server.Game/Packets/StatsPacket.cs (1)
StatsPacket(10-101)
Maple2.Server.Game/Model/Field/Tombstone.cs (4)
Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (2)
FieldPlayer(15-486)FieldPlayer(67-80)Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.cs (1)
Broadcast(606-615)Maple2.Server.Game/Packets/RevivalPacket.cs (1)
RevivalPacket(9-40)Maple2.Model/Metadata/Constants.cs (1)
Constant(10-939)
Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (6)
Maple2.Server.Game/Model/Field/Tombstone.cs (2)
Tombstone(8-39)Tombstone(27-31)Maple2.Server.Game/Model/Field/Actor/Actor.cs (2)
Update(200-211)OnDeath(235-235)Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementState.cs (1)
Update(184-229)Maple2.Server.Game/Packets/DeadUserPacket.cs (1)
DeadUserPacket(7-15)Maple2.Server.Game/Manager/Config/ConfigManager.cs (1)
UpdateDeathPenalty(316-339)Maple2.Server.Game/Packets/RevivalPacket.cs (1)
RevivalPacket(9-40)
Maple2.Server.Game/Session/GameSession.cs (2)
Maple2.Server.Game/Manager/Config/ConfigManager.cs (3)
SetDeathPenalty(341-344)LoadRevival(303-310)AddInstantReviveCount(346-354)Maple2.Server.World/Containers/PlayerConfigLookUp.cs (1)
DeathInfo(128-142)
Maple2.Server.World/Containers/PlayerConfigLookUp.cs (1)
Maple2.Database/Extensions/DateTimeExtensions.cs (1)
ToEpochSeconds(6-12)
Maple2.Server.Game/Manager/Config/ConfigManager.cs (6)
Maple2.Database/Storage/Game/GameStorage.User.cs (3)
Player(196-264)Character(74-92)Character(447-457)Maple2.Model/Game/User/Player.cs (2)
Player(8-22)Player(17-21)Maple2.Server.Game/Packets/RevivalPacket.cs (1)
RevivalPacket(9-40)Maple2.Model/Game/User/Character.cs (1)
Character(7-53)Maple2.Model/Metadata/Constants.cs (1)
Constant(10-939)Maple2.Server.World/Containers/PlayerConfigLookUp.cs (1)
DeathInfo(128-142)
Maple2.Database/Storage/Game/GameStorage.User.cs (2)
Maple2.Database/Model/CharacterConfig.cs (3)
SkillPoint(133-137)SkillBook(128-131)CharacterConfig(9-58)Maple2.Model/Game/User/SkillPoint.cs (2)
SkillPoint(10-63)SkillPoint(22-30)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (78)
Maple2.Server.Game/Packets/DeadUserPacket.cs (1)
7-15: Well-implemented packet creation for player death events!The implementation follows the established pattern for packet creation in the codebase. The
Deadmethod properly constructs a packet with theDeadUseroperation code and writes the required data (player object ID and tombstone appearance flag). This aligns well with the PR objective of implementing a death system for players.Maple2.File.Ingest/Maple2.File.Ingest.csproj (1)
22-22: Package Reference Update VerificationThe package reference for
Maple2.File.Parser.Tadeuccihas been updated from version2.2.4to2.2.6. This upgrade should provide any recent improvements or bug fixes required for the new death and revival mechanics. Please ensure that this newer version does not introduce any breaking changes with respect to the game’s existing functionalities. It would be helpful to review the package release notes and run integration tests if available.Maple2.Server.Game/Model/Field/Tombstone.cs (1)
27-31: Good implementation of the constructor with appropriate use of game constants.The constructor properly initializes the tombstone with the correct hit count based on the player's death statistics, using game constants to determine the maximum values.
Maple2.Server.Game/Packets/RevivalPacket.cs (4)
1-1: LGTM. Additional import seems appropriate.The additional import of
Maple2.Tools.Extensionsis necessary for the new functionality in this class, particularly for theWriteClass<Tombstone>method used in the newTombstonemethod.Also applies to: 5-5
10-17: Appropriate transformation of revival penalty handling.The
UpdatePenaltymethod effectively replaces the previousConfirmmethod, changing from an actor-based approach to a more direct parameter-based one. This aligns well with the PR objectives of implementing a Death and Revival system.
19-19: LGTM. More descriptive method name.Renaming from
CounttoRevivalCountmakes the method's purpose clearer and more consistent with the revival-related terminology used throughout the PR.
34-39: Good integration with the new Tombstone class.The
Tombstonemethod properly integrates with the newTombstoneclass defined in the related file, facilitating the broadcasting of tombstone state changes to clients. This is a key part of the new death and revival system.Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (8)
3-3: Imports look good.The newly added namespaces (
Maple2.Model.ErrorandMaple2.Model.Metadata) are used later for error handling (e.g.,MigrationError) and metadata checks in this file, so these additions appear necessary and correct.Also applies to: 5-5
22-31: State property update logic is clear.By comparing the existing
stateagainst the newvalue, the setter avoids redundant flag updates, which helps reduce unnecessary state synchronization events. This is a clean approach that improves efficiency with minimal overhead.
32-40: OverriddenIsDeadproperty is well-encapsulated.Defining
isDeadas a private field with a protected setter ensures that only the class and its subclasses can toggle the death state. Updating theDeadflag automatically on state change is concise and consistent with the existing flag mechanism.
56-56: NewTombstoneproperty is straightforward.Allowing
Tombstoneto be nullable provides flexibility. The code checks fornullbefore attempting any tombstone operations, preventing null pointer errors.
122-122:base.Update(tickCount)call maintains the essential parent logic.Invoking the base class
Updateensures core actor updates (like death checks inActor) remain consistent before applying custom player logic. This ordering is appropriate and avoids missing base behaviors.
130-139: Automatic revival check aligns with new death mechanics.This block ensures that if the player is flagged
IsDeadand the tombstone runs out of hits, revival occurs automatically. For manual revival flows, ensure the wider game logic accommodates them, but for typical “auto-revive on zero hits,” this is correct.
140-143: Battle state timeout is clearly managed.Marking the player as out of battle after 2 seconds of inactivity is a straightforward, low overhead approach. If more nuanced battle durations are required, consider making the timeout configurable. Nevertheless, this logic is fine as is.
289-298:OnDeathoverride properly handles death flow.Stopping crafting, resetting held items, creating the tombstone, and broadcasting death ensures clarity in the game’s logic. This neatly coordinates with the rest of the new tombstone-based system.
Maple2.Model/Metadata/BeginCondition.cs (1)
16-16: Good addition for death state controlThe new
AllowDeadproperty improves the condition system by explicitly tracking whether actions can be performed in a dead state, aligning with the death and revival system implementation.Maple2.Server.Game/Util/ConditionUtil.cs (2)
162-164: New condition types for death & revival systemThe addition of these three conditions appropriately extends the condition system to handle the new death and revival mechanics.
294-296: Consistent implementation of condition typesThese conditions are consistently implemented between the CheckCode and CheckTarget methods, which is good practice.
Maple2.Model/Metadata/MapMetadata.cs (1)
52-52: Improved type safety with enumChanging from
inttoAutoReviveTypeenum improves type safety and code readability, making the intention of this field clearer. This is a good practice when dealing with fields that have a limited set of valid values.Maple2.File.Ingest/Mapper/MapMapper.cs (1)
55-55: Consistent type conversion for AutoReviveTypeThe explicit casting to
AutoReviveTypeproperly aligns with the type change in theMapMetadataPropertyclass, maintaining consistency across the codebase.Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.cs (1)
391-397: Good addition of random spawn point selection!This enhancement to the
TryGetPlayerSpawnmethod allows for randomly selecting an enabled spawn point when given a negative ID. This functionality is essential for the new death and revival system, providing a clean way to respawn players at random locations.Maple2.Model/Metadata/ServerTable/JobConditionTable.cs (1)
29-29: Added DeathPenalty property to support new featureThe addition of this boolean property aligns with the PR objectives regarding death penalties. This properly extends the JobConditionMetadata to track whether a job condition involves a death penalty.
Maple2.Model/Metadata/ServerTable/ScriptConditionTable.cs (1)
24-24: Added DeathPenalty property to support death mechanicsThis addition allows script conditions to know about and potentially modify death penalty behavior, consistent with the PR objectives. The property fits well within the existing structure of the ScriptConditionMetadata record.
Maple2.Model/Enum/Map.cs (1)
51-55: Great addition of AutoReviveType enumThe new AutoReviveType enum improves type safety and clearly documents the available auto-revival behaviors. The comment for the Countdown option provides helpful context about its specific use case with map 65000003 (Treasure Island).
Maple2.Database/Storage/Game/GameStorage.ServerInfo.cs (1)
35-35: Good addition to support the new revival system.This line resets the
InstantRevivalCountto 0 during daily resets, which aligns with the death and revival metrics implemented in this PR. This ensures players receive a fresh count of instant revivals each day.Maple2.Server.Game/Manager/Config/BuffManager.cs (2)
412-419: Good flow control improvement in OnDeath method.The added
continuestatement ensures that only buffs that should be kept after death have their enabled state updated. This prevents unnecessary processing of buffs that have already been removed.
436-440: Good addition of centralized buff enabled state update method.The new
UpdateEnabledmethod provides a convenient way to refresh the enabled state of all buffs, which is useful when the player's state changes (like during revival). This method supports the death and revival system being implemented in this PR.Maple2.Server.Game/Util/NpcTalkUtil.cs (2)
134-136: Good implementation of death penalty check for job conditions.This condition prevents players with an active death penalty from satisfying job-related NPC interaction conditions. This aligns with the PR's objective of implementing death penalties and the revival system.
220-222: Good implementation of death penalty check for script conditions.This condition prevents players with an active death penalty from satisfying script-related NPC interaction conditions. It maintains consistency with the job condition check implementation and supports the death penalty system.
Maple2.File.Ingest/MapperExtensions.cs (1)
330-330: Good addition of AllowDead property to support dead state interactions.Adding the
AllowDeadproperty to theBeginConditionconstructor allows skills to specify whether they can be used while the player is in a dead state. This is essential for the death and revival system, particularly for revival-related abilities.Maple2.Server.Game/Util/SkillUtils.cs (1)
75-77: Approve: Death check for skill usage.The added condition ensures that skills cannot be used by dead players unless specifically allowed via the
AllowDeadproperty. This aligns with the new death and revival system implementation.Maple2.Server.Game/Model/Field/Buff.cs (1)
106-106: Visibility modifier change to support buff updates during death/revival.Making
UpdateEnabledpublic allows it to be called byBuffManager.UpdateEnabled(), which is necessary for updating buffs when a player dies or is revived.Maple2.Database/Model/CharacterConfig.cs (1)
22-22: Replaced death tracking with revival tracking.The shift from tracking death statistics (
DeathTickandDeathCount) to tracking revival statistics (InstantRevivalCount) aligns with the implementation of the new revival system. This will track how many times a player has been instantly revived.Maple2.Server.Game/PacketHandlers/TombstoneHandler.cs (1)
9-22: Well-structured handler for tombstone interaction.This new handler correctly processes tombstone interaction packets, reducing the hits remaining on a player's tombstone. The validation checks prevent errors by ensuring both the player and tombstone exist before making changes.
Maple2.Server.Game/PacketHandlers/HomeDoctorHandler.cs (2)
21-23: Good addition of early return check for death penalty expiration.This new check ensures that the home doctor only deals with active death penalties, preventing unnecessary processing when the death penalty has already expired.
31-32: Dynamic cost implementation for home doctor usage.The fixed cost has been replaced with a dynamic calculation based on the player's level and death count, making the system more flexible and balanced.
Maple2.Server.Game/Commands/KillCommand.cs (3)
17-17: Updated command description to include players.The description now accurately reflects the expanded functionality of the kill command to include players.
22-22: Added new command for killing players.Registered the new KillPlayerCommand to the command system, enabling admin users to target players.
108-137: Well-implemented KillPlayerCommand with appropriate validations.The command implementation includes:
- Proper validation for empty player names
- Efficient player lookup by name in the current field
- Direct health reduction to kill the player
- Appropriate user feedback based on action results
This complements the existing NPC/mob kill commands and follows the same pattern.
Maple2.File.Ingest/Mapper/ServerTableMapper.cs (3)
151-151: Added DeathPenalty property to ScriptConditionMetadata in NPC script conditions.This addition enables NPCs to have conditions based on player death penalties, aligning with the new death and revival system.
214-215: Added DeathPenalty property to ScriptConditionMetadata in quest script conditions.This properly extends the death penalty condition to quest scripts, ensuring consistent implementation across both NPC and quest-based conditions.
396-397: Added DeathPenalty property to JobConditionMetadata.This addition completes the implementation of death penalties across all relevant condition types, providing a consistent mechanism for gameplay logic related to death penalties.
Maple2.Server.Game/PacketHandlers/RevivalHandler.cs (6)
35-37: Added important revival restriction checks.These checks prevent revival in fields where it's not allowed or where no valid revival return point exists, avoiding potential issues with player state or positioning.
39-41: Improved player revival state management.Setting the tombstone to null after successful revival ensures proper cleanup of the death state and prevents potential state inconsistencies.
48-50: Consistent revival restriction check for instant revival.This check properly applies the same validation for instant revival as for safe revival, maintaining consistent behavior across revival methods.
54-60: Implemented revival voucher consumption logic.The code now properly handles:
- Checking if the player has a revival voucher
- Consuming the voucher if available
- Notifying the player of successful voucher use
This provides players with a premium option for revival that doesn't cost in-game currency.
61-72: Well-implemented meso-based revival with limits and dynamic cost.The implementation includes important checks:
- Daily limit on instant revivals
- Dynamic cost calculation based on player level
- Validation of player's meso balance
This prevents potential exploits while providing a currency sink in the game economy.
74-77: Proper tracking and state update for instant revival.Incrementing the revival count and updating the player state ensures accurate metrics and proper game state management.
Maple2.Server.Core/proto/world/world.proto (4)
537-537: Renamed oneof field for expanded functionalityThe oneof field has been renamed from "buff" to "player_config" to better represent its broader scope, which now includes death information along with buffs.
531-531: Added death_info field to PlayerConfigRequest.SaveThis adds support for saving death-related information along with buffs and skill cooldowns, supporting the new death and revival system as mentioned in the PR objectives.
546-546: Added death_info field to PlayerConfigResponseThis allows the server to send death-related information back to the client, ensuring death state synchronization across the game.
567-571: Added new DeathInfo message typeThe new DeathInfo message captures essential death state information with three key fields:
count: Tracks the number of deathsms_remaining: Time remaining on the death penalty in millisecondsstop_time: When the death penalty expiresThis implementation elegantly supports the death and revival tracking mentioned in the PR objectives.
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs (2)
86-89: Improved player spawn logicChanged the player spawn logic to use the
TryGetPlayerSpawnmethod instead of directly searching for a spawn point. This change enhances code reusability and consistency in handling spawn points.
778-779: Set player object flag on additionThis important addition sets the player's flag to
PlayerObjectFlag.Allwhen they're added to the field, ensuring that all player attributes and states (including death state mentioned in PR objectives) are properly initialized and visible to the game system.Maple2.Server.Game/PacketHandlers/ResolveDeathPenaltyHandler.cs (3)
10-12: New handler class for resolving death penaltiesThis new handler class processes requests to resolve death penalties, aligning with the PR objective of implementing a death and revival system.
13-21: Death penalty resolution validationThe handler validates that:
- The interaction is with a valid Doctor NPC (kind 81)
- The player has an active death penalty (DeathPenaltyEndTick hasn't passed)
This ensures players can only use appropriate NPCs to resolve penalties and prevents unnecessary processing when no penalty exists.
23-27: Cost calculation and currency checkThe handler calculates the penalty resolution cost based on:
- Player level
- Death count
- Field continent (Shadow World has a different calculation mode)
It then verifies the player has sufficient Meso before proceeding, preventing transaction failures.
Maple2.Server.World/Service/WorldService.PlayerConfig.cs (5)
8-9: Updated case statement to use PlayerConfigCaseRenamed the oneof case check from
BuffCasetoPlayerConfigCase, matching the proto file changes and reflecting the expanded scope beyond just buffs.
11-11: Updated case enum to use PlayerConfigOneofCaseUpdated the case enum name to match the proto file changes, ensuring consistency.
19-19: Added DeathInfo to the configuration retrieval resultUpdated the return tuple from
playerConfigLookUp.Retrieveto includeDeathInfo death, allowing the service to retrieve death-related information alongside buffs and skill cooldowns.
41-42: Added DeathInfo to the PlayerConfigResponseIncluded the retrieved death information in the response, ensuring it's sent back to the requesting client. This allows clients to synchronize death state data.
46-46: Added DeathInfo to the configuration save methodUpdated the
playerConfigLookUp.Savecall to includesave.DeathInfo, allowing the service to persist death-related information alongside buffs and skill cooldowns.Maple2.Server.Game/Session/GameSession.cs (2)
445-445: No concerns with new revival loading logic.The call to
Config.LoadRevival();upon field entry helps ensure the revival mechanics are accurately initialized. This addition looks good.
604-604: Verify the daily reset logic forInstantReviveCount.Calling
Config.AddInstantReviveCount(-1);resets the revive count to 0 instead of reducing it by one. Confirm whether this is the intended behavior or if a decrement was needed.Maple2.Server.Game/Manager/Config/ConfigManager.cs (4)
32-32: Use of signed integer forInstantReviveCount.Tracking revives with an
intis acceptable; however, ensure that negative values don’t propagate through other code paths. The helper methods appear to reset to zero, so this is likely safe.
57-57: LoadingInstantReviveCountfrom DB.Including the instant revive count in the load is consistent with the new mechanic and ensures the in-game data remains accurate. No issues spotted.
303-310: Straightforward revival initialization.
LoadRevival()resets the death penalty if expired and updates the client. This centralizes revival logic effectively.
346-354: Clarify full reset vs. decrement logic.
AddInstantReviveCountzeroes outInstantReviveCountwhenever a negative value is passed. If the design requires partial decrements, you might want a separate method or a different approach to handle negative increments.Maple2.Server.World/Containers/PlayerConfigLookUp.cs (5)
14-14: Concurrent dictionary ensures thread safety.Introducing
private readonly ConcurrentDictionary<long, DeathInfo> deaths;is a good approach for concurrent access when tracking death data.
20-20: Dictionary initialization is consistent.
deaths = [];aligns with other initializations, maintaining consistency for storing death info.
23-23: Extended save method.Adding a
DeathInfoparameter toSaveproperly integrates death tracking with existing buff and skill-cooldown data. No issues observed.
64-64: Enhanced retrieval signature is comprehensive.Returning
DeathInfoalongside buffs and cooldowns improves the single retrieval call efficiency. Nicely done.
128-142: Correctly handles expired death records.
RetrieveDeathremoves an entry ifmsSurpassed > death.MsRemaining. This is a clean and straightforward approach to managing death info lifecycles.Maple2.Database/Storage/Game/GameStorage.User.cs (5)
312-314: Method signature update replacing death tracking with revival trackingThe method signature has been updated to replace
DeathTickandDeathCountwithInstantRevivalCount, which aligns with the PR objective of implementing a Death and Revival system for players.
317-317: Return value updated for revival countThe default return value has been properly updated to return
0for the newInstantRevivalCountparameter instead of the removed death-related parameters.
347-347: Return value assignment revised for instant revival countThe return statement now correctly includes
config.InstantRevivalCountinstead of the previous death-related properties, maintaining consistency with the database model changes.
367-367: Method parameter updated for revival trackingThe
SaveCharacterConfigmethod parameter has been updated to useinstantRevivalCountinstead of the previous death-related parameters, maintaining API consistency with the loading method.
389-389: Config property assignment updatedThe property assignment has been correctly updated to store the new
instantRevivalCountparameter in theInstantRevivalCountproperty of the config object.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
Maple2.Server.Game/PacketHandlers/HomeDoctorHandler.cs (1)
33-34:⚠️ Potential issueFix typo in condition type name.
There's a typo in the condition type name: "panelty" should be "penalty".
-session.ConditionUpdate(ConditionType.resolve_panelty); +session.ConditionUpdate(ConditionType.resolve_penalty);Maple2.Server.Game/Commands/KillCommand.cs (1)
108-141: 🛠️ Refactor suggestionImplement a dedicated Kill method in FieldPlayer class.
The KillPlayerCommand correctly checks if the player is already dead before attempting to kill them, which is good. However, instead of directly manipulating the player's health, it would be better to encapsulate this logic in a dedicated method in the FieldPlayer class. This would follow the Don't Repeat Yourself (DRY) principle and provide a consistent way to kill players across the codebase.
Consider implementing a Kill method in the FieldPlayer class:
// Add to FieldPlayer class /// <summary> /// Kills the player by setting their health to zero if they aren't already dead. /// </summary> /// <returns>True if the player was killed, false if they were already dead.</returns> public bool Kill() { // Check if player is already dead if (DeathState != DeathState.Alive) { return false; } // Set health to zero Stats.Values[BasicAttribute.Health].Add(-Stats.Values[BasicAttribute.Health].Current); Session.Send(StatsPacket.Update(this, BasicAttribute.Health)); return true; }Then update the KillPlayerCommand to use this method:
private void Handle(InvocationContext ctx, string name) { if (string.IsNullOrEmpty(name)) { ctx.Console.Out.WriteLine("Name cannot be empty."); return; } FieldPlayer? player = session.Field.GetPlayers().Values .FirstOrDefault(player => string.Equals(player.Value.Character.Name, name, StringComparison.OrdinalIgnoreCase)); if (player is null) { ctx.Console.Out.WriteLine($"Player {name} not found."); return; } if (player.DeathState != DeathState.Alive) { ctx.Console.Out.WriteLine($"Player {name} is already dead."); return; } - player.ConsumeHp((int)player.Stats.Values[BasicAttribute.Health].Current); + if (player.Kill()) { + ctx.Console.Out.WriteLine($"Player {name} has been killed."); + } }Maple2.Server.Game/Session/GameSession.cs (1)
789-793: Added death info persistenceAdded code to create and save a DeathInfo object with the current death count and remaining penalty time. This ensures death state is properly persisted between sessions.
Consider clamping the
MsRemainingvalue to ensure it's never negative:- MsRemaining = (int) (Config.DeathPenaltyEndTick - fieldTick), + var msRem = (int) (Config.DeathPenaltyEndTick - fieldTick); + MsRemaining = msRem > 0 ? msRem : 0;
🧹 Nitpick comments (5)
Maple2.Model/Game/User/PlayerInfo.cs (1)
66-67: Consider initializing DeathState in constructors.The DeathState property has been added, but it's not explicitly initialized in the CharacterInfo constructors, which may lead to a default value of 0 (DeathState.Alive). While this might be the intended behavior, explicitly initializing it would improve code clarity.
public CharacterInfo(CharacterInfo other) { AccountId = other.AccountId; CharacterId = other.CharacterId; Name = other.Name; Motto = other.Motto; Picture = other.Picture; Gender = other.Gender; Job = other.Job; Level = other.Level; MapId = other.MapId; Channel = other.Channel; LastOnlineTime = other.LastOnlineTime; GuildId = other.GuildId; GuildName = other.GuildName; + DeathState = other.DeathState; }Maple2.Server.Game/Manager/Config/ConfigManager.cs (3)
29-36: Well-structured properties with direct character state access.The implementation of
DeathPenaltyEndTickandDeathCountas properties that directly access the player's Character properties provides good encapsulation. However,InstantReviveCountis inconsistent with this pattern - it's a public field without getters/setters that sync with the character model.Consider making
InstantReviveCountfollow the same pattern as the other death-related properties:-public int InstantReviveCount; +public int InstantReviveCount { + get => session.Player.Value.Character.InstantReviveCount; + set => session.Player.Value.Character.InstantReviveCount = value; +}
320-343: Comprehensive death penalty update logic with minor duplication.The method correctly implements death penalty mechanics with level checks and client notifications, but contains some duplicated reset logic.
Consider extracting the reset logic into a private helper method to avoid duplication:
public void UpdateDeathPenalty(long endTick) { // Skip penalty for low level players if (session.Player.Value.Character.Level < Constant.UserRevivalPaneltyMinLevel) { return; } // Reset penalty if endTick is 0 if (endTick == 0) { - DeathCount = 0; - DeathPenaltyEndTick = 0; + ResetDeathPenalty(); } // Otherwise update penalty else { // Reset count if previous penalty expired if (session.Field.FieldTick > DeathPenaltyEndTick) { DeathCount = 0; } DeathCount++; DeathPenaltyEndTick = endTick; } // Send update to client session.Send(RevivalPacket.UpdatePenalty(session.Player.ObjectId, (int) DeathPenaltyEndTick, DeathCount)); } +private void ResetDeathPenalty() { + DeathCount = 0; + DeathPenaltyEndTick = 0; +}
350-358: Unusual handling of negative revival count values.The method resets the count to zero when a negative value is provided instead of simply preventing it from going below zero.
Consider clarifying the intention with a more explicit approach:
public void AddInstantReviveCount(int count = 1) { - if (count < 0) { - InstantReviveCount = 0; - } else { - InstantReviveCount += count; - } + // If negative count is provided, reset to zero + if (count < 0) { + InstantReviveCount = 0; + } else { + InstantReviveCount += count; + } session.Send(RevivalPacket.RevivalCount(InstantReviveCount)); }Alternatively, if the intention is to prevent negative values:
public void AddInstantReviveCount(int count = 1) { - if (count < 0) { - InstantReviveCount = 0; - } else { - InstantReviveCount += count; - } + // Add count and ensure it doesn't go below zero + InstantReviveCount = Math.Max(0, InstantReviveCount + count); session.Send(RevivalPacket.RevivalCount(InstantReviveCount)); }Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (1)
389-427: Added server-side health state synchronization with debug message.The
RecoverHpandConsumeHpmethods now properly update the player's health state across the server systems, but there's a debugConsole.WriteLinestatement that might not be intended for production.Remove the debug console output in the
ConsumeHpmethod:-Console.WriteLine($"Consuming HP: {stat.Current} / {stat.Total}");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
Maple2.File.Ingest/Program.cs(1 hunks)Maple2.Model/Enum/Death.cs(1 hunks)Maple2.Model/Game/User/Character.cs(1 hunks)Maple2.Model/Game/User/IPlayerInfo.cs(1 hunks)Maple2.Model/Game/User/PlayerInfo.cs(2 hunks)Maple2.Server.Core/Sync/PlayerInfoUpdateEvent.cs(3 hunks)Maple2.Server.Core/Sync/PlayerInfoUpdateExtensions.cs(3 hunks)Maple2.Server.Core/proto/sync.proto(2 hunks)Maple2.Server.Game/Commands/KillCommand.cs(2 hunks)Maple2.Server.Game/Manager/Config/ConfigManager.cs(4 hunks)Maple2.Server.Game/Manager/PartyManager.cs(1 hunks)Maple2.Server.Game/Manager/StatsManager.cs(1 hunks)Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs(8 hunks)Maple2.Server.Game/PacketHandlers/HomeDoctorHandler.cs(1 hunks)Maple2.Server.Game/PacketHandlers/ResolveDeathPenaltyHandler.cs(1 hunks)Maple2.Server.Game/PacketHandlers/RevivalHandler.cs(2 hunks)Maple2.Server.Game/PacketHandlers/TombstoneHandler.cs(1 hunks)Maple2.Server.Game/Packets/FieldPacket.cs(1 hunks)Maple2.Server.Game/Packets/PartyPacket.cs(4 hunks)Maple2.Server.Game/Session/GameSession.cs(5 hunks)Maple2.Server.Game/Util/ConditionUtil.cs(2 hunks)Maple2.Server.World/Service/WorldService.Sync.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- Maple2.File.Ingest/Program.cs
- Maple2.Server.Game/Util/ConditionUtil.cs
- Maple2.Server.Game/PacketHandlers/ResolveDeathPenaltyHandler.cs
- Maple2.Server.Game/PacketHandlers/TombstoneHandler.cs
🧰 Additional context used
🧬 Code Definitions (7)
Maple2.Server.Game/Manager/PartyManager.cs (1)
Maple2.Server.Core/Formulas/BaseStat.cs (1)
Health(206-233)
Maple2.Server.Game/PacketHandlers/RevivalHandler.cs (4)
Maple2.Server.Game/Session/GameSession.cs (3)
GameSession(37-803)GameSession(105-115)GameSession(696-696)Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (1)
Revive(329-372)Maple2.Server.Core/Packets/NoticePacket.cs (1)
NoticePacket(9-77)Maple2.Model/Game/User/Player.cs (1)
Currency(45-59)
Maple2.Server.Game/Commands/KillCommand.cs (2)
Maple2.Server.Game/Session/GameSession.cs (3)
GameSession(37-803)GameSession(105-115)GameSession(696-696)Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (3)
FieldPlayer(15-533)FieldPlayer(90-102)ConsumeHp(404-427)
Maple2.Server.Game/PacketHandlers/HomeDoctorHandler.cs (4)
Maple2.Server.Game/Session/GameSession.cs (4)
GameSession(37-803)GameSession(105-115)GameSession(696-696)ConditionUpdate(525-528)Maple2.Server.Game/PacketHandlers/ResolveDeathPenaltyHandler.cs (1)
Handle(13-32)Maple2.Model/Game/User/Character.cs (1)
Character(7-56)Maple2.Server.Game/Manager/Config/ConfigManager.cs (1)
UpdateDeathPenalty(320-343)
Maple2.Server.Game/Session/GameSession.cs (2)
Maple2.Server.Game/Manager/Config/ConfigManager.cs (3)
SetDeathPenalty(345-348)LoadRevival(307-314)AddInstantReviveCount(350-358)Maple2.Server.World/Containers/PlayerConfigLookUp.cs (1)
DeathInfo(128-142)
Maple2.Server.Game/Manager/Config/ConfigManager.cs (4)
Maple2.Model/Game/User/Character.cs (1)
Character(7-56)Maple2.Server.Game/Packets/RevivalPacket.cs (1)
RevivalPacket(9-40)Maple2.Model/Metadata/Constants.cs (1)
Constant(10-939)Maple2.Server.World/Containers/PlayerConfigLookUp.cs (1)
DeathInfo(128-142)
Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (2)
Maple2.Server.Game/Model/Field/Tombstone.cs (2)
Tombstone(8-39)Tombstone(27-31)Maple2.Server.Game/Model/Field/Actor/Actor.cs (2)
Update(200-211)OnDeath(235-235)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (41)
Maple2.Model/Enum/Death.cs (1)
3-7: Well-structured death state enum with clear values.The new
DeathStateenum properly defines three states (Alive,FirstDeath, andMetal) with appropriate values and usesshortas the underlying type, which is consistent with the PR objectives of tracking player death states in memory.Maple2.Model/Game/User/IPlayerInfo.cs (1)
21-21: Good integration of DeathState property.The addition of the
DeathStateproperty to theIPlayerInfointerface is logically placed within the "Health" section, following the HP properties. This enables proper tracking of player death states as described in the PR objectives.Maple2.Server.World/Service/WorldService.Sync.cs (1)
39-39: Death state properly included in player info response.The addition of the
DeathStatefield to thePlayerInfoResponseensures that a player's death state is properly communicated between services. The casting tointis consistent with how other enum values are handled in this response (e.g., Gender, Job).Maple2.Server.Game/Manager/PartyManager.cs (1)
365-365: Death state updates now properly trigger party stats updates.The conditional logic has been modified to send party member stat updates when a player's death state changes, in addition to when their health changes. This ensures that party members are properly notified of death status changes, which is essential for the Death & Revival system described in the PR objectives.
Maple2.Server.Game/Manager/StatsManager.cs (1)
126-134: Good enhancement to maintain health state synchronizationThis addition ensures that the player's health information is properly synchronized with the player's session after stats are refreshed. This is important for the new death and revival system as it allows the system to accurately track the player's health state.
The asynchronous processing flag is a good choice to prevent blocking operations when updating this information.
Maple2.Server.Game/PacketHandlers/HomeDoctorHandler.cs (3)
20-22: Good early return conditionAdding this early return condition ensures that the home doctor service is only used when the player actually has a death penalty to resolve. This prevents unnecessary processing and potential exploits.
24-25: Dynamic cost calculation is a better approachReplacing the static cost with a dynamic calculation based on the player's level and death count provides a more flexible and balanced system. This aligns well with the death system rework.
30-31: Good implementation of penalty resolutionThe penalty resolution now properly deducts the dynamically calculated cost and resets the death penalty. This ensures consistent handling of death penalties.
Maple2.Server.Core/proto/sync.proto (3)
68-69: Good addition of death state trackingAdding the death_state field to the PlayerUpdateRequest enables the death state to be properly synchronized across services. This is essential for the new death and revival system.
71-71: Correct field position updateThe async field position has been correctly updated to accommodate the new death_state field. This maintains proper protocol buffer structure.
104-104: Good consistency in message structuresThe death_state field has been consistently added to both request and response messages, ensuring proper bidirectional communication of death state information.
Maple2.Model/Game/User/Character.cs (1)
52-54: Good implementation of death tracking fieldsThe addition of DeathCount, DeathTick, and DeathState fields provides the necessary data structure to support the new death and revival system. These fields allow tracking of:
- How many times a player has died (DeathCount)
- When the player last died (DeathTick)
- The current death state of the player (DeathState)
This implementation aligns well with the PR objectives.
Maple2.Model/Game/User/PlayerInfo.cs (2)
126-126: Good addition of DeathState property to CharacterInfo class.This new property integrates well with the existing health-related properties and is appropriately placed in the "Health" section of the class.
67-67: Great addition of DeathState serialization.The serialization is properly placed after writing CurrentHp, maintaining the logical order of health-related data in the packet structure.
Maple2.Server.Game/Packets/FieldPacket.cs (1)
302-303: Updated packet structure to include death information.The change correctly serializes the player's current HP and death count in the packet structure. This ensures that clients receive accurate information about the player's health status.
Maple2.Server.Game/Packets/PartyPacket.cs (2)
196-197: Updated party stats packet structure to include death state.The changes to the UpdateStats method now properly serialize the player's death state along with their health information. This ensures that party members receive accurate status updates about each other.
9-9: Added ReSharper suppression comments to improve code quality.These comments help maintain code quality by suppressing false-positive warnings from ReSharper.
Also applies to: 207-207, 247-247
Maple2.Server.Game/Commands/KillCommand.cs (1)
17-17: Updated command description and added KillPlayerCommand.The command description now includes "Players" to reflect the new capability, and the new command is properly integrated into the main command class.
Also applies to: 22-22
Maple2.Server.Core/Sync/PlayerInfoUpdateExtensions.cs (3)
51-53: Death state tracking integrated properlyThis addition ensures that player death state is properly synchronized when the update includes the
UpdateField.Deathflag. The implementation follows the same pattern as other state updates in this file.
108-110: Death state propagation between players implemented correctlyThe addition of death state update in the player-to-player update method ensures consistent death state synchronization between players, which is critical for the new death and revival system.
165-167: Death state serialization properly implementedThe addition ensures that when setting fields in a
PlayerUpdateRequest, the player's death state is properly converted to an integer for transmission. This completes the full synchronization cycle for death state updates.Maple2.Server.Game/PacketHandlers/RevivalHandler.cs (5)
1-3: Added necessary imports for death/revival functionalityThe addition of the Model.Enum and Model.Game imports supports the new types used in the death and revival system.
31-42: Improved safe revival conditions and handlingThe implementation now properly checks if revival is allowed in the current field and if a revival return ID is set. This prevents revival in areas where it should be restricted.
44-70: Robust implementation of instant revival with voucher supportThe implementation now checks revival conditions and provides two revival options: using a voucher or mesos. The voucher consumption is handled correctly with proper user notification.
74-77: Correctly increment revival count and clean up tombstoneThis section properly increments the player's instant revival count and calls the Revive method with the instant flag set to true. It also correctly nullifies the tombstone object to complete the revival process.
61-71:Details
❓ Verification inconclusive
Verify meso revival cost calculations
The meso-based revival logic calculates costs dynamically using Lua scripts and checks both daily limits and player currency. Make sure the formula in Lua scripts correctly scales with level to maintain game balance.
🏁 Script executed:
#!/bin/bash # Check the Lua script that calculates revival costs # Find files that might contain the revival cost calculation logic echo "Searching for Lua files with revival price calculation:" rg -l "CalcRevivalPrice" --type luaLength of output: 305
I'll re-run the search without a file type filter to ensure we're scanning all files for the
"CalcRevivalPrice"keyword. This will help verify where the cost calculation logic is implemented.
🏁 Script executed:
#!/bin/bash # Search for 'CalcRevivalPrice' across the entire repository echo "Searching for 'CalcRevivalPrice' across the entire repository:" rg -l "CalcRevivalPrice"Length of output: 217
Action Required: Verify Lua Script Implementation for Revival Cost Scaling
After searching the codebase, the only reference to
CalcRevivalPriceis found inMaple2.Server.Game/PacketHandlers/RevivalHandler.cs. No separate Lua file or definition forCalcRevivalPricewas detected in the repository. This implies that the method likely delegates to an external or dynamically loaded Lua script that is not part of the current codebase.
- Action Items:
- Manually verify that the Lua script being loaded at runtime implements
CalcRevivalPricewith the intended logic and correct scaling based on character level.- Confirm that the cost calculation formula maintains game balance, especially regarding how it scales with player level.
- Ensure that any updates or documentation regarding the Lua script are correctly synchronized with the C# invocation.
Maple2.Server.Core/Sync/PlayerInfoUpdateEvent.cs (4)
22-22: Death flag added to UpdateField enumThe Death flag with value 4096 has been properly added to the UpdateField enum, allowing for tracking of player death state changes.
28-28: Death state added to Party preset flagsThe Death flag has been added to the Party preset, ensuring that death state changes are propagated to party members. This is essential for party members to know when someone has died and needs revival.
81-83: Player death state change detection implementedThis condition checks if the request contains a death state and if it differs from the player's current death state. If so, it sets the Death flag in the Type field, triggering a death state update.
113-117: Improved collection check logicThe code now checks if the collections have items (Count > 0) rather than if they're not null, which is more precise. This ensures updates are only applied when there are actual entries to compare.
Also applies to: 119-123
Maple2.Server.Game/Session/GameSession.cs (4)
258-258: Initialized death penalty on server entryAdded code to set the death penalty based on the player's configuration data when entering the server. This ensures the death state is properly restored when players reconnect.
429-429: TODO comment for persisting player stateThere's a TODO comment about saving player health in memory to help determine if the player is dead upon login. This should be implemented to ensure consistent state management.
Consider implementing this TODO or creating a tracking issue for it as it's important for proper death state management.
446-446: Centralized revival handling in Config managerReplaced direct packet sending with a call to
Config.LoadRevival(), which centralizes revival handling logic in the Config manager. This improves code organization and separation of concerns.
605-605: Updated daily revival count resetChanged from directly setting
Config.DeathCount = 0to usingConfig.AddInstantReviveCount(-1), which is more appropriate for the new revival system where negative values reset the count to zero.Maple2.Server.Game/Manager/Config/ConfigManager.cs (2)
307-314: Good initialization for revival system.The method properly initializes revival mechanics by checking for expired death penalties and sending appropriate client updates.
345-348: Clean implementation for setting death penalty from external info.This method effectively sets the death penalty based on external death information.
Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (5)
22-49: Improved state management with effective change detection.The refactoring of
StateandIsDeadproperties to use backing fields with custom change detection logic is excellent. TheIsDeadproperty now properly updates the character's death state based on field properties and death count.
65-79: Well-designed death state tracking with proper synchronization.The
Tombstoneproperty andDeathStateproperty effectively manage the player's death state and ensure changes are properly communicated to other systems through the player info service.
152-164: Comprehensive death state handling in update loop.The update method now properly checks for player death state and manages revival through the tombstone system. The battle state check has been appropriately moved after the death check to ensure proper execution order.
313-323: Enhanced death handling with proper state cleanup.The
OnDeathmethod now properly handles related states (craft mode, held cube), creates a tombstone, and broadcasts death information to other players.
325-372: Robust revival implementation with comprehensive state management.The
Revivemethod thoroughly handles all aspects of revival including health restoration, death penalty application, condition updates, client notification, potential map changes, and buff updates.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Maple2.Server.Game/Commands/KillCommand.cs (1)
108-141: 🛠️ Refactor suggestionConsider adding invulnerability or field-specific checks
Currently, this command kills a player by setting HP to zero without verifying buffs (e.g., invulnerability) or special field rules. Adding these checks would mirror prior suggestions on handling edge cases.Maple2.Server.Game/Session/GameSession.cs (1)
789-793: Clamp negative penalty durations before saving
As previously recommended, ensure(int) (Config.DeathPenaltyEndTick - fieldTick)is clamped to zero to avoid inconsistencies if it’s negative.- MsRemaining = (int) (Config.DeathPenaltyEndTick - fieldTick), + var msRem = (int) (Config.DeathPenaltyEndTick - fieldTick); + MsRemaining = msRem > 0 ? msRem : 0;
🧹 Nitpick comments (3)
Maple2.Server.Game/PacketHandlers/ResolveDeathPenaltyHandler.cs (1)
10-33: Validate cost calculation to avoid negative or zero amounts
While this logic is sound, ensureCalcResolvePenaltyPricecannot produce negative or zero costs (which might allow free resolution or unexpected behavior). If needed, add guards to handle invalid cost values.Maple2.Server.Game/Session/GameSession.cs (1)
429-429: Follow through on the TODO
Consider persisting HP/SP/EP in memory for accurate dead state detection on login. Let me know if you'd like assistance implementing this.Maple2.Server.Game/Manager/Config/ConfigManager.cs (1)
317-344: Well-structured death penalty update implementationThe method has clear comments and handles different cases appropriately: skipping penalties for low-level players, resetting when endTick is 0, and handling expiration logic.
Note: There's a minor typo in the constant name
Constant.UserRevivalPaneltyMinLevel(should be "PenaltyMinLevel").- if (session.Player.Value.Character.Level < Constant.UserRevivalPaneltyMinLevel) { + if (session.Player.Value.Character.Level < Constant.UserRevivalPenaltyMinLevel) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
Maple2.Model/Enum/Death.cs(1 hunks)Maple2.Model/Game/User/Character.cs(1 hunks)Maple2.Model/Game/User/IPlayerInfo.cs(1 hunks)Maple2.Model/Game/User/PlayerInfo.cs(2 hunks)Maple2.Server.Core/Sync/PlayerInfoUpdateEvent.cs(3 hunks)Maple2.Server.Core/Sync/PlayerInfoUpdateExtensions.cs(3 hunks)Maple2.Server.Core/proto/sync.proto(2 hunks)Maple2.Server.Game/Commands/KillCommand.cs(2 hunks)Maple2.Server.Game/Manager/Config/ConfigManager.cs(4 hunks)Maple2.Server.Game/Manager/PartyManager.cs(1 hunks)Maple2.Server.Game/Manager/StatsManager.cs(1 hunks)Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs(8 hunks)Maple2.Server.Game/PacketHandlers/HomeDoctorHandler.cs(1 hunks)Maple2.Server.Game/PacketHandlers/ResolveDeathPenaltyHandler.cs(1 hunks)Maple2.Server.Game/PacketHandlers/RevivalHandler.cs(2 hunks)Maple2.Server.Game/PacketHandlers/TombstoneHandler.cs(1 hunks)Maple2.Server.Game/Packets/FieldPacket.cs(1 hunks)Maple2.Server.Game/Packets/PartyPacket.cs(4 hunks)Maple2.Server.Game/Session/GameSession.cs(5 hunks)Maple2.Server.Game/Util/ConditionUtil.cs(2 hunks)Maple2.Server.World/Service/WorldService.Sync.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (17)
- Maple2.Model/Enum/Death.cs
- Maple2.Server.World/Service/WorldService.Sync.cs
- Maple2.Model/Game/User/IPlayerInfo.cs
- Maple2.Server.Game/Util/ConditionUtil.cs
- Maple2.Server.Game/Manager/PartyManager.cs
- Maple2.Server.Game/PacketHandlers/HomeDoctorHandler.cs
- Maple2.Server.Game/Packets/FieldPacket.cs
- Maple2.Server.Game/Packets/PartyPacket.cs
- Maple2.Server.Game/Manager/StatsManager.cs
- Maple2.Server.Core/Sync/PlayerInfoUpdateExtensions.cs
- Maple2.Server.Core/proto/sync.proto
- Maple2.Server.Core/Sync/PlayerInfoUpdateEvent.cs
- Maple2.Model/Game/User/PlayerInfo.cs
- Maple2.Model/Game/User/Character.cs
- Maple2.Server.Game/PacketHandlers/TombstoneHandler.cs
- Maple2.Server.Game/PacketHandlers/RevivalHandler.cs
- Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs
🧰 Additional context used
🧬 Code Definitions (4)
Maple2.Server.Game/Commands/KillCommand.cs (2)
Maple2.Server.Game/Session/GameSession.cs (3)
GameSession(37-803)GameSession(105-115)GameSession(696-696)Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (3)
FieldPlayer(15-532)FieldPlayer(90-102)ConsumeHp(404-426)
Maple2.Server.Game/Session/GameSession.cs (2)
Maple2.Server.Game/Manager/Config/ConfigManager.cs (3)
SetDeathPenalty(346-349)LoadRevival(308-315)AddInstantReviveCount(351-359)Maple2.Server.World/Containers/PlayerConfigLookUp.cs (1)
DeathInfo(128-142)
Maple2.Server.Game/PacketHandlers/ResolveDeathPenaltyHandler.cs (4)
Maple2.Server.Game/Session/GameSession.cs (4)
GameSession(37-803)GameSession(105-115)GameSession(696-696)ConditionUpdate(525-528)Maple2.Server.Game/PacketHandlers/HomeDoctorHandler.cs (1)
Handle(14-35)Maple2.Model/Game/User/Player.cs (1)
Currency(45-59)Maple2.Server.Game/Manager/Config/ConfigManager.cs (1)
UpdateDeathPenalty(321-344)
Maple2.Server.Game/Manager/Config/ConfigManager.cs (3)
Maple2.Model/Game/User/Character.cs (1)
Character(7-56)Maple2.Server.Game/Packets/RevivalPacket.cs (1)
RevivalPacket(9-40)Maple2.Server.World/Containers/PlayerConfigLookUp.cs (1)
DeathInfo(128-142)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: format
- GitHub Check: build
🔇 Additional comments (11)
Maple2.Server.Game/Commands/KillCommand.cs (2)
17-17: Description updated to reflect broader functionality
The updated description accurately conveys that this command now supports killing NPCs, mobs, and players.
22-22: Addition of the KillPlayerCommand
Registering this new command aligns well with the expanded kill feature.Maple2.Server.Game/Session/GameSession.cs (2)
446-446: Revival loading integration
Loading revival data here appears intentional for parity with the rest of the login flow.
605-605: InstantReviveCount decrement
This negative parameter use is safe given the clamp inAddInstantReviveCount.Maple2.Server.Game/Manager/Config/ConfigManager.cs (7)
29-36: Good implementation of death penalty propertiesThe properties provide a clean interface to access character death state while maintaining encapsulation. Using getters and setters that delegate to the underlying Character object is a good practice.
37-37: LGTM: InstantReviveCount initializationThis new field properly tracks the number of instant revivals available to the player, aligning with the PR's objective to implement death and revival mechanics.
62-62: LGTM: Constructor parameter updateThe addition of InstantReviveCount parameter properly replaces the removed deathPenaltyTick and DeathCounter parameters.
308-315: Good implementation of revival loading logicThe LoadRevival method appropriately checks if the death penalty has expired and resets it if needed, then sends the necessary packets to update the client with the current revival state.
346-349: LGTM: Death penalty setting from DeathInfoThe method correctly sets the death penalty values based on the provided DeathInfo object and current tick.
351-359: Confirm the intended behavior for negative count valuesThe method resets InstantReviveCount to 0 when count is negative, rather than decrementing it. Is this the intended behavior?
If decrementing is intended and you want to ensure it doesn't go below zero, consider this alternative:
- if (count < 0) { - InstantReviveCount = 0; - } else { - InstantReviveCount += count; - } + InstantReviveCount = Math.Max(0, InstantReviveCount + count);
576-576: LGTM: Save method updated correctlyThe Save method now includes InstantReviveCount, ensuring that the revival state persists between sessions.
2 participants
Summary by CodeRabbit