Skip to content

Conversation

@glinesbdev
Copy link
Contributor

@glinesbdev glinesbdev commented Jan 15, 2026

Maple2.Model/Enum/Skill.cs

Introduced a new Skill enum: DotTargetType. It was found for DotDamageProperty XML values can really only be two values. This change makes it easier to reason about who is the caster and who is the owner of the dot (de)buff.

Previously, dot damage values were using the SkillTargetType enum. This enum was updated to DotTargetType for dot damage buff application. Other areas, like skill target decisions, still use the SkillTargetType.

Because the enum mapping has changed, Maple2.File.Ingest will need to be run to update the new enum mapping.

Summary by CodeRabbit

  • Bug Fixes
    • Refined damage-over-time (DoT) buff behavior so effects consistently apply to the correct recipient (caster or buff owner), reducing misapplied ticks and improving reliability of DoT abilities during combat for more predictable gameplay.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 15, 2026

Walkthrough

Introduced DotTargetType and switched DOT buff targeting from SkillTargetType to DotTargetType; updated metadata, mapper, and server buff application logic to use the new enum and adjusted which entity receives the applied buff based on that value.

Changes

Cohort / File(s) Summary
New Enum Definition
Maple2.Model/Enum/Skill.cs
Added DotTargetType enum with members Caster = 0 and Owner = 1.
Metadata / API Surface
Maple2.Model/Metadata/AdditionalEffectMetadata.cs
Updated AdditionalEffectMetadataDot.DotBuff constructor signature: Target parameter type changed from SkillTargetType to DotTargetType.
Ingest / Mapping
Maple2.File.Ingest/Mapper/AdditionalEffectMapper.cs
Mapper now casts dot-buff target values to DotTargetType when constructing AdditionalEffectMetadataDot.DotBuff.
Runtime Logic
Maple2.Server.Game/Model/Field/Buff.cs
ApplyDotBuff now checks DotTargetType.Caster and routes AddBuff calls to the appropriate recipient (Caster.Buffs.AddBuff(...) vs Owner.Buffs.AddBuff(...)) with adjusted argument ordering.

Sequence Diagram(s)

sequenceDiagram
    actor Caster
    actor Owner
    participant Mapper
    participant Metadata
    participant Buffs

    Mapper->>Metadata: create DotBuff(Target: DotTargetType)
    Caster->>Buffs: ApplyDotBuff(DotBuff)
    alt Target == DotTargetType.Caster
        Buffs->>Caster.Buffs: AddBuff(source=Owner, target=Caster, ...)
    else Target == DotTargetType.Owner
        Buffs->>Owner.Buffs: AddBuff(source=Caster, target=Owner, ...)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Fix: Buffs & DotDamage #245: Modifies ApplyDotBuff buff-application logic and AddBuff call patterns; strongly related to the server-side changes here.

Suggested reviewers

  • Zintixx

Poem

🐰 A tiny hop, a tiny nudge,
DotTarget guides the buffing judge,
Caster or Owner — which to be?
I sniff the code and plant the key,
Hooray, buffs bound where they should trudge! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Skill Dot Debuff Refactor' accurately reflects the main change: introducing a new DotTargetType enum to replace SkillTargetType for dot buff/debuff application, refactoring the dot damage property handling across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 54ffe6e and ab0e474.

📒 Files selected for processing (1)
  • Maple2.Server.Game/Model/Field/Buff.cs
🧰 Additional context used
🧬 Code graph analysis (1)
Maple2.Server.Game/Model/Field/Buff.cs (2)
Maple2.Server.Game/Manager/BuffManager.cs (1)
  • AddBuff (73-187)
Maple2.Server.Game/Trigger/TriggerContext.Field.cs (1)
  • AddBuff (361-379)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: format
  • GitHub Check: build
  • GitHub Check: build
🔇 Additional comments (1)
Maple2.Server.Game/Model/Field/Buff.cs (1)

251-262: LGTM!

The refactored ApplyDotBuff logic correctly uses the new DotTargetType enum and properly routes the buff application:

  • DotTargetType.Caster: Adds buff to Caster.Buffs with Owner as the buff's caster and Caster as the buff's owner.
  • Otherwise (DotTargetType.Owner): Adds buff to Owner.Buffs with Caster as the buff's caster and Owner as the buff's owner.

This aligns with the AddBuff(IActor caster, IActor owner, ...) signature in BuffManager.cs and addresses the previous review feedback.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Zintixx Zintixx merged commit 348db71 into MS2Community:master Jan 15, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants