Skip to content

Parse titlename.xml as its own table - #86

Merged
AngeloTadeucci merged 1 commit into
masterfrom
feat/parse-title-name
Oct 7, 2026
Merged

AngeloTadeucci merged 1 commit into
masterfrom
feat/parse-title-name

Conversation

@AngeloTadeucci

@AngeloTadeucci AngeloTadeucci commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

The server has no list of the titles the client can name. The client builds the profile title dropdown from titlename.xml, after the locale and feature filter. It skips a title with no name but still counts it when it picks the preselected row, so a single such title makes the dropdown preselect the wrong row (PrivateMaple2#1573). PrivateMaple2 needs that list to stop granting and sending such titles. ParseTitleTag cannot provide it, because it only returns names for ids that have a titletag.xml row.

  • Maple2.File.Parser/TableParser.cs: adds ParseTitleName(), which reads {language}/titlename.xml through the existing StringMapping type, so keys come out already resolved by the locale and feature filter.
  • Maple2.File.Tests/TableParserTest.cs: TestParseTitleName checks three rows under Live NA: Ace Archer is kept, 10000754 with feature="Fame_Geo01" is dropped, and 10000750 with only locale="CN" is dropped.
  • Maple2.File.Parser/Maple2.File.Parser.csproj: 2.4.27 to 2.4.28.

Verification

  • I could not run TestParseTitleName, or any other test in Maple2.File.Tests, on my machine. TestUtils' static constructor builds an AssetIndex, and that throws on my data folder, a duplicate empty key in AssetIndex.ParseNtFile, before any test runs. TestParseTitleTag fails the same way, so it is not caused by this change.
  • I packed this branch as 2.4.28-debug.0 and ran the PrivateMaple2 ingest against it. It wrote 791 titles. A separate Python pass over the same XML, applying the locale and feature rules by hand, also gives 791, and the ids in the range 10000840 to 10000901 match one for one.

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

  • New Features
    • Added support for parsing title IDs and names from localized title data.

ParseTitleTag only returns names for ids that have a titletag.xml row, and
the client builds its profile title dropdown from titlename.xml alone. The
server needs every title the client can name, after the locale and feature
filter, to stop sending titles the dropdown cannot show.

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

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1a281475-1805-4ed0-adeb-cf876a3e4fbe
📥 Commits

Reviewing files that changed from the base of the PR and between f1e5fa5 and 88af437.

📒 Files selected for processing (3)
  • Maple2.File.Parser/Maple2.File.Parser.csproj
  • Maple2.File.Parser/TableParser.cs
  • Maple2.File.Tests/TableParserTest.cs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

TableParser adds ParseTitleName to read title-name entries for the configured language. A test checks a known entry and confirms two IDs are absent. The parser package version changes from 2.4.27 to 2.4.28.

Changes

Title-name parsing

Layer / File(s) Summary
Parse and validate title-name entries
Maple2.File.Parser/TableParser.cs, Maple2.File.Tests/TableParserTest.cs, Maple2.File.Parser/Maple2.File.Parser.csproj
ParseTitleName yields integer IDs and names from the configured language’s title-name XML. The test checks the “Ace Archer” entry and two absent IDs. The parser package version changes to 2.4.28.

Priority: ➖ Normal

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

Change: Feature

Suggested reviewers: zintixx

Merge Risk: ⚪ Minimal · up to 88af4

The title-name parser and version update are ready to merge after normal checks; the reported test-initialization failure prevents treating the new test as a successful run.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 88af4

The new API reuses existing archive-reading and filtering controls. No introduced security concern was confirmed, but downstream use and runtime verification remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is library-level: an invoking consumer can obtain title IDs and names from a selected entry in its supplied archive reader. The evidence does not establish attacker-facing invocation, tenant scope, service privileges, or downstream title-granting authority.

Trust Boundaries and Controls

  • observed — The new API does not open a filesystem path. It selects an indexed packed-file entry through the existing suffix-matching lookup and reads decrypted XML through M2dReader. Language influences entry selection, but this mechanism already exists in other parser methods.
  • observed — ParseTitleName uses the filtered key property rather than raw deserialized storage. Feature membership and matching or neutral locale remain enforced by the existing resolver; no filter bypass was found in this path.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: parsing titlename.xml as a separate table through the new ParseTitleName method.
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@AngeloTadeucci
AngeloTadeucci merged commit b073253 into master Oct 7, 2026
3 checks passed
@AngeloTadeucci
AngeloTadeucci deleted the feat/parse-title-name branch October 7, 2026 16:15
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.

1 participant