Skip to content

Fix: Remove Sanitize on Script Condition#47

Merged
AngeloTadeucci merged 1 commit intoMS2Community:masterfrom
Zintixx:script-condition
Apr 7, 2025
Merged

Fix: Remove Sanitize on Script Condition#47
AngeloTadeucci merged 1 commit intoMS2Community:masterfrom
Zintixx:script-condition

Conversation

@Zintixx
Copy link

@Zintixx Zintixx commented Apr 7, 2025

Summary by CodeRabbit

  • Chores

    • Updated the package to version 2.2.6.
  • Bug Fixes

    • Streamlined XML data processing by removing unnecessary steps.
    • Set explicit default values for key numerical settings to ensure more consistent and predictable behavior.

@coderabbitai
Copy link

coderabbitai bot commented Apr 7, 2025

Walkthrough

This update increments the package version in the project file and modifies the XML parsing logic. The sanitization call is removed from two parsing methods in the ServerTableParser class, while integer fields in the QuestScriptCondition class are now explicitly initialized to -1. These revisions refine initial values and streamline the XML data retrieval process without altering overall behavior.

Changes

File Change Summary
Maple2.File.Parser/…/Maple2.File.Parser.csproj Updated <PackageVersion> from 2.2.5 to 2.2.6 in the project file.
Maple2.File.Parser/ServerTableParser.cs Removed Sanitizer.RemoveEmpty() calls from ParseNpcScriptCondition and ParseQuestScriptCondition methods, directly obtaining XML strings from the xmlReader.
Maple2.File.Parser/…/QuestScriptCondition.cs Initialized integer fields (privilege, panelty, weddingState, weddingHallBooking, marriageDate) to -1 in the QuestScriptCondition class.

Sequence Diagram(s)

sequenceDiagram
    participant Parser as ServerTableParser
    participant XR as xmlReader
    participant DR as Deserializer

    Parser->>XR: GetEntry("table/Server/npcScriptCondition_Final.xml")
    XR-->>Parser: entry
    Parser->>XR: GetString(entry)
    XR-->>Parser: xmlData
    Parser->>DR: Deserialize(xmlData)
    DR-->>Parser: Parsed Condition Object
Loading

Poem

I'm a rabbit hopping through the code night,
Version bumped with a leap so light,
Removing excess fluff from XML text,
Default values now clearly fixed and next,
My whiskers twitch with a joyful delight,
Celebrating clean changes under moonlit byte!


📜 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 676f754 and f14a418.

📒 Files selected for processing (3)
  • Maple2.File.Parser/Maple2.File.Parser.csproj (1 hunks)
  • Maple2.File.Parser/ServerTableParser.cs (2 hunks)
  • Maple2.File.Parser/Xml/Table/Server/QuestScriptCondition.cs (1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
Maple2.File.Parser/ServerTableParser.cs (1)
Maple2.File.IO/M2dReader.cs (1)
  • GetString (64-67)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: format
  • GitHub Check: build
🔇 Additional comments (5)
Maple2.File.Parser/Maple2.File.Parser.csproj (1)

16-16: Version incremented appropriately.

Package version has been updated from 2.2.5 to 2.2.6, which aligns with the changes made to the XML parsing logic in this PR.

Maple2.File.Parser/ServerTableParser.cs (2)

114-114:

Details

✅ Verification successful

Removed sanitization from Quest script condition XML parsing.

The Sanitizer.RemoveEmpty() call has been removed, which is consistent with the change in ParseNpcScriptCondition(). This change allows empty elements in the XML to be preserved during parsing.


🏁 Script executed:

#!/bin/bash
# Check if there's a corresponding Sanitizer class that processes empty XML elements
rg -A 3 "class Sanitizer" --type cs

Length of output: 353


Sanitization Removal Verified

The removal of the Sanitizer.RemoveEmpty() call in ServerTableParser.cs is intentional and consistent with the changes in ParseNpcScriptCondition(). Our search confirms that the only available sanitizer method in Sanitizer.cs is SanitizeMagicPath, with no evidence of a RemoveEmpty() function. This verifies that empty XML elements are now preserved as expected. No further action is required.


103-103:

Details

❓ Verification inconclusive

Removed sanitization from NPC script condition XML parsing.

The Sanitizer.RemoveEmpty() call has been removed, allowing the raw XML string to be processed directly. This change preserves any empty elements that might be intentionally part of the XML structure.


🏁 Script executed:

#!/bin/bash
# Verify if there are any remaining usages of Sanitizer.RemoveEmpty with npcScriptCondition
rg -n "Sanitizer\.RemoveEmpty.*npcScriptCondition" --type cs

Length of output: 62


Verification: Removal of Sanitizer.RemoveEmpty for NPC Script Condition XML

The call to Sanitizer.RemoveEmpty() in the NPC script condition parsing has been removed as intended. A search for any remaining usages of this sanitization tied to npcScriptCondition produced no results. However, since the automated search output was minimal, please perform a manual verification to confirm that no unintended side effects arise from this change.

  • File: Maple2.File.Parser/ServerTableParser.cs (Line 103)
  • Change: The line now directly retrieves the XML string without sanitization:
            string xml = xmlReader.GetString(xmlReader.GetEntry("table/Server/npcScriptCondition_Final.xml"));
  • Impact: Removing the Sanitizer.RemoveEmpty() call ensures that any empty XML elements are preserved. Manual verification is advised to ensure that this behavior aligns with the intended XML structure processing.
Maple2.File.Parser/Xml/Table/Server/QuestScriptCondition.cs (2)

22-23: Initialized integer fields with default -1 value.

Integer fields privilege and panelty are now initialized to -1 instead of their default value (0). This change helps distinguish between unset values and values explicitly set to 0.


30-32:

Details

❓ Verification inconclusive

Initialized wedding-related fields with default -1 value.

Wedding-related integer fields (weddingState, weddingHallBooking, and marriageDate) are now initialized to -1. This change complements the removal of XML sanitization by ensuring these fields have a distinct "unset" value when not present in the XML.


🏁 Script executed:

#!/bin/bash
# Check if there are any other places in the codebase that rely on these fields being 0 by default
rg -A 3 "(weddingState|weddingHallBooking|marriageDate).*== 0" --type cs

Length of output: 74


Action: Verify the "unset" initialization for wedding-related fields

The changes initializing weddingState, weddingHallBooking, and marriageDate to -1 appear correct for marking these fields as "unset" since our search did not reveal any comparisons expecting a default of 0. However, please double-check manually to ensure that no other code assumes a 0 value for these fields.

  • File: Maple2.File.Parser/Xml/Table/Server/QuestScriptCondition.cs, Lines: 30-32
  • Context: Wedding-related fields are now explicitly initialized with -1 to reflect an "unset" state.
✨ 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
🪧 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.

@AngeloTadeucci AngeloTadeucci merged commit 3334711 into MS2Community:master Apr 7, 2025
3 checks passed
Zintixx added a commit to Zintixx/Maple2.File that referenced this pull request Apr 9, 2025
@Zintixx Zintixx deleted the script-condition branch April 9, 2025 23:31
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