Skip to content

fix(validation): reject invalid character names in handlers - #579

Merged
AngeloTadeucci merged 1 commit into
masterfrom
dev2
Oct 1, 2025
Merged

fix(validation): reject invalid character names in handlers#579
AngeloTadeucci merged 1 commit into
masterfrom
dev2

Conversation

@AngeloTadeucci

@AngeloTadeucci AngeloTadeucci commented Oct 1, 2025

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Bug Fixes
    • Corrected character name validation across character creation and name checks. Valid names are now accepted, and invalid names are consistently rejected with an appropriate error message. This resolves cases where some valid names were previously blocked and ensures clearer feedback when names violate naming rules, improving reliability during account setup and character creation.

@coderabbitai

coderabbitai Bot commented Oct 1, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Character name validation logic was inverted in two packet handlers. In the game server handler, the flow now treats invalid names as the path to proceed with error emission. In the login server handler, creation now continues on invalid names and errors on valid names, reversing prior behavior.

Changes

Cohort / File(s) Summary
Game server — name check handler
Maple2.Server.Game/PacketHandlers/CheckCharacterNameHandler.cs
Inverted condition in Handle: proceeds with invalid names to send s_char_err_ban_all; prior logic proceeded only on valid names.
Login server — character management
Maple2.Server.Login/PacketHandlers/CharacterManagementHandler.cs
Inverted validation in HandleCreate: sends error on valid names and allows invalid names to continue.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant GS as GameServer
  participant V as NameValidator

  rect rgba(230,240,255,0.5)
  note over C,GS: Previous flow (expected)
  C->>GS: Check/Create with name
  GS->>V: Validate(name)
  V-->>GS: isValid = true/false
  alt isValid == true
    GS-->>C: Proceed (OK)
  else isValid == false
    GS-->>C: Error (name invalid)
  end
  end

  rect rgba(255,235,230,0.5)
  note over C,GS: Current flow (after inversion)
  C->>GS: Check/Create with name
  GS->>V: Validate(name)
  V-->>GS: isValid = true/false
  alt isValid == true
    GS-->>C: Error (treated as invalid)
  else isValid == false
    GS-->>C: Proceed (treated as valid)
  end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I thump my paws, a puzzled bun,
Names flipped around—what have we done?
Valid is void, invalid’s in—
A topsy-turvy rabbit grin.
Let’s hop back through the flow, rewind,
And set the checks to match the mind.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “fix(validation): reject invalid character names in handlers” clearly and concisely captures the primary change of inverting the validation logic to reject invalid names in the handler methods without extraneous detail or ambiguity.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dev2

📜 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 95ce480 and f73c9f0.

📒 Files selected for processing (2)
  • Maple2.Server.Game/PacketHandlers/CheckCharacterNameHandler.cs (1 hunks)
  • Maple2.Server.Login/PacketHandlers/CharacterManagementHandler.cs (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
Maple2.Server.Game/PacketHandlers/CheckCharacterNameHandler.cs (1)
Maple2.Model/Validators/CharacterNameValidator.cs (2)
  • NameValidator (8-43)
  • ValidName (18-34)
Maple2.Server.Login/PacketHandlers/CharacterManagementHandler.cs (1)
Maple2.Model/Validators/CharacterNameValidator.cs (2)
  • NameValidator (8-43)
  • ValidName (18-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: build
🔇 Additional comments (2)
Maple2.Server.Login/PacketHandlers/CharacterManagementHandler.cs (1)

139-142: LGTM! Validation logic is now correct.

The negation correctly rejects invalid names (when ValidName returns false) and allows valid names to proceed with character creation.

Note: The AI summary incorrectly describes this change as inverting the logic to an incorrect state. This fix actually corrects the validation to properly reject invalid character names.

Maple2.Server.Game/PacketHandlers/CheckCharacterNameHandler.cs (1)

49-52: LGTM! Validation logic is now correct and consistent.

The negation correctly rejects invalid names (when ValidName returns false) and allows valid names to proceed. This matches the corrected logic in CharacterManagementHandler.cs.

Note: The AI summary incorrectly characterizes these changes as introducing incorrect behavior. Both handlers now properly validate character names using the same correct pattern.


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

❤️ Share

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

@AngeloTadeucci
AngeloTadeucci merged commit f8cf0c0 into master Oct 1, 2025
4 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Oct 12, 2025
@AngeloTadeucci
AngeloTadeucci deleted the dev2 branch October 13, 2025 00:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants