ParseConstant: discover itemoptionconstant_* files instead of a hardcoded suffix list - #77
Conversation
…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>
📝 WalkthroughWalkthrough
ChangesParser and package updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Maple2.File.Parser/ItemOptionParser.cs (1)
107-108: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression coverage for dynamic file discovery.
Maple2.File.Tests/ItemOptionParserTest.csonly checks that returned items are non-null. It does not verify discovery of a customitemoption/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
📒 Files selected for processing (3)
Maple2.File.Parser/ItemOptionParser.csMaple2.File.Parser/Maple2.File.Parser.csprojMaple2.File.Parser/Xml/Map/MapXBlock.cs
Summary
ParseConstantiterated a hardcodedconstantSuffixarray to build filenames. This replaces it with a prefix scan over the archive's file list — the same idiom already used byItemParser,AchieveParser,MapParser,AdditionalEffectParser,FunctionCubeParser, etc.Why
itemoptionconstant_*file (including custom merge files) with no suffix list to maintain.GetEntry(...).First(...)when a listed suffix has no file.Removes the now-dead
constantSuffixarray.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 (archiveFileListorder vs the fixed array) doesn't affect correctness.Bumps
PackageVersionto 2.4.20 (2.4.19 is already published).🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit
Bug Fixes
Compatibility
Chores