Fix: Buffs & DotDamage

[shuabritze]

Buffs: Now apply correctly from dots & triggers
Dot damage: Now applies the correct caster when calculating the damage.

Summary by CodeRabbit

Release Notes

• New Features

  • Enhanced leveling-up process with automatic player statistics refresh.
  • Improved buff management system, streamlining how buffs are applied to players and NPCs.

• Bug Fixes

  • Corrected logic for determining buff application and removal, ensuring more accurate gameplay.

• Refactor

  • Simplified logic in skill effect application by directly referencing the caster.
  • Updated methods to focus on NPCs instead of mobs in specific contexts.
  • Standardized parameter names for clarity in buff management methods.

• Documentation

  • Updated logging statements to reflect changes in parameter names for clarity.

[coderabbitai]

Walkthrough

The changes involve modifications to various classes and interfaces within the Maple2 server codebase, focusing on buff management, experience leveling, and damage calculations. Key updates include the addition of a statistics refresh during player level-ups, simplifications in the actor's target attack logic, the removal of the AddBuff method from the IActor interface, and adjustments to how buffs are applied and removed. Additionally, the NpcsInBox method has been altered to return NPCs instead of mobs under specific conditions.

Changes

File Path	Change Summary
Maple2.Server.Game/Manager/ExperienceManager.cs	Added session.Stats.Refresh() call in LevelUp method to refresh player statistics after leveling up.
Maple2.Server.Game/Model/Field/Actor/Actor.cs	Simplified TargetAttack method by removing the intermediate caster variable, directly using record.Caster for condition checks.
Maple2.Server.Game/Model/Field/Actor/IActor.cs	Removed AddBuff method from the IActor interface, affecting buff management functionality.
Maple2.Server.Game/Model/Field/Buff.cs	Updated ApplyDotBuff method to use Owner.Buffs.AddBuff and Caster.Buffs.AddBuff for adding buffs, reflecting a restructuring of the buff management system.
Maple2.Server.Game/Model/Skill/DotDamageRecord.cs	Modified DotDamageRecord constructor to streamline damage calculations by removing dependency on Rate for hpAmount.
Maple2.Server.Game/Trigger/TriggerContext.Field.cs	Changed AddBuff and RemoveBuff methods to use ignorePlayer instead of isPlayer, altering the logic for applying/removing buffs.
Maple2.Server.Game/Trigger/TriggerContext.Npc.cs	Modified NpcsInBox method to return NPCs from Field.EnumerateNpcs() instead of mobs when boxIds is empty or contains zero.
Maple2.File.Ingest/Utils/TriggerDefinitionOverride.cs	Updated argument name from isPlayer to ignorePlayer in ActionOverride for "add_buff" action, reflecting a semantic shift in buff management.
Maple2.Server.Game/Scripting/Trigger/TriggerContext.cs	Renamed isPlayer to ignorePlayer in AddBuff and RemoveBuff methods for consistency and clarity in buff application logic.

Sequence Diagram(s)

sequenceDiagram
    participant Player
    participant ExperienceManager
    participant Stats

    Player->>ExperienceManager: LevelUp()
    ExperienceManager->>Player: Update player conditions
    ExperienceManager->>Stats: Refresh()
    ExperienceManager->>Player: Send update request

Loading

🐇 In the code where bunnies play,
Buffs and stats come out to play.
With each level, a hop so bright,
Refreshing stats, a joyful sight!
Simplified paths, oh what a cheer,
In Maple's world, we hold so dear! 🌟

Suggested reviewers

• Zintixx
• AngeloTadeucci

---

Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7018305 and 7b02bac.

Files selected for processing (2)

• Maple2.File.Ingest/Utils/TriggerDefinitionOverride.cs (1 hunks)
• Maple2.Server.Game/Scripting/Trigger/TriggerContext.cs (2 hunks)

Additional comments not posted (4)

Maple2.Server.Game/Scripting/Trigger/TriggerContext.cs (2)

113-113: Verify the impact of the RemoveBuff method parameter name change.

The parameter name change from isPlayer to ignorePlayer enhances clarity by explicitly conveying the purpose of the parameter. However, it's essential to ensure that all invocations of the RemoveBuff method have been updated to use the new parameter name and pass the correct value based on the new semantics.

Run the following script to verify the RemoveBuff method usage:

#!/bin/bash
# Description: Verify all invocations of the `RemoveBuff` method have been updated to use the new `ignorePlayer` parameter name.

# Test: Search for the method usage. Expect: No occurrences of the old `isPlayer` parameter name.
rg --type csharp -A 5 $'RemoveBuff\(.*isPlayer'

---

9-9: Verify the impact of the AddBuff method parameter name change.

The parameter name change from isPlayer to ignorePlayer enhances clarity by explicitly conveying the purpose of the parameter. However, it's essential to ensure that all invocations of the AddBuff method have been updated to use the new parameter name and pass the correct value based on the new semantics.

Run the following script to verify the AddBuff method usage:

Verification successful

---

Parameter name change successfully implemented and verified

The renaming of the isPlayer parameter to ignorePlayer in the AddBuff method has been successfully implemented and verified across the codebase. Our analysis shows:

• No occurrences of the old isPlayer parameter name in AddBuff method calls.
• Consistent usage of the new ignorePlayer parameter name in all AddBuff method invocations.
• The change has been applied uniformly across multiple files, including:
  • Maple2.Server.Game/Trigger/TriggerContext.Field.cs
  • Maple2.Server.Game/Scripting/Trigger/TriggerContext.cs
  • Maple2.Server.Game/Manager/Config/BuffManager.cs
  • Maple2.Server.Game/Model/Field/Actor/Actor.cs

This verification confirms that the parameter name change has been thoroughly and correctly implemented, improving code clarity without introducing inconsistencies.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all invocations of the `AddBuff` method have been updated to use the new `ignorePlayer` parameter name.

# Test: Search for the method usage. Expect: No occurrences of the old `isPlayer` parameter name.
rg --type csharp -A 5 $'AddBuff\(.*isPlayer'

Length of output: 48

---

Script:

#!/bin/bash
# Description: Verify all invocations of the `AddBuff` method have been updated to use the new `ignorePlayer` parameter name.

# Test 1: Search for the method usage with old parameter name. Expect: No occurrences of the old `isPlayer` parameter name.
echo "Searching for AddBuff with old 'isPlayer' parameter:"
rg --type csharp -A 5 $'AddBuff\(.*isPlayer'

# Test 2: Search for the method usage with new parameter name. Expect: Occurrences of the new `ignorePlayer` parameter name.
echo "Searching for AddBuff with new 'ignorePlayer' parameter:"
rg --type csharp -A 5 $'AddBuff\(.*ignorePlayer'

# Test 3: List all AddBuff method invocations for manual review if needed.
echo "Listing all AddBuff method invocations:"
rg --type csharp -A 5 $'AddBuff\('

Length of output: 11882

Maple2.File.Ingest/Utils/TriggerDefinitionOverride.cs (2)

Line range hint 1-1048: No further comments.

The rest of the file looks good, and no additional changes were found that require comments.

---

41-42: Verify the impact of renaming the argument to "ignorePlayer".

The argument "isPlayer" has been renamed to "ignorePlayer", which seems to have the opposite meaning. Please verify if this renaming aligns with the intended behavior of the "add_buff" action.

Additionally, check if there is any existing code that uses the "isPlayer" argument and needs to be updated to use "ignorePlayer" instead.

Run the following script to verify the usage of the "isPlayer" argument:

Verification successful

---

Renaming from "isPlayer" to "ignorePlayer" is correct and consistently implemented.

The verification process has confirmed that the renaming of the argument from "isPlayer" to "ignorePlayer" in the "add_buff" action is correct and has been consistently implemented across the codebase. The new name accurately reflects the argument's behavior:

• When ignorePlayer is false, the buff is applied to players.
• When ignorePlayer is true, the buff is applied to monsters (effectively ignoring players).

This renaming improves code readability and self-documentation. No issues were found, and no further actions are required.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the "isPlayer" argument in the "add_buff" action.

# Test 1: Search for the usage of "isPlayer" argument. Expect: No occurrences.
rg --type csharp -A 5 $'isPlayer'

# Test 2: Search for the usage of "ignorePlayer" argument. Expect: Occurrences in the implementation of "add_buff" action.
rg --type csharp -A 5 $'ignorePlayer'

Length of output: 5748

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  -- I pushed a fix in commit <commit_id>, please review it.
  -- Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  -- @coderabbitai generate unit testing code for this file.
  -- @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  -- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  -- @coderabbitai read src/utils.ts and generate unit testing code.
  -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  -- @coderabbitai help me debug CodeRabbit configuration file.

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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 708b529 and 7018305.

Files selected for processing (7)

• Maple2.Server.Game/Manager/ExperienceManager.cs (1 hunks)
• Maple2.Server.Game/Model/Field/Actor/Actor.cs (1 hunks)
• Maple2.Server.Game/Model/Field/Actor/IActor.cs (0 hunks)
• Maple2.Server.Game/Model/Field/Buff.cs (1 hunks)
• Maple2.Server.Game/Model/Skill/DotDamageRecord.cs (1 hunks)
• Maple2.Server.Game/Trigger/TriggerContext.Field.cs (1 hunks)
• Maple2.Server.Game/Trigger/TriggerContext.Npc.cs (1 hunks)

Files not reviewed due to no reviewable changes (1)

• Maple2.Server.Game/Model/Field/Actor/IActor.cs

Additional comments not posted (7)

Maple2.Server.Game/Model/Skill/DotDamageRecord.cs (2)

28-28: LGTM!

The addition of the Rate property to the Properties object is a valid change and aligns with the provided list of alterations. The property is likely used in damage calculations or other game mechanics.

---

36-36: Verify the impact of the change on game balance and damage output.

The change in the damage calculation logic, specifically the removal of the dependency on dotDamage.Rate during the accumulation of hpAmount, may impact the overall damage output and game balance.

Please ensure that the DamageCalculator.CalculateDamage method accurately calculates the intended damage and that the change does not introduce any unintended consequences or imbalances in the game.

To verify the impact of the change, consider the following:

1. Review the implementation of the DamageCalculator.CalculateDamage method to ensure it correctly calculates the damage based on the desired formula and factors.
2. Conduct thorough testing with various scenarios and edge cases to validate that the damage output aligns with the expected values and does not introduce any exploits or imbalances.
3. Analyze the game balance implications of the change, considering factors such as player progression, enemy difficulty, and overall gameplay experience.
4. If necessary, adjust the damage calculation formula or other game parameters to maintain the desired balance and gameplay experience.

Maple2.Server.Game/Model/Field/Actor/Actor.cs (1)

175-176: LGTM!

The change simplifies the logic by removing the intermediate caster variable and directly using record.Caster in the condition check. This streamlines the code without altering the overall control flow.

Maple2.Server.Game/Manager/ExperienceManager.cs (1)

179-180: LGTM!

The addition of session.Stats.Refresh() after updating the player's conditions for leveling up is a good practice. It ensures that the player's statistics are up-to-date before sending the update request to the player's information.

Refreshing the player's statistics at this point is crucial for maintaining consistency in the game state and ensuring that any subsequent operations or calculations that rely on the player's current state have access to the most recent data.

Maple2.Server.Game/Model/Field/Buff.cs (1)

226-228: LGTM!

The changes to the ApplyDotBuff method look good. The updates to how buffs are added to the Owner and Caster entities are consistent with the AI-generated summary and appear to be part of a larger refactoring effort to improve the buff management system. The changes are localized and do not appear to introduce any new issues.

Maple2.Server.Game/Trigger/TriggerContext.Npc.cs (1)

249-249: LGTM! The change aligns the method behavior with its name and purpose.

The modification in the NpcsInBox method to return Field.EnumerateNpcs() instead of Field.Mobs.Values when boxIds is empty or contains a zero is a positive change. It ensures that the method returns a collection of NPCs, which aligns with its name and expected behavior.

This change appears to be a bug fix or correction, as the method name NpcsInBox implies it should return NPCs, not mobs. The rest of the method logic remains intact, filtering the returned collection based on the provided boxIds.

Overall, this change improves the consistency and correctness of the method.

Maple2.Server.Game/Trigger/TriggerContext.Field.cs (1)

Line range hint 372-382: Verify the ignorePlayer parameter usage across the codebase.

The changes to the RemoveBuff function, including the parameter rename and logic inversion, look good. However, ensure that all code paths invoking this function have been updated to pass the ignorePlayer parameter correctly to maintain the intended behavior.

Run the following script to verify the parameter usage:

#!/bin/bash 
# Description: Verify all invocations of `RemoveBuff` pass the `ignorePlayer` parameter correctly.

# Test: Search for the function usage. Expect: Only occurrences passing `ignorePlayer` as the 3rd argument.
rg --type cs -A 5 $'RemoveBuff\('