Quest Group

[Zintixx]

Summary by CodeRabbit

• New Features

  • Parser now supports quest group data, letting apps read and process quest group configurations and relationships.

• Chores

  • Package version bumped from 2.3.6 to 2.3.7.

• Tests

  • Added test coverage for quest group parsing.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be8007d and 24fdaab.

📒 Files selected for processing (1)

• Maple2.File.Tests/TableParserTest.cs

---

📝 Walkthrough

Walkthrough

Adds quest group XML DTOs and a ParseQuestGroup method to TableParser, wires a new XmlSerializer, adds a test that iterates the parsed results, and bumps the package version from 2.3.6 to 2.3.7.

Changes

Cohort / File(s)	Summary
Package Version Maple2.File.Parser/Maple2.File.Parser.csproj	Bumped PackageVersion from 2.3.6 to 2.3.7.
Quest Group Parsing Maple2.File.Parser/TableParser.cs, Maple2.File.Parser/Xml/Table/QuestGroupTable.cs	Added QuestGroupRoot and QuestGroup XML DTOs; added questGroupSerializer field and initialized it in the TableParser constructor; added public ParseQuestGroup() to deserialize table/questgrouptable.xml and yield (Id, QuestGroup) pairs.
Tests Maple2.File.Tests/TableParserTest.cs	Added TestQuestGroup() which iterates over _parser.ParseQuestGroup() (no assertions).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Parse Auto Action Table #53 — Similar pattern: adds XML DTOs, XmlSerializer field, and a Parse* method in TableParser.
• Parse Season Data #50 — Similar changes to TableParser introducing serializers and Parse* methods for additional table types.

Suggested reviewers

• AngeloTadeucci

Poem

🐰 I hopped through XML fields today,
Quest groups lined up in tidy array,
Ids and picks all read with care,
A tiny version bump to share,
Tests walked through — the parser’s on its way! 🎋

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ 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%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ 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 'Quest Group' directly aligns with the main changes: adding quest group parsing support, new QuestGroup/QuestGroupRoot types, and related tests.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (2)

Maple2.File.Tests/TableParserTest.cs (1)

736-742: Consider removing or using the results variable.

The results variable is assigned but never used. Most other tests in this file only iterate with foreach. Either remove the unused variable or add assertions to verify the parsed data.

♻️ Option 1: Remove unused variable (aligns with other tests)

     [TestMethod]
     public void TestQuestGroup() {
-        var results = _parser.ParseQuestGroup().ToList();
         foreach ((_, _) in _parser.ParseQuestGroup()) {
             continue;
         }
     }

♻️ Option 2: Add assertions to validate parsed data

     [TestMethod]
     public void TestQuestGroup() {
         var results = _parser.ParseQuestGroup().ToList();
+        Assert.IsTrue(results.Count > 0, "Expected at least one quest group");
         foreach ((_, _) in _parser.ParseQuestGroup()) {
             continue;
         }
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Maple2.File.Tests/TableParserTest.cs` around lines 736 - 742, The
TestQuestGroup method creates an unused local variable results from
_parser.ParseQuestGroup(); either remove that unused assignment or use it to
assert parsed values: locate the TestQuestGroup test and delete the line "var
results = _parser.ParseQuestGroup().ToList();" to match other tests that just
iterate, or replace it by using results (from
_parser.ParseQuestGroup()/ToList()) to perform assertions about expected items
(e.g. count or contents) before/after the existing foreach over
_parser.ParseQuestGroup(); ensure you reference the same
_parser.ParseQuestGroup() invocation and keep the foreach behavior consistent.

Maple2.File.Parser/Xml/Table/QuestGroupTable.cs (1)

12-16: Add nullable annotation or default value to dependency field.

The project has <Nullable>enable</Nullable> in the csproj. The string dependency field lacks a default value or nullable annotation, which may cause nullable warnings when the XML attribute is missing. Most similar string fields in the codebase use = string.Empty defaults.

♻️ Recommended approaches

-    [XmlAttribute] public string dependency;
+    [XmlAttribute] public string dependency = string.Empty;

Or:

-    [XmlAttribute] public string dependency;
+    [XmlAttribute] public string? dependency;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Maple2.File.Parser/Xml/Table/QuestGroupTable.cs` around lines 12 - 16, The
QuestGroup class's string field dependency lacks a nullable annotation or
default, causing nullable warnings under Nullable enabled; update the dependency
field in QuestGroup to either be nullable (string? dependency) or initialize it
with a default (dependency = string.Empty) and adjust any callers/deserializers
accordingly so XML deserialization still populates it; specifically modify the
dependency field declaration in class QuestGroup to use one of these two options
to silence nullable warnings and match the codebase conventions.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@Maple2.File.Parser/Xml/Table/QuestGroupTable.cs`:
- Around line 12-16: The QuestGroup class's string field dependency lacks a
nullable annotation or default, causing nullable warnings under Nullable
enabled; update the dependency field in QuestGroup to either be nullable
(string? dependency) or initialize it with a default (dependency = string.Empty)
and adjust any callers/deserializers accordingly so XML deserialization still
populates it; specifically modify the dependency field declaration in class
QuestGroup to use one of these two options to silence nullable warnings and
match the codebase conventions.

In `@Maple2.File.Tests/TableParserTest.cs`:
- Around line 736-742: The TestQuestGroup method creates an unused local
variable results from _parser.ParseQuestGroup(); either remove that unused
assignment or use it to assert parsed values: locate the TestQuestGroup test and
delete the line "var results = _parser.ParseQuestGroup().ToList();" to match
other tests that just iterate, or replace it by using results (from
_parser.ParseQuestGroup()/ToList()) to perform assertions about expected items
(e.g. count or contents) before/after the existing foreach over
_parser.ParseQuestGroup(); ensure you reference the same
_parser.ParseQuestGroup() invocation and keep the foreach behavior consistent.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 78983ee and be8007d.

📒 Files selected for processing (4)

• Maple2.File.Parser/Maple2.File.Parser.csproj
• Maple2.File.Parser/TableParser.cs
• Maple2.File.Parser/Xml/Table/QuestGroupTable.cs
• Maple2.File.Tests/TableParserTest.cs