Fix Npc being stuck after using skill

[AngeloTadeucci]

Npc used skill and immediately tried to go back to spawn before skill keyframe ended

Summary by CodeRabbit

• New Features

  • Idle behavior for NPCs is now refined to prevent idle movement during active skill casting and to recognize "sit" animations as idle, improving animation handling.

• Refactor

  • Modernized conditional checks using property pattern matching for better code clarity.
  • Changed internal class and field visibility to public properties for improved access control.

• Style

  • Simplified lambda expressions and variable usage for cleaner code.

• Chores

  • Removed unused namespace imports.
  • Adjusted logging levels for cinematic UI operations.
  • Excluded specific packet types from verbose logging.

[coderabbitai]

Walkthrough

This update introduces code modernizations, logging adjustments, and refinements to NPC idle behavior. It replaces field checks with property pattern matching, exposes a task property, broadens idle animation detection, and prevents idle movement during active casting. Several unused imports are removed, and logging verbosity is reduced for specific opcodes and cinematic UI actions.

Changes

File(s)	Change Summary
Maple2.Server.Core/Network/Session.cs	Excludes SendOp.Vibrate packets from verbose logging in LogSend.
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.cs Maple2.Server.Game/Service/ChannelService.Heartbeat.cs	Removes unused namespace imports.
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/BattleState.cs	Refactors type and property checks to use C# property pattern matching for FieldPet conditions.
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.SkillCast.cs	Converts private castTask field to a public CastTask property with a private setter; updates all references to use the property.
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.SkillCastTask.cs	Changes NpcSkillCastTask class from internal to public; updates references to use CastTask property.
Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/TaskState.cs	Simplifies lambda expressions and discards unused out variable in QueueTask.
Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs	Adds CastTask null check before starting idle movement; expands idle animation detection to include "sit" sequences.
Maple2.Server.Game/Trigger/TriggerContext.Cinematic.cs	Changes log level for SetCinematicUi from warning to debug.

Sequence Diagram(s)

sequenceDiagram
    participant FieldNpc
    participant MovementState

    FieldNpc->>MovementState: Check CastTask
    alt CastTask is null
        FieldNpc->>MovementState: Start idle move to patrol/origin
    else CastTask active
        FieldNpc-->>FieldNpc: Skip idle movement
    end

    FieldNpc->>FieldNpc: Animate(sequence)
    alt sequence contains "idle" or "sit"
        FieldNpc->>FieldNpc: Set idle task
    end

Loading

Possibly related PRs

• AngeloTadeucci/Maple2#382: Modifies FieldNpc.Animate to detect idle animations by "idle" sequences and adds idle state handling in MovementState; both PRs adjust idle animation logic and idle state behavior.

Suggested reviewers

• Zintixx

Poem

In the code fields where NPCs sit or idle,
A rabbit hops, refactoring with a smile.
Patterns match, tasks exposed,
Unused usings now deposed.
With debug logs and idle checks anew,
The server hops on, steady and true!
🐇✨

---

📜 Recent review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 96661ba and e3fb581.

📒 Files selected for processing (9)

• Maple2.Server.Core/Network/Session.cs (1 hunks)
• Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.cs (0 hunks)
• Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/BattleState.cs (1 hunks)
• Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateStates/MovementState.SkillCast.cs (3 hunks)
• Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/MovementStateTasks/MovementState.SkillCastTask.cs (4 hunks)
• Maple2.Server.Game/Model/Field/Actor/ActorStateComponent/TaskState.cs (2 hunks)
• Maple2.Server.Game/Model/Field/Actor/FieldNpc.cs (2 hunks)
• Maple2.Server.Game/Service/ChannelService.Heartbeat.cs (0 hunks)
• Maple2.Server.Game/Trigger/TriggerContext.Cinematic.cs (1 hunks)

💤 Files with no reviewable changes (2)

• Maple2.Server.Game/Service/ChannelService.Heartbeat.cs
• Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.cs

🧰 Additional context used

🧬 Code Graph Analysis (1)

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

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

• FieldPet (13-115)
• FieldPet (27-47)

Maple2.Server.Game/Packets/PetPacket.cs (1)

• PetPacket (11-245)

🔇 Additional comments (12)

Maple2.Server.Core/Network/Session.cs (1)

284-284: Good logging optimization.

Adding SendOp.Vibrate to the excluded opcodes list is consistent with the pattern of reducing verbose logging for frequently sent packets.

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

41-41: Appropriate log level reduction.

Changing from WarnLog to DebugLog for cinematic UI operations is appropriate since these are normal operational events that don't require warning-level visibility.

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

72-72: Good modernization with property pattern matching.

The refactor to use C# property pattern matching syntax is cleaner and more modern while maintaining the same logic.

---

78-78: Excellent use of multiple property pattern matching.

The modernization combines multiple property checks (OwnerId: 0, TamingPoint: <= 0) in a single, readable pattern match, improving code clarity.

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

16-16: LGTM! Good code modernization.

The lambda expression is simplified by removing unnecessary explicit type annotations, improving readability while maintaining the same functionality.

---

31-31: LGTM! Proper use of discard pattern.

Using _ to discard the unused priority parameter is a clean approach that eliminates unnecessary variable assignment.

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

10-10: LGTM! Good encapsulation with property pattern.

Converting the private field to a public property with private setter allows external components to check cast task state while maintaining controlled access. This change directly supports the PR objective of preventing idle movement during active casting.

---

19-19: LGTM! Consistent property usage.

All references have been properly updated to use the new CastTask property, maintaining consistency throughout the codebase.

Also applies to: 58-58, 63-63

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

11-11: LGTM! Appropriate visibility change.

Making NpcSkillCastTask public enables external type checking, which is necessary for the cast task state management improvements.

---

36-36: LGTM! Consistent property usage.

All references have been properly updated to use the new CastTask property, maintaining consistency with the encapsulation changes.

Also applies to: 80-82, 112-112

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

202-202: Excellent fix for the core issue!

Adding the check for MovementState.CastTask is null directly addresses the PR objective by preventing idle movement back to spawn while a skill cast is active. This ensures NPCs don't get stuck by moving before the skill keyframe animation completes.

---

333-333: LGTM! Improved idle animation detection.

Expanding the idle animation detection to include "sit" sequences (case-insensitive) is a sensible enhancement that broadens the classification of idle behaviors.

✨ Finishing Touches

• 📝 Generate Docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Explain this complex logic.
  • 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 explain this code block.
  • @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 explain its main purpose.
  • @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.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @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 generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

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

CodeRabbit Configuration File (.coderabbit.yaml)

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

Documentation and Community

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