Skip to content

ParseConstant: discover itemoptionconstant_* files instead of a hardcoded suffix list - #77

Merged
AngeloTadeucci merged 3 commits into
MS2Community:masterfrom
Zintixx:flexible-itemoptionconstant-scan
Aug 2, 2026
Merged

ParseConstant: discover itemoptionconstant_* files instead of a hardcoded suffix list#77
AngeloTadeucci merged 3 commits into
MS2Community:masterfrom
Zintixx:flexible-itemoptionconstant-scan

Conversation

@Zintixx

@Zintixx Zintixx commented Aug 2, 2026

Copy link
Copy Markdown

Summary

ParseConstant iterated a hardcoded constantSuffix array to build filenames. This replaces it with a prefix scan over the archive's file list — the same idiom already used by ItemParser, AchieveParser, MapParser, AdditionalEffectParser, FunctionCubeParser, etc.

foreach (PackFileEntry entry in xmlReader.Files.Where(entry =>
        entry.Name.StartsWith("itemoption/constant/itemoptionconstant_"))) {
    string xml = Sanitizer.RemoveEmpty(xmlReader.GetString(entry));
    ...

Why

  • Flexible: auto-discovers every itemoptionconstant_* file (including custom merge files) with no suffix list to maintain.
  • Robust: skips absent files instead of throwing via GetEntry(...).First(...) when a listed suffix has no file.
  • Consistent: matches the dominant enumeration pattern across the other parsers.

Removes the now-dead constantSuffix array.

No behavioral regression: the ingest consumer keys constant options by (code, grade) and throws on true duplicates regardless of order, so the enumeration-order change (archive FileList order vs the fixed array) doesn't affect correctness.

Bumps PackageVersion to 2.4.20 (2.4.19 is already published).

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • Bug Fixes

    • Improved item option constant discovery by automatically recognizing all matching archive entries.
  • Compatibility

    • Added UTF-8 encoding metadata to map data files for more consistent text handling.
  • Chores

    • Updated the parser package version to 2.4.20.

Zintixx and others added 2 commits August 2, 2026 14:55
…oded suffix list

Enumerate all itemoption/constant/itemoptionconstant_* pack entries (matching
the pattern already used by ItemParser/AchieveParser/MapParser/etc.) rather
than iterating a fixed constantSuffix array. This auto-discovers any constant
file (including custom merge files) with no list to maintain, and skips missing
files instead of throwing on GetEntry.

Removes the now-dead constantSuffix array and bumps the package to 2.4.20.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

ParseConstant now discovers all matching item option constant files by filename prefix. The package version changes to 2.4.20, and MapXBlock.cs gains a UTF-8 byte-order mark.

Changes

Parser and package updates

Layer / File(s) Summary
Dynamic item option constant discovery
Maple2.File.Parser/ItemOptionParser.cs
ParseConstant filters archive entries by the itemoption/constant/itemoptionconstant_ prefix instead of iterating a fixed suffix list.
Package and source encoding updates
Maple2.File.Parser/Maple2.File.Parser.csproj, Maple2.File.Parser/Xml/Map/MapXBlock.cs
The package version changes from 2.4.18 to 2.4.20. MapXBlock.cs receives a UTF-8 byte-order mark.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: angelotadeucci

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change to discover itemoptionconstant_* files without a hardcoded suffix list.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch flexible-itemoptionconstant-scan
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
Maple2.File.Parser/ItemOptionParser.cs (1)

107-108: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add regression coverage for dynamic file discovery.

Maple2.File.Tests/ItemOptionParserTest.cs only checks that returned items are non-null. It does not verify discovery of a custom itemoption/constant/itemoptionconstant_* file or safe handling of missing legacy suffixes. Add fixtures for both cases and assert the expected constant records.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Maple2.File.Parser/ItemOptionParser.cs` around lines 107 - 108, Extend
ItemOptionParserTest to cover dynamic discovery of custom
itemoption/constant/itemoptionconstant_* files and handling of missing legacy
suffixes. Add fixtures for both scenarios, invoke the parser, and assert the
expected constant records rather than only checking that the returned items are
non-null.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@Maple2.File.Parser/ItemOptionParser.cs`:
- Around line 107-108: Extend ItemOptionParserTest to cover dynamic discovery of
custom itemoption/constant/itemoptionconstant_* files and handling of missing
legacy suffixes. Add fixtures for both scenarios, invoke the parser, and assert
the expected constant records rather than only checking that the returned items
are non-null.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 059fcc92-2453-4cd7-8fb3-6885929f9304

📥 Commits

Reviewing files that changed from the base of the PR and between 5303eb8 and f6a93da.

📒 Files selected for processing (3)
  • Maple2.File.Parser/ItemOptionParser.cs
  • Maple2.File.Parser/Maple2.File.Parser.csproj
  • Maple2.File.Parser/Xml/Map/MapXBlock.cs

@AngeloTadeucci
AngeloTadeucci merged commit d12f8a2 into MS2Community:master Aug 2, 2026
4 checks passed
@Zintixx
Zintixx deleted the flexible-itemoptionconstant-scan branch August 3, 2026 03:35
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