Skip to content

Additional Buff and Skill BeginConditions - #411

Merged
Zintixx merged 9 commits into
masterfrom
buff-and-skill-beginconditions
Apr 10, 2025
Merged

Additional Buff and Skill BeginConditions#411
Zintixx merged 9 commits into
masterfrom
buff-and-skill-beginconditions

Conversation

@Zintixx

@Zintixx Zintixx commented Apr 10, 2025 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Collaborator
  • Implements more BeginConditions for skills
  • Fixes Striker Class #257
  • Adjustments on AnimationState => Renamed to AnimationManager
  • Renamed AnimationSequence to AnimationSequenceMetadata

Summary by CodeRabbit

  • New Features
    • Introduced a new animation management system to deliver smoother and more consistent animations for characters and NPCs.
    • Enhanced gameplay conditions by integrating additional checks and refined stat comparisons to enrich skill and buff behaviors.
  • Refactor
    • Streamlined animation handling and consolidated logic across core systems for improved reliability and performance.
    • Updated underlying data structures for animation sequences to better reflect metadata and ensure consistent visual behavior.
  • Chores
    • Upgraded package dependencies to leverage the latest improvements and maintain optimal performance.

@coderabbitai

coderabbitai Bot commented Apr 10, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Walkthrough

This pull request updates a package reference and refactors the animation and buff management systems. It upgrades the Maple2.File.Parser.Tadeucci package version and replaces the generic AnimationSequence with the more detailed AnimationSequenceMetadata in multiple components. A new AnimationManager class is introduced to consolidate animation control, replacing the old AnimationState paradigm. Enhancements to the BeginCondition conversion and buff resetting logic are also included, along with expanded condition validations in skill utilities.

Changes

File(s) Change Summary
Maple2.File.Ingest/Maple2.File.Ingest.csproj Updated package reference for Maple2.File.Parser.Tadeucci from 2.2.6 to 2.2.7.
Maple2.File.Ingest/Mapper/AnimationMapper.cs, Maple2.Model/Game/Npc/Npc.cs, Maple2.Model/Metadata/AnimationMetadata.cs, Maple2.Model/Metadata/BeginCondition.cs Renamed/updated animation sequence types from AnimationSequence to AnimationSequenceMetadata; adjusted mapping, dictionary types, and property initializations.
Multiple Animation-related Files
Maple2.Server.Game/Commands/DebugCommand.cs, AnimationManager.cs, Actor.cs, FieldActor.cs, FieldNpc.cs, FieldPlayer.cs, IActor.cs, AnimateRoutine.cs, JumpRoutine.cs, MoveRoutine.cs, NpcRoutine.cs, SkillHandler.cs, NpcControlPacket.cs, GameSession.cs, etc.		 Introduced a new AnimationManager class and replaced all references of AnimationState with AnimationManager; updated method calls, property types, and control flow for animation playback and state management.
Maple2.File.Ingest/MapperExtensions.cs, Maple2.Server.Game/Manager/Config/BuffManager.cs, Maple2.Server/Game/Model/Field/Buff.cs Enhanced BeginCondition conversion by adding new properties and a ParseStat method; made buff properties mutable and added a ResetActor method for improved buff state management.
Maple2.Server.Game/Util/SkillUtils.cs Expanded condition validations for both caster and target, including new checks for survival modes, map parameters, stat conditions, and buff exclusions.

Sequence Diagram(s)

sequenceDiagram
    participant S as GameSession
    participant A as Actor (FieldPlayer/FieldActor)
    participant AM as AnimationManager
    participant U as SkillHandler/Other Callers

    S->>AM: Initialize AnimationManager on EnterServer
    A->>AM: Request TryPlaySequence(animationName, speed, type)
    AM-->>A: Return sequence play status
    U->>AM: Update/cancel animation (e.g., on buff or skill events)
    AM->>AM: Process keyframe events and looping logic
    A->>S: Reflect updated animation state
Loading

Assessment against linked issues

Objective Addressed Explanation
Striker class buff durations, combo chaining, and stance logic (#257) ❌ No modifications were made to combat or buff duration/combination logic.

Possibly related PRs

Suggested labels

Field

Poem

I'm a rabbit with a joyful leap,
Hopping through code both wide and deep.
Animation flows now sleek and neat,
With metadata changes that are hard to beat.
I nibble bugs and play with light,
Celebrating fresh code in the night!
🐰✨

✨ Finishing Touches
  • 📝 Generate Docstrings
🪧 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 generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @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.

@Zintixx Zintixx added the Skill / Buff skill and buff related label Apr 10, 2025
coderabbitai[bot]
coderabbitai Bot requested changes Apr 10, 2025
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (20)
Maple2.Server.Game/Model/Skill/SkillQueue.cs (2)

43-45: Looks good, but consider adding XML comment documentation.

The new Contains method is well-implemented and efficiently uses LINQ to check if a skill ID exists in the queue. The null-conditional operator properly handles potential null entries in the array.

Consider adding XML documentation to maintain consistency with code documentation standards:

+    /// <summary>
+    /// Checks if a skill with the specified ID exists in the queue.
+    /// </summary>
+    /// <param name="skillId">The skill ID to search for.</param>
+    /// <returns>True if the skill ID exists in the queue; otherwise, false.</returns>
     public bool Contains(int skillId) {
         return casts.Any(cast => cast?.SkillId == skillId);
     }

43-45: Consider using a for-loop for consistency with other methods.

While the LINQ implementation is concise and effective, it differs from the iterative approach used in the other methods (Get, Remove).

For consistency with the existing codebase style, you might consider using a for-loop implementation:

     public bool Contains(int skillId) {
-        return casts.Any(cast => cast?.SkillId == skillId);
+        for (int i = 0; i < MAX_PENDING; i++) {
+            if (casts[i]?.SkillId == skillId) {
+                return true;
+            }
+        }
+        return false;
     }

This maintains the same functional behavior while following the iteration pattern used in the rest of the class.

Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationRecord.cs (3)

19-26: Consider explicitly initializing all properties in the default constructor.

While most properties are properly initialized, EndTick, LoopEndTick, and LoopOnlyOnce are not explicitly set in the default constructor. They will default to their type's default values, but for better clarity and maintainability, consider explicitly initializing all properties.

public AnimationRecord() {
    Speed = 1;
    LastTime = 0;
    Loop = new LoopData(0, 0);
    IsLooping = false;
    EndTime = 0;
    Type = AnimationType.Misc;
+   EndTick = 0;
+   LoopEndTick = 0;
+   LoopOnlyOnce = false;
}

35-43: Consider using properties instead of public fields in LoopData struct.

The LoopData struct exposes public fields directly. For better encapsulation and future-proofing, consider using properties with getters and setters.

public struct LoopData {
-   public float start;
-   public float end;
+   public float Start { get; set; }
+   public float End { get; set; }

    public LoopData(float start, float end) {
-       this.start = start;
-       this.end = end;
+       Start = start;
+       End = end;
    }
}

45-60: Consider using properties for TickPair struct and adding XML documentation.

Similar to LoopData, the TickPair struct uses public fields instead of properties. Also, while there are comments indicating the purpose of each constructor ("mobs" and "players"), it would be more helpful to add proper XML documentation for better IDE support and clarity.

public struct TickPair {
-   public long server;
-   public long client;
+   public long Server { get; set; }
+   public long Client { get; set; }

-   // mobs
+   /// <summary>
+   /// Creates a new TickPair with identical server and client ticks, typically used for mobs.
+   /// </summary>
+   /// <param name="server">The server tick value</param>
    public TickPair(long server) {
-       this.server = server;
-       this.client = server;
+       Server = server;
+       Client = server;
    }

-   // players
+   /// <summary>
+   /// Creates a new TickPair with separate server and client ticks, typically used for players.
+   /// </summary>
+   /// <param name="server">The server tick value</param>
+   /// <param name="client">The client tick value</param>
    public TickPair(long server, long client) {
-       this.server = server;
-       this.client = client;
+       Server = server;
+       Client = client;
    }
}
Maple2.Server.Game/Model/Field/Actor/Actor.cs (2)

63-63: Validate AnimationManager instantiation.
Currently, the AnimationManager is always created for each Actor. If some actors do not require animation, consider injecting a no-op animation manager or making Animation nullable for clarity.

-Animation = new AnimationManager(this);
+Animation = animationManager ?? new AnimationManager(this);

66-66: Initialize PositionTick with a dedicated struct or method.
Using ValueTuple<Vector3, long, long> is fine, but a small dedicated struct or record improves clarity (e.g. PositionTickRecord). This helps convey meaning and maintain type-safety for the fields.

Maple2.Model/Metadata/BeginCondition.cs (3)

14-15: Validate stored durations for large values.
DurationWithoutDamage and DurationWithoutMoving are stored as int. Consider if these values could exceed the int range under rare conditions. A long or time-based struct (e.g., TimeSpan) might be safer if extremely long durations are valid in the future.

24-27: Possible large arrays for Maps, MapTypes, Continents, ActiveSkill.
Since these arrays can grow large, verify they do not degrade performance or cause memory overhead issues. If needed, consider using specialized data structures or partial loading.

55-55: Consider using records or classes that handle comparisons.
BeginConditionStat relies on multiple fields (attribute, value, compare, valueType). If you need more robust logic or future expansions, reevaluating this structure or introducing helper methods may reduce duplication in multiple condition checks.

Maple2.File.Ingest/MapperExtensions.cs (3)

1-3: Imports appear newly added for reflection usage.
These references (System.ComponentModel, System.Diagnostics, System.Reflection) suggest new reflection-based logic or annotations. Confirm they are truly needed and that reflection usage is optimal for performance-critical paths.

322-327: Refine local dungeonGroupType list usage.
The dungeonGroupType list accumulates valid enums, then is not used directly in the returned BeginCondition. It’s fine, but you could inline this logic directly in the final DungeonGroupType property or reuse the local list for clarity.

401-401: Empty array usage.
Returning [] is simpler than Array.Empty<BeginConditionTarget.HasBuff>(), but watch for potential extra allocations if called frequently in large loops. Typically, Array.Empty<T>() is recommended to avoid repeated allocations.

Maple2.Server.Game/Manager/AnimationManager.cs (4)

15-18: Consider using private fields with public properties for Current and queued.
Restricting these to private scope and exposing them through properties can help encapsulate state and maintain consistent access patterns throughout the class.

- public AnimationRecord? Current;
- private AnimationRecord? queued;
+ private AnimationRecord? _current;
+ private AnimationRecord? _queued;
+
+ public AnimationRecord? Current => _current;

56-59: Log a warning when RigMetadata is null.
Currently, the constructor silently defaults to IdleSequenceId = 0 if metadata is missing. Logging a warning or error can help with debugging missing or invalid metadata.

if (RigMetadata is null) {
+   DebugPrint("Warning: RigMetadata is null. No valid animation data found.");
    IdleSequenceId = 0;
    return;
}

204-275: Consider extracting sub-methods in Update(...).
This method handles numerous responsibilities (timing, looping, keyframe events, sequence end checks, etc.). Extracting separate methods for each major step (e.g., HandleKeyframes, HandleLooping) can improve readability and maintainability.

353-381: Ensure keyframe events are idempotent.
Inside HitKeyframe(...), repeated triggers in the same tick could become problematic if the server timing or floating-point comparisons fire the event multiple times. You might want to store the last triggered keyframe or time to prevent double-firing.

Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs (3)

75-78: Revisit exposing AnimationSequenceMetadata fields as public readonly.
Declaring these as public readonly is acceptable if they're truly meant to be exposed directly. Otherwise, consider private fields with public getters or properties for stronger encapsulation.

230-230: Avoid repeated dictionary lookups and validate presence.
In lines 230 and 246, you perform lookups on Value.Animations using TryGetValue(routineName, out AnimationSequenceMetadata? ...). Consider capturing the result once, or gracefully handling missing sequences in a single code path.

Also applies to: 246-246

309-309: Check for missing or invalid sequences in Animate(string sequenceName, ...).
A logger error is placed, but no fallback logic is used if the sequence is missing. Consider gracefully handling or defaulting to idle.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 15ead17 and 822880a.

📒 Files selected for processing (34)
  • Maple2.File.Ingest/Maple2.File.Ingest.csproj (1 hunks)
  • Maple2.File.Ingest/Mapper/AnimationMapper.cs (1 hunks)
  • Maple2.File.Ingest/MapperExtensions.cs (4 hunks)
  • Maple2.Model/Enum/CompareType.cs (1 hunks)
  • Maple2.Model/Game/Npc/Npc.cs (1 hunks)
  • Maple2.Model/Metadata/AnimationMetadata.cs (1 hunks)
  • Maple2.Model/Metadata/BeginCondition.cs (3 hunks)
  • Maple2.Server.Game/Commands/DebugCommand.cs (2 hunks)
  • Maple2.Server.Game/Manager/AnimationManager.cs (1 hunks)
  • Maple2.Server.Game/Manager/Config/BuffManager.cs (1 hunks)
  • Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/Actor.cs (2 hunks)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationRecord.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationState.cs (0 hunks)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementState.cs (5 hunks)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.Emote.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.SkillCast.cs (2 hunks)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.Walk.cs (3 hunks)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.EmoteTask.cs (2 hunks)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.SkillCastTask.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/FieldActor.cs (2 hunks)
  • Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs (6 hunks)
  • Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/IActor.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/Routine/AnimateRoutine.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/Routine/JumpRoutine.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/Routine/MoveRoutine.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/Routine/NpcRoutine.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Buff.cs (2 hunks)
  • Maple2.Server.Game/Model/Skill/SkillQueue.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/SkillHandler.cs (5 hunks)
  • Maple2.Server.Game/Packets/NpcControlPacket.cs (1 hunks)
  • Maple2.Server.Game/Session/GameSession.cs (2 hunks)
  • Maple2.Server.Game/Util/SkillUtils.cs (4 hunks)
💤 Files with no reviewable changes (1)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationState.cs
🧰 Additional context used
🧬 Code Graph Analysis (17)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.Emote.cs (1)
Maple2.Server.Game/Manager/AnimationManager.cs (2)
  • TryPlaySequence (105-122)
  • TryPlaySequence (133-152)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs (4)
Maple2.Server.Game/Model/Field/Buff.cs (1)
  • ResetActor (58-65)
Maple2.Server.Game/Manager/Config/BuffManager.cs (1)
  • ResetActor (50-55)
Maple2.Server.Game/Manager/AnimationManager.cs (1)
  • ResetActor (81-83)
Maple2.Server.Game/Manager/StatsManager.cs (1)
  • ResetActor (137-139)
Maple2.Server.Game/Model/Field/Actor/Routine/MoveRoutine.cs (3)
Maple2.Server.Game/Model/Field/Actor/Routine/WaitRoutine.cs (2)
  • WaitRoutine (3-19)
  • WaitRoutine (6-8)
Maple2.Server.Game/Model/Field/Actor/Routine/NpcRoutine.cs (2)
  • NpcRoutine (6-39)
  • NpcRoutine (15-19)
Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs (2)
  • FieldNpc (22-474)
  • FieldNpc (97-128)
Maple2.Server.Game/Model/Field/Actor/Routine/JumpRoutine.cs (3)
Maple2.Server.Game/Model/Field/Actor/Routine/WaitRoutine.cs (2)
  • WaitRoutine (3-19)
  • WaitRoutine (6-8)
Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs (2)
  • FieldNpc (22-474)
  • FieldNpc (97-128)
Maple2.Server.Game/Model/Field/Actor/State/StateJumpNpc.cs (3)
  • StateJumpNpc (8-57)
  • StateJumpNpc (26-35)
  • StateJumpNpc (37-44)
Maple2.Server.Game/Manager/Config/BuffManager.cs (2)
Maple2.Server.Game/Model/Field/Buff.cs (3)
  • Buff (13-299)
  • Buff (41-56)
  • ResetActor (58-65)
Maple2.Server.Game/Manager/StatsManager.cs (1)
  • ResetActor (137-139)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.SkillCast.cs (2)
Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs (1)
  • AppendDebugMessage (429-451)
Maple2.Server.Game/Manager/AnimationManager.cs (1)
  • GetSequenceSegmentTime (310-345)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.Walk.cs (1)
Maple2.Server.Game/Manager/AnimationManager.cs (2)
  • TryPlaySequence (105-122)
  • TryPlaySequence (133-152)
Maple2.Model/Game/Npc/Npc.cs (2)
Maple2.File.Ingest/MapperExtensions.cs (4)
  • IReadOnlyDictionary (279-293)
  • Dictionary (17-27)
  • Dictionary (29-39)
  • Dictionary (457-516)
Maple2.Database/Storage/Metadata/NpcMetadataStorage.cs (1)
  • AnimationMetadata (73-89)
Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (4)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs (1)
  • FieldPlayer (62-100)
Maple2.Server.Game/Session/GameSession.cs (3)
  • GameSession (37-805)
  • GameSession (106-116)
  • GameSession (698-698)
Maple2.Server.Game/Session/GameSession.State.cs (1)
  • GameSession (12-60)
Maple2.Model/Game/User/Player.cs (2)
  • Player (8-22)
  • Player (17-21)
Maple2.Server.Game/Model/Field/Actor/FieldActor.cs (1)
Maple2.Server.Game/Manager/AnimationManager.cs (3)
  • AnimationManager (14-400)
  • AnimationManager (48-68)
  • AnimationManager (70-79)
Maple2.Server.Game/Packets/NpcControlPacket.cs (5)
Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs (1)
  • Talk (319-324)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementState.cs (2)
  • MovementState (11-310)
  • MovementState (37-48)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.EmoteTask.cs (1)
  • MovementState (7-53)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.TalkTask.cs (1)
  • MovementState (6-24)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.WalkTask.cs (1)
  • MovementState (7-182)
Maple2.Server.Game/Util/SkillUtils.cs (3)
Maple2.Server.Game/Model/Stats.cs (7)
  • Stat (141-199)
  • Stat (149-149)
  • Stat (151-155)
  • Stats (8-139)
  • Stats (17-33)
  • Stats (35-41)
  • Total (106-116)
Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (2)
  • FieldPlayer (15-525)
  • FieldPlayer (90-103)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs (1)
  • FieldPlayer (62-100)
Maple2.Server.Game/Model/Field/Actor/IActor.cs (1)
Maple2.Server.Game/Manager/AnimationManager.cs (3)
  • AnimationManager (14-400)
  • AnimationManager (48-68)
  • AnimationManager (70-79)
Maple2.Model/Metadata/BeginCondition.cs (2)
Maple2.File.Ingest/MapperExtensions.cs (1)
  • BeginConditionTarget (362-455)
Maple2.File.Ingest/Utils/AttributeExtensions.cs (1)
  • BasicAttribute (6-44)
Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs (9)
Maple2.Server.Game/Manager/Field/Navigation.cs (3)
  • DtCrowdAgent (51-56)
  • Navigation (15-76)
  • Navigation (24-34)
Maple2.Model/Game/Npc/Npc.cs (2)
  • Npc (6-18)
  • Npc (14-17)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementState.cs (10)
  • MovementState (11-310)
  • MovementState (37-48)
  • NpcTask (50-60)
  • NpcTask (62-73)
  • NpcTask (75-85)
  • NpcTask (87-90)
  • NpcTask (92-95)
  • NpcTask (97-99)
  • NpcTask (114-119)
  • NpcTask (121-123)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.EmoteTask.cs (1)
  • MovementState (7-53)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.SkillCastTask.cs (1)
  • MovementState (10-124)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.TalkTask.cs (1)
  • MovementState (6-24)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.CleanupTask.cs (1)
  • MovementState (6-21)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.StandbyTask.cs (1)
  • MovementState (7-33)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.WalkTask.cs (1)
  • MovementState (7-182)
Maple2.Server.Game/Manager/AnimationManager.cs (6)
Maple2.Database/Storage/Metadata/NpcMetadataStorage.cs (1)
  • AnimationMetadata (73-89)
Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (3)
  • FieldPlayer (15-525)
  • FieldPlayer (90-103)
  • KeyframeEvent (506-508)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs (3)
  • FieldPlayer (62-100)
  • FieldNpc (102-122)
  • FieldNpc (124-126)
Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs (3)
  • FieldNpc (22-474)
  • FieldNpc (97-128)
  • KeyframeEvent (220-222)
Maple2.Server.Game/Model/Field/Actor/IActor.cs (1)
  • KeyframeEvent (30-30)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationRecord.cs (1)
  • LoopData (39-42)
Maple2.Server.Game/Model/Field/Actor/Actor.cs (3)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/SkillState.cs (2)
  • SkillState (8-68)
  • SkillState (11-13)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementState.cs (1)
  • Update (184-229)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/BattleState.cs (1)
  • Update (57-89)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: build
  • GitHub Check: build
🔇 Additional comments (70)
Maple2.Model/Enum/CompareType.cs (1)

10-14: Well-structured enum addition for stat comparison types

The new CompareStatValueType enum is cleanly implemented and follows proper C# enum conventions. Its purpose is clear - to distinguish between comparing stat values as percentages versus absolute values. This addition aligns well with the PR objective of implementing additional BeginConditions for skills, particularly for conditions that need to perform statistical comparisons.

Maple2.File.Ingest/Maple2.File.Ingest.csproj (1)

20-28: Update Package Version Verification

The package reference for Maple2.File.Parser.Tadeucci has been updated from version 2.2.6 to 2.2.7. This bump likely introduces enhancements or fixes needed for the updated animation handling logic (e.g., renaming to AnimationSequenceMetadata and the transition to AnimationManager). Please ensure that version 2.2.7 is fully compatible with the rest of the codebase and does not introduce any breaking changes.

Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationRecord.cs (2)

1-18: Class design looks good with well-defined properties.

The AnimationRecord class has a clear purpose with appropriate properties for managing animation state. The properties cover animation sequence metadata, timing, looping behavior, and optional skill metadata, providing a comprehensive model for animation tracking.

28-34: LGTM! Good constructor chaining pattern.

The parameterized constructor properly calls the default constructor using : this() and then sets the specific properties. This is a good practice to avoid code duplication.

Maple2.Server.Game/Manager/Config/BuffManager.cs (1)

52-54: Great addition to prevent orphaned buff references!

This change ensures that when an actor is reset, all buffs in the collection update their internal references to that actor. Without this update, buffs might hold stale references to the previous actor instance, which could lead to inconsistent state or potential bugs.

Maple2.Server.Game/Model/Field/Actor/Routine/AnimateRoutine.cs (2)

9-9: Parameter type change consistent with metadata refactor

The constructor parameter has been updated from AnimationSequence to AnimationSequenceMetadata to align with the broader refactoring of animation-related classes mentioned in the PR objectives.

15-15: Property access updated for new type

The property access has been updated to use sequenceMetadata.Time instead of sequence.Time, maintaining the same functionality with the new type.

Maple2.Model/Metadata/AnimationMetadata.cs (1)

5-5: Type update in AnimationMetadata record

The Sequences property type has been updated from IReadOnlyDictionary<string, AnimationSequence> to IReadOnlyDictionary<string, AnimationSequenceMetadata> as part of the rename.

Maple2.File.Ingest/Mapper/AnimationMapper.cs (3)

18-18: Type update in sequences collection

The collection type has been updated from IEnumerable<(string Name, AnimationSequence Sequence)> to IEnumerable<(string Name, AnimationSequenceMetadata Sequence)> to match the record type change.

21-24: Constructor and default time value changes

Two changes here:

  1. Updated constructor to use AnimationSequenceMetadata instead of AnimationSequence
  2. Changed the default time value from default to explicitly 0 when no "end" key is found

Both changes maintain the same behavior while aligning with the new naming convention.

28-30: Improved dictionary handling with TryAdd

The change from lookup.Add(name, sequence) to lookup.TryAdd(name, sequence) is a safer approach that avoids throwing exceptions for duplicate keys, instead checking the return value to determine success. This is a good defensive programming practice.

Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.Emote.cs (1)

19-19: Correct property update from AnimationState to Animation.

The code now uses actor.Animation instead of actor.AnimationState to play the emote sequence. This change aligns with the broader refactoring where AnimationState has been renamed to AnimationManager and the property name has been updated to Animation.

Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs (1)

74-74: Added Animation reset for consistent actor initialization.

This addition ensures that the player's animation state is properly reset when spawning, which complements the existing resets for stats and buffs. This is a good practice for maintaining consistent state initialization.

Maple2.Server.Game/Model/Field/Actor/Routine/JumpRoutine.cs (2)

43-43: Updated to use IdleSequenceMetadata instead of IdleSequence.

The code has been modified to use the new IdleSequenceMetadata property instead of the previous IdleSequence property, which is consistent with the codebase-wide refactoring of animation sequence handling.

51-51: Updated to use IdleSequenceMetadata instead of IdleSequence.

The change from IdleSequence to IdleSequenceMetadata ensures consistency with the updated animation system throughout the codebase. This maintains proper animation handling after the jump routine completes.

Maple2.Server.Game/Model/Field/Actor/Routine/MoveRoutine.cs (2)

13-13: Updated to use IdleSequenceMetadata instead of IdleSequence.

The code has been modified to use the new IdleSequenceMetadata property instead of the previous IdleSequence property in the error handling case. This ensures consistency with the animation system refactoring.

21-21: Updated to use IdleSequenceMetadata instead of IdleSequence.

Similar to the change in the Walk method, this update to use IdleSequenceMetadata in the Run method's error handling ensures consistent animation handling throughout the codebase.

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

34-34: Property name updated for consistency with refactored animation system

The change from Npc.IdleSequence.Id to Npc.IdleSequenceMetadata.Id aligns with the broader refactoring of the animation system, where AnimationSequence has been renamed to AnimationSequenceMetadata. This ensures consistent naming throughout the codebase.

Maple2.Server.Game/Commands/DebugCommand.cs (1)

98-98: Updated animation debug property access

The change from session.Player.AnimationState.DebugPrintAnimations to session.Player.Animation.DebugPrintAnimations correctly reflects the new animation management system, where the AnimationState property has been renamed to Animation and uses the new AnimationManager class.

Maple2.Server.Game/Session/GameSession.cs (2)

103-103: Added AnimationManager property to GameSession

The new Animation property of type AnimationManager is part of the animation system refactoring. This property will hold the animation management functionality for the game session.

151-151: Initialized AnimationManager during EnterServer

The Animation property is properly initialized in the EnterServer method with a new instance of AnimationManager passing the current session. This ensures the animation manager is set up when a player enters the server.

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

18-18: Updated IActor interface to use AnimationManager

The AnimationState property has been replaced with Animation of type AnimationManager. This change is consistent with the animation system refactoring, allowing actors to access the new animation management functionality through a standard getter instead of the previous init accessor.

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

28-28: Animation system refactoring looks good.

The change from AnimationState to AnimationManager aligns with the PR objectives which mentioned renaming AnimationState to AnimationManager. The property and initialization have been consistently updated.

Also applies to: 37-37

Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.EmoteTask.cs (1)

39-39: Correctly updated animation references.

The references to actor.AnimationState have been correctly updated to actor.Animation to align with the new animation system architecture. Both changes look consistent with the animation system refactoring.

Also applies to: 49-49

Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.SkillCast.cs (2)

26-27: Animation property reference updated properly.

The code correctly updates the animation reference check from AnimationState to Animation in line with the system-wide refactoring.

107-107: Animation method call updated appropriately.

The call to GetSequenceSegmentTime has been properly updated to use the new Animation property instead of AnimationState. Based on the provided context, this method remains functionally equivalent in the new AnimationManager class.

Maple2.Model/Game/Npc/Npc.cs (1)

8-8: Animation data structure type updated correctly.

The change from AnimationSequence to AnimationSequenceMetadata aligns with the PR objectives that mentioned renaming AnimationSequence to AnimationSequenceMetadata. Both the property type declaration and initialization have been updated consistently.

Also applies to: 16-16

Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.SkillCastTask.cs (1)

100-100: Property name and type change refactor for Animation system.

The code has been updated to use actor.Animation.TryPlaySequence with a return type of AnimationSequenceMetadata? instead of the previous actor.AnimationState.TryPlaySequence with AnimationSequence?. This change is consistent with the broader refactoring where AnimationState has been renamed to AnimationManager and AnimationSequence to AnimationSequenceMetadata.

Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementState.cs (5)

17-17: Type change refactor for Animation system.

The stateSequence variable type has been updated from AnimationSequence? to AnimationSequenceMetadata? as part of the animation system refactoring.

40-40: Property name change refactor for Animation system.

The code now references actor.Animation.RigMetadata instead of actor.AnimationState.RigMetadata as part of the animation system refactoring.

144-148: Property name change refactor for Animation system.

These changes update the code to use the new Animation property instead of AnimationState and reflect the type change from AnimationSequence to AnimationSequenceMetadata. The animation sequence playing and assignment logic remains unchanged.

185-185: Property name change refactor for Animation system.

Changed to use actor.Animation.PlayingSequence instead of actor.AnimationState.PlayingSequence to match the renamed property.

206-209: Property name change refactor for Animation system.

Both instances of the AnimationState property references have been updated to use the new Animation property while maintaining the same functionality.

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

19-20: Changed readonly fields to properties with private setters.

The Caster and Owner fields have been changed from readonly fields to properties with private setters. This change allows for updating these references after initialization, which is used by the new ResetActor method.

58-65: Added new method to update actor references.

This new ResetActor method allows updating the Caster or Owner references when an actor with the same ObjectId is provided. This is useful when actors need to be reloaded or reconnected while maintaining buff associations.

Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.Walk.cs (3)

25-25: Type change refactor for Animation system.

The walkSequence variable type has been updated from AnimationSequence? to AnimationSequenceMetadata? as part of the animation system refactoring.

48-49: Property name change refactor for Animation system.

Changed to use actor.Animation.PlayingSequence and actor.Animation.TryPlaySequence instead of the previous AnimationState equivalents. This matches the renamed property being used throughout the codebase.

169-170: Property name change refactor for Animation system.

Updated the code to use the new Animation property instead of AnimationState while maintaining the same functionality for playing and assigning animation sequences.

Maple2.Server.Game/PacketHandlers/SkillHandler.cs (6)

161-161: Consistent use of animation parameters

The TryPlaySequence method now accepts metadata as an additional parameter, providing the sequence with skill metadata context. This is a good improvement as it makes the animation system more aware of the skill being used.

338-340: Animation property access update

The code now uses session.Player.Animation.PlayingSequence instead of the previous AnimationState pattern. This aligns with the PR objective of renaming AnimationState to AnimationManager.

360-360: Consistent method access pattern

Now using session.Animation directly instead of accessing it through the player. This standardizes the animation access pattern, though I notice this call doesn't pass the metadata parameter unlike line 161.

Was the omission of the metadata parameter at this location intentional? The pattern is inconsistent with line 161 where metadata is included.

378-378: Updated animation reference

Correctly updated reference to access the animation sequence name through the new Animation property structure.

385-385: Updated sequence loop method

Correctly updating the SetLoopSequence call to use the new Animation property path.

403-403: Animation cancellation access update

Now using the direct session.Animation reference to cancel sequences instead of going through the player object. This standardizes the animation access pattern throughout the code.

Maple2.Server.Game/Util/SkillUtils.cs (8)

85-87: Added survival mode condition check

Good addition to verify if the caster's field is a survival-type map before allowing skill execution. This enhances game mode-specific skill restrictions.

110-114: Added duration without moving condition

This new check validates if the player has remained stationary for a specified duration. Useful for skills that require standing still to cast.

115-123: Added map-specific conditions

These new checks verify if the skill is being used in appropriate maps, map types, and continents. Great for restricting skills to specific game areas.

124-128: Added dungeon group type check

This ensures skills only work in specific dungeon types by checking if the field is a DungeonFieldManager and if it matches the required dungeon group types.

129-131: Added active skill check

Verifies that the owner's current animation skill ID matches one of the specified IDs in the condition. Useful for skill combos or skills that can only be used during specific other animations.

166-184: Added target stat comparison logic

This sophisticated check allows comparing various target stats using different comparison types and value types (current percentage or total value). Great enhancement for conditional skills that depend on target stats.

186-188: Added negative buff check

This condition ensures the target doesn't have specific buffs, which is useful for skills that shouldn't work on targets with certain status effects.

190-207: Added actor type-specific conditions

Great enhancement that adds specialized checks depending on whether the target is a player (checking states, sub-states, and masteries) or an NPC (checking NPC IDs). This allows for more granular control over skill targeting.

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

90-92: Simplified constructor and animation assignment

The constructor now directly uses session.NpcMetadata instead of a separate method call, and properly assigns the Animation property from the session. This aligns with the animation system refactoring and simplifies the code.

Maple2.Server.Game/Packets/NpcControlPacket.cs (4)

49-49: Updated animation property access

Now accessing sequence speed via npc.Animation instead of AnimationState, consistent with the PR's renaming objective.

55-55: Updated idle sequence ID access

Now retrieving the idle sequence ID from the Animation property rather than AnimationState.

62-62: Updated playing sequence access

The code now accesses the playing sequence through the Animation property, maintaining the same null-coalescing pattern with the default sequence ID.

67-67: Updated jump sequence check

Now checking if the animation is a jump sequence through the Animation property instead of AnimationState.

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

43-43: Ensure property immutability is intended.
Declaring Animation with an init; accessor ensures it cannot be changed after object construction. If you need to reassign or reload the animation system at runtime, consider using a set; accessor or a method-based approach.

Please confirm that Animation will remain immutable post-construction throughout the actor's lifecycle.

51-55: Consider concurrency and data consistency for PositionTick.
The newly introduced property may be accessed or updated in a multi-threaded context. Confirm threadsafety measures to avoid race conditions or data inconsistencies when reading/writing position data and duration.

56-56: Constructor initialization logic check.
This constructor sets up the new PositionTick and Animation fields. Ensure any future delegated instantiation of AnimationManager or BuffManager occurs here or is clearly documented if moved elsewhere.

Maple2.Model/Metadata/BeginCondition.cs (4)

18-18: Confirm survival-only logic.
OnlySurvival seems to limit usage to certain contexts. Verify that other logic segments or tools do not incorrectly override or ignore this flag, potentially allowing the condition to trigger outside survival maps.

20-21: Check for contradictory conditions regarding mounts.
AllowOnBattleMount and OnlyOnBattleMount can conflict if both are set incorrectly. Ensure that your condition builder enforces a meaningful combination (e.g., if OnlyOnBattleMount is true, AllowOnBattleMount might be redundant or must also be true).

22-22: Ensure DungeonGroupType array is validated.
When automatically parsing the array, confirm that invalid or unrecognized types are handled gracefully. Consider logging or ignoring unknown entries to prevent unexpected condition triggers.

39-46: Expanded target condition checks.
These new target-based properties (Event, Stat, States, SubStates, Masteries, etc.) offer fine-grained conditions. Ensure that the logic which interprets them is well-tested, especially in edge cases (e.g., multiple states or partial mastery data).

Maple2.File.Ingest/MapperExtensions.cs (3)

361-361: Default condition target might omit newly added fields.
Using empty arrays and null event conditions is fine, but verify that no other property leads to unintended default logic (e.g., default HasNotBuffIds or States). A domain-specific fallback might be clearer.

369-395: Parsing expanded sub-condition fields.
Successfully integrates new fields: Event, Stat, States, SubStates, Masteries, NpcIds, HasNotBuffIds. Ensure the caller properly handles a null return if DefaultBeginConditionTarget.Equals(result).

433-455: Verify parse logic for compareStat.
This block enumerates each compareStat and sets the first attribute found with a non-default float. If multiple attributes in the same record are non-zero, only the first will be stored. Confirm that’s intended or consider storing all valid attributes if the design demands it.

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

67-68: Validate dictionary lookups for idle sequence.
Using .Value.Id without checking if FirstOrDefault returned a default can lead to a KeyNotFoundException if the sequence does not exist. Consider null checks or a safer fallback.

Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs (2)

97-98: Handle fallback behavior when no valid idle animation is found.
Your fallback uses an empty AnimationSequenceMetadata with id = -1. Ensure this prevents runtime errors if the NPC tries to play a nonexistent idle animation.

261-261: Validate waypoint-based lookup results.
When retrieving arriveSequence (line 261) and patrolSequence (line 269), confirm that you handle null returns. Missing sequences can break the NPC's patrol routine.

Also applies to: 269-269

Comment thread Maple2.Model/Metadata/AnimationMetadata.cs
public record AnimationMetadata(string Model, IReadOnlyDictionary<string, AnimationSequenceMetadata> Sequences);

public record AnimationSequence(string Name, short Id, float Time, List<AnimationKey>? Keys);
public record AnimationSequenceMetadata(string Name, short Id, float Time, List<AnimationKey> Keys);

@coderabbitai coderabbitai Bot Apr 10, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Nullable annotation removed from Keys property

The Keys property is no longer marked as nullable (List<AnimationKey> instead of List<AnimationKey>?). Ensure that all initialization paths provide a non-null list to avoid potential null reference exceptions.

🏁 Script executed:

#!/bin/bash
# Search for places where AnimationSequenceMetadata is created to verify Keys is never null
rg -A 3 "new AnimationSequenceMetadata\(" --type cs

Length of output: 1095

Action Required: Ensure Non-Null Initialization for the Keys Property

Our investigation revealed that the instantiation in Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs is passing null to the Keys parameter:

  • Problematic instance:

    new AnimationSequenceMetadata(string.Empty, -1, 1f, null);

    Since the record now defines Keys as a non-nullable List<AnimationKey>, passing null risks causing a null reference exception.

Next Steps:

  • Update this instantiation to provide a valid non-null List<AnimationKey> (e.g., an empty list) to ensure safe operation.
  • Verify similar instantiation paths across the codebase for consistency.

Comment thread Maple2.Server.Game/Model/Field/Actor/Actor.cs
Comment thread Maple2.Server.Game/Manager/AnimationManager.cs
coderabbitai[bot]
coderabbitai Bot requested changes Apr 10, 2025
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🧹 Nitpick comments (11)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationRecord.cs (4)

6-18: Consider adding XML documentation comments to improve code readability.

The AnimationRecord class appears to be a central component for animation management, but lacks XML documentation. Adding summary comments for the class and each property would help other developers understand the purpose and usage of this component.

+ /// <summary>
+ /// Manages the state of an animation, including sequence metadata, timing, and looping behavior.
+ /// </summary>
 public class AnimationRecord {
+    /// <summary>
+    /// The animation sequence metadata that defines the animation.
+    /// </summary>
     public AnimationSequenceMetadata? Sequence { get; set; }
+    /// <summary>
+    /// The playback speed multiplier for the animation.
+    /// </summary>
     public float Speed { get; set; }

35-43: Consider making LoopData immutable to prevent unexpected behavior.

LoopData is defined as a mutable struct with public fields, which can lead to unexpected behavior in some scenarios. Consider making it immutable with readonly fields and properties.

-    public struct LoopData {
-        public float start;
-        public float end;
+    public readonly struct LoopData {
+        public readonly float Start;
+        public readonly float End;
 
         public LoopData(float start, float end) {
-            this.start = start;
-            this.end = end;
+            Start = start;
+            End = end;
         }
     }

45-60: Consider making TickPair immutable and adding descriptive comments.

Similar to LoopData, TickPair is a mutable struct with public fields. Additionally, while there are comments indicating which constructor is for mobs vs. players, consider adding more comprehensive documentation.

-    public struct TickPair {
-        public long server;
-        public long client;
+    /// <summary>
+    /// Represents a pair of server and client tick values for synchronizing animations.
+    /// </summary>
+    public readonly struct TickPair {
+        public readonly long Server;
+        public readonly long Client;
 
-        // mobs
+        /// <summary>
+        /// Creates a new TickPair with identical server and client ticks, typically used for mobs.
+        /// </summary>
+        /// <param name="server">The server-side tick value</param>
         public TickPair(long server) {
-            this.server = server;
-            this.client = server;
+            Server = server;
+            Client = server;
         }
 
-        // players
+        /// <summary>
+        /// Creates a new TickPair with separate server and client ticks, typically used for players.
+        /// </summary>
+        /// <param name="server">The server-side tick value</param>
+        /// <param name="client">The client-side tick value</param>
         public TickPair(long server, long client) {
-            this.server = server;
-            this.client = client;
+            Server = server;
+            Client = client;
         }
     }

6-61: Consider implementing equality members for value types.

Both LoopData and TickPair are structs (value types) but don't override equality members. For value types that will be compared, it's good practice to implement proper equality comparison.

For LoopData and TickPair, consider implementing Equals, GetHashCode, and the == and != operators.

Here's an example for LoopData:

public readonly struct LoopData : IEquatable<LoopData> {
    public readonly float Start;
    public readonly float End;

    public LoopData(float start, float end) {
        Start = start;
        End = end;
    }

    public override bool Equals(object? obj) => obj is LoopData other && Equals(other);

    public bool Equals(LoopData other) => Start == other.Start && End == other.End;

    public override int GetHashCode() => HashCode.Combine(Start, End);

    public static bool operator ==(LoopData left, LoopData right) => left.Equals(right);

    public static bool operator !=(LoopData left, LoopData right) => !left.Equals(right);
}
Maple2.Model/Metadata/BeginCondition.cs (4)

14-15: Clarify duration units.
The newly added DurationWithoutDamage and DurationWithoutMoving fields store time in integers but, as seen in MapperExtensions.cs, they are treated as milliseconds. Consider updating the naming or adding documentation to avoid confusion about the time unit.

22-22: Minor naming consideration.
The property name DungeonGroupType might be clearer if it were plural (e.g., DungeonGroupTypes) since it holds an array.

24-27: Array-based conditions look good.
Storing new map-related conditions as arrays is consistent with the rest of the design. Alternatively, consider IReadOnlyList or IReadOnlyCollection for consistent immutability semantics.

55-55: Float precision consideration.
If attribute comparisons need precise calculations, confirm that float is sufficient. Otherwise, this is a valid addition for the new stat condition.

Maple2.File.Ingest/MapperExtensions.cs (1)

399-401: Returning empty array instead of Array.Empty<>().
While returning [] works, using Array.Empty<BeginConditionTarget.HasBuff>() could clarify the intent and possibly improve performance.

Maple2.Server.Game/Manager/AnimationManager.cs (2)

11-14: Mismatch in documentation.
The summary refers to “AnimationState” instead of “AnimationManager.” Update to maintain clarity and consistency.

-/// Initializes a new instance of the AnimationState class.
+/// Initializes a new instance of the AnimationManager class.

15-32: Visibility of fields vs. properties.
public AnimationRecord? Current; is exposed as a field; consider using a property to enforce encapsulation in the future.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 15ead17 and 822880a.

📒 Files selected for processing (34)
  • Maple2.File.Ingest/Maple2.File.Ingest.csproj (1 hunks)
  • Maple2.File.Ingest/Mapper/AnimationMapper.cs (1 hunks)
  • Maple2.File.Ingest/MapperExtensions.cs (4 hunks)
  • Maple2.Model/Enum/CompareType.cs (1 hunks)
  • Maple2.Model/Game/Npc/Npc.cs (1 hunks)
  • Maple2.Model/Metadata/AnimationMetadata.cs (1 hunks)
  • Maple2.Model/Metadata/BeginCondition.cs (3 hunks)
  • Maple2.Server.Game/Commands/DebugCommand.cs (2 hunks)
  • Maple2.Server.Game/Manager/AnimationManager.cs (1 hunks)
  • Maple2.Server.Game/Manager/Config/BuffManager.cs (1 hunks)
  • Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/Actor.cs (2 hunks)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationRecord.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationState.cs (0 hunks)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementState.cs (5 hunks)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.Emote.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.SkillCast.cs (2 hunks)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.Walk.cs (3 hunks)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.EmoteTask.cs (2 hunks)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.SkillCastTask.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/FieldActor.cs (2 hunks)
  • Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs (6 hunks)
  • Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/IActor.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/Routine/AnimateRoutine.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/Routine/JumpRoutine.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/Routine/MoveRoutine.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/Routine/NpcRoutine.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Buff.cs (2 hunks)
  • Maple2.Server.Game/Model/Skill/SkillQueue.cs (1 hunks)
  • Maple2.Server.Game/PacketHandlers/SkillHandler.cs (5 hunks)
  • Maple2.Server.Game/Packets/NpcControlPacket.cs (1 hunks)
  • Maple2.Server.Game/Session/GameSession.cs (2 hunks)
  • Maple2.Server.Game/Util/SkillUtils.cs (4 hunks)
💤 Files with no reviewable changes (1)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationState.cs
🧰 Additional context used
🧬 Code Graph Analysis (16)
Maple2.Server.Game/Model/Field/Actor/Routine/JumpRoutine.cs (4)
Maple2.Server.Game/Model/Field/Actor/Routine/WaitRoutine.cs (2)
  • WaitRoutine (3-19)
  • WaitRoutine (6-8)
Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs (2)
  • FieldNpc (22-474)
  • FieldNpc (97-128)
Maple2.Model/Game/Npc/Npc.cs (2)
  • Npc (6-18)
  • Npc (14-17)
Maple2.Server.Game/Model/Field/Actor/State/StateJumpNpc.cs (3)
  • StateJumpNpc (8-57)
  • StateJumpNpc (26-35)
  • StateJumpNpc (37-44)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs (4)
Maple2.Server.Game/Model/Field/Buff.cs (1)
  • ResetActor (58-65)
Maple2.Server.Game/Manager/Config/BuffManager.cs (1)
  • ResetActor (50-55)
Maple2.Server.Game/Manager/AnimationManager.cs (1)
  • ResetActor (81-83)
Maple2.Server.Game/Manager/StatsManager.cs (1)
  • ResetActor (137-139)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.SkillCast.cs (2)
Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs (1)
  • AppendDebugMessage (429-451)
Maple2.Server.Game/Manager/AnimationManager.cs (1)
  • GetSequenceSegmentTime (310-345)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.SkillCastTask.cs (1)
Maple2.Server.Game/Manager/AnimationManager.cs (2)
  • TryPlaySequence (105-122)
  • TryPlaySequence (133-152)
Maple2.Server.Game/Manager/Config/BuffManager.cs (2)
Maple2.Server.Game/Model/Field/Buff.cs (3)
  • Buff (13-299)
  • Buff (41-56)
  • ResetActor (58-65)
Maple2.Server.Game/Manager/StatsManager.cs (1)
  • ResetActor (137-139)
Maple2.Server.Game/Util/SkillUtils.cs (3)
Maple2.Server.Game/Model/Stats.cs (7)
  • Stat (141-199)
  • Stat (149-149)
  • Stat (151-155)
  • Stats (8-139)
  • Stats (17-33)
  • Stats (35-41)
  • Total (106-116)
Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (2)
  • FieldPlayer (15-525)
  • FieldPlayer (90-103)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs (1)
  • FieldPlayer (62-100)
Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (4)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs (1)
  • FieldPlayer (62-100)
Maple2.Server.Game/Session/GameSession.cs (3)
  • GameSession (37-805)
  • GameSession (106-116)
  • GameSession (698-698)
Maple2.Server.Game/Session/GameSession.State.cs (1)
  • GameSession (12-60)
Maple2.Model/Game/User/Player.cs (2)
  • Player (8-22)
  • Player (17-21)
Maple2.Server.Game/Model/Field/Actor/IActor.cs (1)
Maple2.Server.Game/Manager/AnimationManager.cs (3)
  • AnimationManager (14-400)
  • AnimationManager (48-68)
  • AnimationManager (70-79)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.EmoteTask.cs (1)
Maple2.Server.Game/Manager/AnimationManager.cs (2)
  • TryPlaySequence (105-122)
  • TryPlaySequence (133-152)
Maple2.Model/Metadata/AnimationMetadata.cs (1)
Maple2.Database/Storage/Metadata/NpcMetadataStorage.cs (2)
  • AnimationMetadata (73-89)
  • List (65-71)
Maple2.Server.Game/Model/Field/Actor/FieldActor.cs (1)
Maple2.Server.Game/Manager/AnimationManager.cs (3)
  • AnimationManager (14-400)
  • AnimationManager (48-68)
  • AnimationManager (70-79)
Maple2.Server.Game/Packets/NpcControlPacket.cs (3)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.EmoteTask.cs (1)
  • MovementState (7-53)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementState.cs (2)
  • MovementState (11-310)
  • MovementState (37-48)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.TalkTask.cs (1)
  • MovementState (6-24)
Maple2.Model/Metadata/BeginCondition.cs (3)
Maple2.File.Ingest/MapperExtensions.cs (1)
  • BeginConditionTarget (362-455)
Maple2.File.Ingest/Utils/AttributeExtensions.cs (1)
  • BasicAttribute (6-44)
Maple2.Model/ModelExtensions.cs (1)
  • ActorState (97-246)
Maple2.Server.Game/Model/Field/Actor/Actor.cs (3)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/SkillState.cs (2)
  • SkillState (8-68)
  • SkillState (11-13)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementState.cs (1)
  • Update (184-229)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/BattleState.cs (1)
  • Update (57-89)
Maple2.Server.Game/Manager/AnimationManager.cs (8)
Maple2.Server.Game/Model/Field/Actor/Actor.cs (4)
  • IActor (187-194)
  • Actor (24-238)
  • Actor (56-67)
  • KeyframeEvent (215-215)
Maple2.Database/Storage/Metadata/NpcMetadataStorage.cs (1)
  • AnimationMetadata (73-89)
Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (3)
  • FieldPlayer (15-525)
  • FieldPlayer (90-103)
  • KeyframeEvent (506-508)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs (3)
  • FieldPlayer (62-100)
  • FieldNpc (102-122)
  • FieldNpc (124-126)
Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs (3)
  • FieldNpc (22-474)
  • FieldNpc (97-128)
  • KeyframeEvent (220-222)
Maple2.Server.Game/Session/GameSession.cs (3)
  • GameSession (37-805)
  • GameSession (106-116)
  • GameSession (698-698)
Maple2.Server.Game/Model/Field/Actor/IActor.cs (1)
  • KeyframeEvent (30-30)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationRecord.cs (1)
  • LoopData (39-42)
Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs (7)
Maple2.Server.Game/Manager/Field/Navigation.cs (3)
  • DtCrowdAgent (51-56)
  • Navigation (15-76)
  • Navigation (24-34)
Maple2.Model/Game/Npc/Npc.cs (2)
  • Npc (6-18)
  • Npc (14-17)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.EmoteTask.cs (1)
  • MovementState (7-53)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementState.cs (10)
  • MovementState (11-310)
  • MovementState (37-48)
  • NpcTask (50-60)
  • NpcTask (62-73)
  • NpcTask (75-85)
  • NpcTask (87-90)
  • NpcTask (92-95)
  • NpcTask (97-99)
  • NpcTask (114-119)
  • NpcTask (121-123)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.CleanupTask.cs (1)
  • MovementState (6-21)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.StandbyTask.cs (1)
  • MovementState (7-33)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.WalkTask.cs (1)
  • MovementState (7-182)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build
🔇 Additional comments (87)
Maple2.Model/Game/Npc/Npc.cs (1)

8-8: Type updated from AnimationSequence to AnimationSequenceMetadata

The type change from AnimationSequence to AnimationSequenceMetadata has been properly implemented in both the property declaration and constructor initialization. This change aligns with the PR objectives of renaming animation-related components for better clarity.

Also applies to: 16-16

Maple2.File.Ingest/Maple2.File.Ingest.csproj (1)

22-22: Package version update looks appropriate.

The Maple2.File.Parser.Tadeucci package has been updated from version 2.2.6 to 2.2.7. This update is likely required to support the animation-related changes in this PR, specifically the transition from AnimationSequence to AnimationSequenceMetadata.

Maple2.Model/Metadata/AnimationMetadata.cs (2)

5-5: Good refactoring of AnimationMetadata type.

The update to use AnimationSequenceMetadata instead of AnimationSequence for the Sequences property aligns with the PR objectives. This provides a more descriptive type name that better represents its purpose.

7-7: Improved type safety with non-nullable Keys collection.

The renamed AnimationSequenceMetadata record improves type safety by changing List<AnimationKey>? to List<AnimationKey>, making the Keys property non-nullable. This ensures that every animation sequence will have a defined list of keys, even if empty.

Maple2.File.Ingest/Mapper/AnimationMapper.cs (3)

18-18: Updated sequence type matches the metadata refactoring.

The type change from AnimationSequence to AnimationSequenceMetadata is consistent with the changes in the AnimationMetadata class, ensuring type compatibility throughout the codebase.

21-21: Constructor updated to match new type.

The constructor has been properly updated to use AnimationSequenceMetadata instead of AnimationSequence. The default value for the Time property is now explicitly set to 0 instead of using default, which improves code clarity.

Also applies to: 24-24

28-30: Improved duplicate handling with TryAdd.

The code now uses Dictionary.TryAdd() instead of checking ContainsKey() before adding, which is a more concise and efficient approach. This reduces the potential for race conditions in concurrent scenarios and makes the code more readable.

Maple2.Model/Enum/CompareType.cs (1)

11-14: New enum for stat comparison type

The CompareStatValueType enum provides a clean way to distinguish between percentage-based and absolute value comparisons. This enum complements the existing CompareType and will be useful for the new BeginConditions implementation mentioned in the PR objectives.

Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.Emote.cs (1)

19-19: Updated property name from AnimationState to Animation

This change aligns with the PR objectives to rename AnimationState to AnimationManager. The property access has been updated from actor.AnimationState to actor.Animation while maintaining the same method call and parameters.

Maple2.Server.Game/Model/Field/Actor/Routine/JumpRoutine.cs (2)

43-43: Updated property from IdleSequence to IdleSequenceMetadata

This change reflects the terminology update mentioned in the PR objectives, changing AnimationSequence to AnimationSequenceMetadata. The update ensures consistency across the codebase.

51-51: Updated property from IdleSequence to IdleSequenceMetadata

Same change as in the previous constructor, maintaining consistency in the property naming. The change properly accesses both the Id and Time properties from the renamed IdleSequenceMetadata.

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

34-34: Updated property from IdleSequence to IdleSequenceMetadata

This change in the OnCompleted method properly updates the reference from IdleSequence to IdleSequenceMetadata, maintaining consistency with the terminology changes throughout the codebase as mentioned in the PR objectives.

Maple2.Server.Game/Model/Field/Actor/Routine/MoveRoutine.cs (2)

13-13: Renamed property reference from IdleSequence to IdleSequenceMetadata

The property reference has been updated to align with the broader refactoring effort mentioned in the PR objectives, where AnimationSequence has been renamed to AnimationSequenceMetadata.

21-21: Renamed property reference from IdleSequence to IdleSequenceMetadata

Consistent with the change on line 13, this update ensures that the Run method also uses the renamed property, maintaining consistency throughout the codebase.

Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.EmoteTask.cs (2)

39-39: Updated property reference from AnimationState to Animation

The code now uses actor.Animation instead of actor.AnimationState, which aligns with the PR objective of renaming AnimationState to AnimationManager. This change ensures compatibility with the new animation management system.

49-49: Updated property reference from AnimationState to Animation

Consistent with the change on line 39, this update changes the reference from AnimationState to Animation when accessing the PlayingSequence property, ensuring all animation-related functionality uses the new property correctly.

Maple2.Server.Game/Session/GameSession.cs (2)

103-103: Added AnimationManager property to GameSession

The new Animation property of type AnimationManager has been added to the GameSession class. This property is properly marked with the null! initializer, indicating it will be set later before being used.

151-151: Initialized AnimationManager in EnterServer method

The Animation property is now properly initialized with a new instance of AnimationManager during the EnterServer method. This placement is appropriate as it keeps the animation initialization consistent with other manager initializations in this method.

Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs (1)

74-74: Added Animation.ResetActor call for player initialization

This new line calls ResetActor on the Animation property, similar to the existing calls for Stats and Buffs. This ensures that the player's animation state is properly reset when spawning, maintaining consistency with the actor references used by other components.

Maple2.Server.Game/Commands/DebugCommand.cs (1)

98-98: Property access updated to use Animation instead of AnimationState

The debug command has been correctly updated to use the new Animation property instead of AnimationState following the refactoring mentioned in the PR objectives.

Maple2.Server.Game/Manager/Config/BuffManager.cs (1)

50-55: Enhanced actor reset functionality

Good addition to also reset the actor reference in each individual buff when resetting the manager's actor. This ensures consistency between the manager and all buff references, preventing potential stale actor references.

public void ResetActor(IActor actor) {
    Actor = actor;
+   foreach ((int id, Buff buff) in Buffs) {
+       buff.ResetActor(actor);
+   }
}
Maple2.Server.Game/Model/Field/Actor/FieldActor.cs (2)

28-28: Updated property from AnimationState to AnimationManager

Property has been successfully renamed from AnimationState to Animation with the updated type of AnimationManager following the refactoring mentioned in the PR objectives.

37-37: Updated initialization to use AnimationManager

Constructor initialization has been correctly updated to instantiate the new AnimationManager class instead of AnimationState, maintaining the same functionality but with the refactored class structure.

Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.SkillCast.cs (2)

26-28: Updated property access to use Animation instead of AnimationState

The property access has been correctly updated to use the new Animation property instead of AnimationState following the refactoring.

107-107: Updated method call to use Animation instead of AnimationState

The method call has been correctly updated to use the new Animation property instead of AnimationState to access the GetSequenceSegmentTime method, maintaining the same functionality.

Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.SkillCastTask.cs (1)

100-100: Code looks good! Properly updated to use Animation instead of AnimationState.

The code correctly adopts the new Animation property of type AnimationManager instead of the old AnimationState. Also, the output parameter has been updated from AnimationSequence? to AnimationSequenceMetadata? to align with the method signature in the AnimationManager class.

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

18-18: Property replacement follows the correct implementation pattern.

The property AnimationState has been replaced with Animation of type AnimationManager, which aligns with the broader refactoring of animation handling in the project. The removal of the init accessor in favor of a simple getter suggests that this property is now initialized differently, likely in the constructor of implementing classes.

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

9-9: Parameter type correctly updated to AnimationSequenceMetadata.

The constructor parameter has been updated from AnimationSequence to AnimationSequenceMetadata, and all references to the parameter properties have been accordingly updated. This change is consistent with the overall refactoring of animation handling throughout the codebase.

Also applies to: 15-15

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

19-20: Property pattern improves flexibility while maintaining encapsulation.

Converting Caster and Owner from readonly fields to properties with private setters provides better encapsulation while allowing controlled internal modification. This change supports the new ResetActor functionality.

58-65: ResetActor method handles actor reference repairing correctly.

The new ResetActor method provides a way to update actor references when necessary (like when actors are respawned or recreated), while ensuring that the references are only updated when the ObjectId matches. This prevents incorrect actor assignments while supporting actor reference maintenance throughout the buff's lifecycle.

Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.Walk.cs (2)

25-25: Adopted animation metadata

Switching from AnimationSequence? to AnimationSequenceMetadata? is consistent with the overall animation refactoring across the codebase. Ensure that all references to walkSequence are handled as metadata elsewhere to maintain type consistency.

48-49: Confirm animation speed usage

Your usage of aniSpeed * Speed when calling TryPlaySequence is valid, though it may be worth verifying that the resulting speed remains consistent with the intended pacing of in-game animations.

Maple2.Server.Game/Packets/NpcControlPacket.cs (4)

49-49: Validate speed scaling

Multiplying npc.Animation.SequenceSpeed by 100 before writing may be correct for your packet format, but confirm that this scaling matches the intended speed representation elsewhere in the codebase.

55-55: Idle sequence usage

Using npc.Animation.IdleSequenceId is a clear way to determine the default sequence when no active animation is playing. This aligns well with the updated animation system.

62-62: Fallback to default sequence

The null-coalescing with defaultSequenceId ensures a proper sequence is written when no sequence is currently playing. This approach avoids errors with null references.

67-67: Concise jump sequence check

Using pattern matching on (npc.Animation.PlayingSequence?.Id ?? -1) for ANI_JUMP_A or ANI_JUMP_B is a clean and readable approach to identifying jump sequences.

Maple2.Server.Game/Util/SkillUtils.cs (9)

6-6: Import for dungeon checks

This new using Maple2.Server.Game.Manager.Field; import is required for referencing DungeonFieldManager in the new condition checks. No issues here.

85-87: Conditional survival logic

Returning early when OnlySurvival is true but the map is neither SurvivalTeam nor SurvivalSolo makes sense. This enforces correct usage restrictions per map type.

110-114: DurationWithoutMoving validation

Checking player.PositionTick.Duration is an effective way to ensure the caster remains stationary long enough. Confirm that PositionTick is updated accurately so this condition isn't bypassed unintentionally.

115-117: Map restriction

If the current map ID is not in the allowed list, returning false properly prevents undesired usage of the skill outside designated maps.

124-128: Dungeon group type check

This ensures the skill is only usable on maps managed by DungeonFieldManager with one of the specified DungeonGroupType values. This logic appears coherent with the new condition features.

129-131: Active skill filter logic

Verifying that none of the IDs match the current skill leads to returning false. Make sure this aligns with the intended design. If the condition requires at least one matching active skill, consider adjusting the logic from All(...) != id to a more direct containment check.

186-188: Disallowed buffs check

This early return when HasNotBuffIds matches any buff on the target is straightforward and logical to block usage if the actor carries a prohibited buff.

190-203: Player-specific sub-states

Your checks correctly differentiate conditions for player states, sub-states, and mastery requirements. Evaluate if partial coverage for other actor types might be needed unless you're certain only players will reach these checks.

204-207: NPC ID requirement

Verifying npc.Value.Id against condition.NpcIds is an effective approach to filter usage only for specific NPCs. This flows well with your updated condition structure.

Maple2.Server.Game/PacketHandlers/SkillHandler.cs (6)

161-161: Updated animation handling in HandleUse method

The TryPlaySequence method now includes the metadata parameter, which provides additional context about the skill being played. This is a good enhancement that allows the animation system to access skill-specific metadata for improved animation behavior.

338-340: Animation property name has been updated

This change is part of the refactoring from AnimationState to AnimationManager. The verification of the playing sequence is now performed on session.Player.Animation instead of AnimationState.

360-360: Animation handling updated in HandleSync method

The animation sequence is now played using session.Animation directly instead of session.Player.AnimationState, consistent with the overall refactoring approach. Note that unlike line 161, this call doesn't pass the metadata parameter.

378-378: Updated animation property reference

The playing sequence check has been updated to use session.Player.Animation.PlayingSequence instead of AnimationState, maintaining consistency with the refactoring pattern.

385-385: Updated animation loop setting

The animation loop sequence is now set using the new Animation property instead of AnimationState, consistent with the refactoring pattern.

403-403: Updated animation sequence cancellation

The animation sequence cancellation now uses session.Animation directly instead of session.Player.AnimationState. This is consistent with some of the previous changes that directly access session.Animation instead of going through session.Player.

Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (2)

90-90: Base constructor call updated

The base constructor call has been simplified to directly use session.NpcMetadata instead of determining the player model based on gender. This change aligns with the removal of the GetPlayerModel method and simplifies the initialization process.

92-92: Animation property initialization

The Animation property is now initialized from Session.Animation, aligning with the updated architecture that centralizes animation management. This ensures that the player's animation state is properly linked to the session's animation manager.

Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementState.cs (6)

17-17: Updated stateSequence type

The stateSequence field type has been changed from AnimationSequence? to AnimationSequenceMetadata?, which aligns with the broader refactoring to use metadata-based animation sequences.

40-40: Updated animation metadata access

The idle animation check now uses actor.Animation.RigMetadata instead of AnimationState, consistent with the refactoring pattern. This change maintains the same functionality while using the new animation management system.

144-147: Updated animation sequence handling in Idle method

The animation playback and sequence tracking now use the Animation property instead of AnimationState. These changes are consistent with the overall refactoring and maintain the same behavior as before.

185-185: Updated animation sequence check in Update method

The check for whether the current animation matches the state sequence now uses actor.Animation.PlayingSequence, maintaining the same functionality while using the new animation management system.

206-206: Updated animation sequence handling for spawn state

The animation sequence for the spawn state now uses actor.Animation.TryPlaySequence, consistent with the overall refactoring pattern.

209-209: Updated animation sequence tracking

The state sequence tracking is now updated from actor.Animation.PlayingSequence, maintaining consistency with the refactoring pattern.

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

43-43: Renamed AnimationState to Animation

The property has been renamed from AnimationState to Animation and its type changed to AnimationManager, as part of a broader refactoring of the animation system. This provides a more accurate name that better reflects its purpose as a manager rather than just a state.

51-54: Added PositionTick property for position tracking

A new property PositionTick has been added to track the actor's position over time, storing the position, last tick, and duration since the last position change. This enhancement will be useful for timing-based mechanics and optimizations based on actor movement.

56-56: Simplified Actor constructor

The constructor has been simplified by removing the modelName parameter, which is no longer needed due to the refactoring of the animation system. This change reduces complexity and dependency on specific model names.

63-63: Updated Animation initialization

The Animation property is now initialized with a new instance of AnimationManager instead of AnimationState, aligning with the broader refactoring of the animation system.

66-66: Initialized PositionTick property

The new PositionTick property is properly initialized with default values, ensuring the actor's position tracking starts in a consistent state.

205-211: Implemented position tracking logic

New logic has been added to update the PositionTick property based on whether the actor's position has changed. This allows tracking how long an actor has remained in the same position, which can be useful for various gameplay mechanics.

The Animation.Update method is now called to update the animation state, consistent with the refactoring pattern.

Maple2.Model/Metadata/BeginCondition.cs (3)

18-18: Logical addition acknowledged.
The introduction of OnlySurvival aligns with the new gameplay condition. No issues identified.

20-21: Battle mount conditions approved.
The AllowOnBattleMount and OnlyOnBattleMount fields are straightforward and coherent with the new mechanics.

39-46: Extended target conditions.
These added fields (Event, Stat, States, SubStates, Masteries, NpcIds, HasNotBuffIds) provide more fine-grained control over target requirements, aligning well with the new logic.

Maple2.File.Ingest/MapperExtensions.cs (7)

1-3: Validate syntax for target-typed new expressions.
List<DungeonGroupType> dungeonGroupType = []; may rely on newer C# features. Ensure the project’s language version supports this.

333-336: New arrays for maps, types, continents, and active skills.
This straightforward mapping from beginCondition to the corresponding arrays is correct.

340-341: Time conversion from seconds to milliseconds.
The approach using TimeSpan.FromSeconds ensures correctness. This is good, but confirm that large values will not overflow when cast to int.

344-344: Mapped OnlySurvival from allowMapleSurvival.
No issues noted with adding OnlySurvival.

346-347: Battle mount booleans.
AllowOnBattleMount and OnlyOnBattleMount successfully reflect the new condition.

361-361: Use of readonly default target.
DefaultBeginConditionTarget helps avoid null checks. This is a neat approach.

434-455: Parsing only the first non-default BasicAttribute.
Within ParseStat, once a non-default value is found, a break stops checking other attributes. Verify that only one attribute can be set per stat block. If multiple attributes can coexist, you may need to store them all.

Maple2.Server.Game/Manager/AnimationManager.cs (5)

49-68: Fallback handling for IdleSequenceId.
If no “Idle_A” sequence is found, the code sets IdleSequenceId to 0. Confirm that a zero sequence ID is appropriately handled at runtime and does not introduce errors.

97-105: TryPlaySequence with speed parameter.
This approach makes sense for quickly playing or queuing animations. The immediate reset on not finding a sequence is a safe fallback.

180-198: CancelSequence within keyframe event.
Queuing a reset if a keyframe event is in progress prevents re-entrancy issues. Solid approach.

304-345: Looping and keyframe replays.
The logic for sequence loops is well-structured, triggering keyframes again after the loop start. Ensure it’s thoroughly tested for edge cases where multiple keyframes occur near loop boundaries.

383-399: DebugPrint gating.
Restricting debug output to players only is a reasonable optimization. No issues found.

Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs (7)

75-78: Type update: Changed animation sequence types to use metadata

The properties have been correctly updated from AnimationSequence to AnimationSequenceMetadata as part of the broader refactoring mentioned in the PR objective. This change aligns with the renaming of AnimationSequence to AnimationSequenceMetadata throughout the codebase.

97-98: Constructor updated to use AnimationSequenceMetadata

The constructor assignment has been properly updated to use AnimationSequenceMetadata instead of AnimationSequence, maintaining consistency with the property type changes. The property name has also been updated from IdleSequence to IdleSequenceMetadata.

230-231: Type update: Updated TryGetValue parameter in NextRoutine method

The change correctly updates the type parameter in TryGetValue to AnimationSequenceMetadata?, maintaining type consistency with the rest of the refactoring.

246-248: Type update: Updated another TryGetValue call in NextRoutine method

The second TryGetValue call in the NextRoutine method has been correctly updated to use AnimationSequenceMetadata? as the out parameter type, consistent with the rest of the changes.

261-263: Type update: Updated TryGetValue in NextWaypoint method

The TryGetValue call in the NextWaypoint method has been properly updated to use AnimationSequenceMetadata? as the out parameter type.

269-275: Type update: Updated another TryGetValue call in NextWaypoint method

The second TryGetValue call in the NextWaypoint method has been correctly updated to use AnimationSequenceMetadata? as the out parameter type, maintaining consistency with the rest of the changes.

309-312: Type update: Updated TryGetValue in Animate method

The TryGetValue call in the Animate method has been properly updated to use AnimationSequenceMetadata? as the out parameter type, consistent with the rest of the changes.

Comment thread Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationRecord.cs
Comment thread Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationRecord.cs
Comment thread Maple2.Server.Game/Model/Skill/SkillQueue.cs Outdated
Comment thread ....Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.Walk.cs
Comment thread Maple2.Server.Game/Util/SkillUtils.cs Outdated
Comment thread Maple2.Server.Game/Util/SkillUtils.cs Outdated
Comment thread Maple2.Server.Game/Util/SkillUtils.cs
Comment thread Maple2.File.Ingest/MapperExtensions.cs
Comment thread Maple2.File.Ingest/MapperExtensions.cs Outdated
coderabbitai[bot]
coderabbitai Bot reviewed Apr 10, 2025
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (2)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationRecord.cs (1)

28-33: Validate constructor parameters.
This repeats the earlier suggestion to validate sequence and speed to avoid null references or invalid speeds. For example:

 public AnimationRecord(AnimationSequenceMetadata sequence, float speed, AnimationType type, SkillMetadata? skill = null) : this() {
+    if (sequence == null)
+        throw new ArgumentNullException(nameof(sequence), "Sequence must not be null");
+    if (speed <= 0)
+        throw new ArgumentOutOfRangeException(nameof(speed), "Speed must be positive");

     Sequence = sequence;
     Speed = speed;
     Type = type;
     Skill = skill;
 }
Maple2.Server.Game/Manager/AnimationManager.cs (1)

81-83: ⚠️ Potential issue

Refresh RigMetadata when resetting the actor.

You reassign Actor but do not update RigMetadata. If the new actor uses different metadata, reusing the old RigMetadata could lead to inconsistencies.

 public void ResetActor(IActor actor) {
     Actor = actor;
+    RigMetadata = actor switch {
+        FieldNpc fieldNpc => actor.Field.NpcMetadata.GetAnimation(fieldNpc.Value.Metadata.Model.Name),
+        _ => RigMetadata
+    };
 }
🧹 Nitpick comments (3)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationRecord.cs (2)

35-43: Consider using properties with PascalCase for struct fields.

In C#, it’s more common to define public fields as properties with PascalCase (e.g., public float Start { get; set; }). This helps maintain consistent naming conventions and can allow for future extensibility (e.g., validation). If these are purely data-holding structs and you’re fine with direct field access, you may keep them as is.

Also applies to: 45-60

6-61: Add unit tests for the new class.

Since AnimationRecord is core to animation handling, consider adding targeted tests to:

  1. Verify default constructor initialization (e.g., Speed = 1, IsLooping = false, etc.).
  2. Confirm parameterized constructor validations and property assignments (once added).
  3. Validate any future logic (like transitions, boundary checks, etc.).

If you’d like assistance creating these tests, let me know!

Maple2.Server.Game/Util/SkillUtils.cs (1)

166-185: Float equality check might need tolerance.

You compare floating values directly with CompareType.Equals => targetValue == value. Floating-point arithmetic can cause minor precision issues. Consider using a small epsilon to compare approximate equality if needed for gameplay integrity.

- CompareType.Equals => targetValue == value,
+ CompareType.Equals => Math.Abs(targetValue - value) < 0.0001f,
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 822880a and d945d0a.

📒 Files selected for processing (4)
  • Maple2.File.Ingest/MapperExtensions.cs (4 hunks)
  • Maple2.Server.Game/Manager/AnimationManager.cs (1 hunks)
  • Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationRecord.cs (1 hunks)
  • Maple2.Server.Game/Util/SkillUtils.cs (4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
Maple2.Server.Game/Util/SkillUtils.cs (3)
Maple2.Server.Game/Model/Stats.cs (7)
  • Stat (141-199)
  • Stat (149-149)
  • Stat (151-155)
  • Stats (8-139)
  • Stats (17-33)
  • Stats (35-41)
  • Total (106-116)
Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (2)
  • FieldPlayer (15-525)
  • FieldPlayer (90-103)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs (1)
  • FieldPlayer (62-100)
Maple2.Server.Game/Manager/AnimationManager.cs (7)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationRecord.cs (4)
  • AnimationRecord (6-61)
  • AnimationRecord (19-26)
  • AnimationRecord (28-33)
  • LoopData (39-42)
Maple2.Database/Storage/Metadata/NpcMetadataStorage.cs (1)
  • AnimationMetadata (73-89)
Maple2.Server.Game/Model/Field/Actor/FieldPlayer.cs (3)
  • FieldPlayer (15-525)
  • FieldPlayer (90-103)
  • KeyframeEvent (506-508)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs (3)
  • FieldPlayer (62-100)
  • FieldNpc (102-122)
  • FieldNpc (124-126)
Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs (3)
  • FieldNpc (22-474)
  • FieldNpc (97-128)
  • KeyframeEvent (220-222)
Maple2.Server.Game/Session/GameSession.cs (3)
  • GameSession (37-805)
  • GameSession (106-116)
  • GameSession (698-698)
Maple2.Server.Game/Model/Field/Actor/IActor.cs (1)
  • KeyframeEvent (30-30)
🔇 Additional comments (33)
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/AnimationRecord.cs (1)

7-17: Good job on restricting setters for key properties.

Making Sequence and Skill have private setters helps limit unexpected external modifications, addressing part of the prior feedback. You may consider restricting additional properties (e.g., Speed, Type) if you want to fully control state changes and enforce invariants more strictly.

Maple2.Server.Game/Manager/AnimationManager.cs (15)

14-33: Overall class structure looks solid.

This block sets up key fields and properties for animation management, clearly establishing the Actor reference, current/queued animations, metadata, and associated speed settings. It's a concise and well-organized foundation.

34-42: Conditional debug logging property is clear.

The DebugPrintAnimations setter only toggles the flag if the bound Actor is a FieldPlayer, preventing unneeded logs for non-player actors. This is a neat approach to minimize spam.

43-80: Constructor logic properly initializes metadata or defaults.

The constructor handles cases where RigMetadata may be null by assigning an IdleSequenceId of 0. Assigning the IdleSequenceId from the first available idle for NPCs or a default “Idle_A” for players is good. This ensures predictable fallback behavior.

85-95: Good defensive reset logic.

The private ResetSequence method terminates ongoing NPC controls (npc.SendControl = true) and cleans up the current animation state, preventing partial or corrupted animation states. This fosters consistency.

97-122: TryPlaySequence effectively guards against missing metadata.

By resetting the sequence and returning false if the specified animation is not found in RigMetadata, you avoid undefined behavior. Also, the queuing logic inside keyframe event handling is a neat way to defer sequence changes until the event is done.

124-152: Overloaded TryPlaySequence returning sequence metadata is intuitive.

Providing an out parameter for the animation metadata helps upstream functions retrieve the final sequence without duplicating the logic. The fallback to the queued sequence is a thoughtful inclusion.

154-178: Playing a new sequence resets old state thoroughly.

PlaySequence method ensures the previous sequence is reset before assigning a new record, which prevents stale state from leaking across transitions. The debug printing is also handy while diagnosing transitions.

180-197: Cancelling sequences mid-keyframe is well-handled.

By deferring a reset if isHandlingKeyframe is true, the code avoids unexpected disposal while halfway through an event. This block is well thought out and gracefully addresses corner cases.

199-275: Comprehensive Update method accurately tracks time and keyframe events.

You factor in both the inherent speed (SequenceSpeed) and type-based multipliers (MoveSpeed / AttackSpeed). Looping logic and keyframe re-processing within the loop windows are handled carefully. No obvious issues.

277-289: SetLoopSequence toggles looping states neatly.

This method centralizes the toggling of loop and loop-only-once flags, reducing potential duplication in other methods.

291-303: Keyframe detection logic is concise.

HasHitKeyframe method straightforwardly checks time boundaries. The condition ensures events only fire once per tick and that the key is within the looping window.

304-345: Segment time calculation method is flexible.

GetSequenceSegmentTime elegantly returns a normalized 0..1 range or -1 for out-of-segment, enabling easy usage in transitions. Checks for malformed segments are a nice safeguard.

347-381: HitKeyframe sets loop parameters and end times responsibly.

You handle the special loopstart/loopend/end markers cleanly, adjusting sequence times accordingly. This helps keep the animation system robust in edge cases.

383-391: Debug printing is well-scoped to players.

Restricting debug prints to player animations avoids spamming logs for other actor types. It’s crucial for maintaining performance in a large environment.

393-399: Solid utility method to check if an animation is active.

IsAnimationPlaying returning true only when Current and PlayingSequence are non-null covers all normal usage scenarios.

Maple2.Server.Game/Util/SkillUtils.cs (8)

6-6: Imports from Maple2.Server.Game.Manager.Field are consistent with your new references.

No concerns here, as referencing the field manager is logical for the newly added condition checks.

85-87: Condition check for Survival maps is correctly limited.

Using pattern matching (MapType.SurvivalTeam or MapType.SurvivalSolo) is straightforward and improves readability. The early return ensures minimal overhead when it fails.

115-123: Map-based checks neatly restrict usage.

Verifying Maps, MapTypes, and Continents ensures the skill can only trigger in the defined geographical or thematic contexts. This is standard practice for region-limited abilities.

124-131: Guarding by DungeonGroupType is well-structured.

The safe cast to DungeonFieldManager and verifying group types ensures relevancy only in dungeons that match the user’s specification.

187-189: HasNotBuffIds block is an elegant negation check.

If any of the specified buffs are present on the target, the condition fails. Straightforward implementation, and it covers typical skill condition logic.

193-203: Expanded checks for ActorState and SubState are well-defined.

Requiring the target’s state/substate to be in certain arrays is standard in conditional skill usage, e.g., disallowing usage while crouched. Implementation looks consistent.

205-209: NPC ID filtering is flexible.

Ensuring the target FieldNpc has an ID in NpcIds extends the system’s coverage of different skill behaviors based on NPC type. This is well-aligned with game design.

110-114:

Details

❓ Verification inconclusive

DurationWithoutMoving check aligns with time-based logic.

Ensuring the player has been stationary for the required duration is a typical approach. Just confirm that PositionTick.Duration is updated consistently on movement to avoid false positives.

Would you like to run a test scenario to confirm PositionTick.Duration increments properly?

🏁 Script executed:

#!/bin/bash
# Attempt to find references of PositionTick.Duration usage in the codebase
rg "PositionTick\.Duration" -A 5 -B 5

Length of output: 916

Action Required: Verify Consistent Update of PositionTick.Duration on Movement

The current logic correctly checks that player.PositionTick.Duration meets the required DurationWithoutMoving threshold. The grep output confirms that this check is implemented as expected. However, please ensure that PositionTick.Duration is updated consistently when the player moves to avoid any false positives.

  • File Affected: Maple2.Server.Game/Util/SkillUtils.cs (around lines 110–114)
  • Verification Suggestion: If not already covered by tests, consider adding a scenario that validates the proper incrementation of PositionTick.Duration upon movement.
Maple2.File.Ingest/MapperExtensions.cs (9)

1-3: Adding System.ComponentModel, System.Diagnostics, and System.Reflection references is consistent with new reflection usage.

These imports are required for your new attribute lookups and debugging logic.

327-330: Populating advanced map constraints.

Mapping from the parser’s requireMapCodes + others into your strongly-typed arrays is a solid approach. This enables the new survival and region-based checks in SkillUtils.

334-335: Converting seconds to milliseconds is correct for internal engine usage.

TimeSpan.FromSeconds(beginCondition.requireDurationWithoutMove).TotalMilliseconds ensures the engine remains consistent with other time-based checks.

338-338: Activating OnlySurvival matches new per-map logic.

A direct assignment of beginCondition.allowMapleSurvival ensures the data is fully utilized in comparison checks. No immediate concerns.

340-341: Adding mount-based conditions is straightforward.

AllowOnBattleMount and OnlyOnBattleMount are cleanly separated so that conditions can specify the presence or exclusivity of a battle mount.

342-345: Dungeon group types are parsed safely.

Your .Where(Enum.TryParse) ... .Select(Enum.Parse) chain ensures only valid DungeonGroupType entries are included. This gracefully handles unknown or invalid strings.

355-355: DefaultBeginConditionTarget helps skip trivial checks.

This sentinel-like target effectively avoids generating overhead conditions if everything is at default. It’s a clean approach to reduce clutter in simpler skill definitions.

363-389: Expanded logic for states, sub-states, masteries, and NPC IDs.

These lines parse complex arrays for advanced condition checks. The reflection approach for ActorState and ActorSubState by matching DescriptionAttribute is slightly more expensive but helps map external definitions elegantly.

395-395: Returning an empty array literal instead of Array.Empty is a neat inline approach.

These minimal array allocations are convenient. Just confirm performance is acceptable if done at scale. Typically not an issue for moderate usage.

@Zintixx
Zintixx merged commit f0e3888 into master Apr 10, 2025
@AngeloTadeucci
AngeloTadeucci deleted the buff-and-skill-beginconditions branch April 10, 2025 22:48
@Zintixx Zintixx mentioned this pull request Apr 15, 2025
Closed
@coderabbitai coderabbitai Bot mentioned this pull request Apr 15, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] requested changes
@AngeloTadeucci AngeloTadeucci AngeloTadeucci approved these changes

Assignees

No one assigned

Labels

Skill / Buff skill and buff related

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Striker Class

2 participants

@Zintixx @AngeloTadeucci