Skip to content

Fix: FishLure#61

Merged
AngeloTadeucci merged 1 commit intoMS2Community:masterfrom
Zintixx:fish
Apr 12, 2026
Merged

Fix: FishLure#61
AngeloTadeucci merged 1 commit intoMS2Community:masterfrom
Zintixx:fish

Conversation

@Zintixx
Copy link
Copy Markdown

@Zintixx Zintixx commented Apr 12, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Improved fishing lure data handling to capture and include level information.
  • Chores

    • Updated package version to 2.4.4.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 12, 2026

📝 Walkthrough

Walkthrough

This pull request updates the NuGet package version from 2.4.3 to 2.4.4 and extends the ParseFishLure() method to return an additional Level value in its result tuple, changing from a 2-tuple to a 3-tuple structure. Related code and tests are updated to reflect this change.

Changes

Cohort / File(s) Summary
Version Update
Maple2.File.Parser/Maple2.File.Parser.csproj
NuGet package version incremented from 2.4.3 to 2.4.4.
FishLure Parsing Enhancement
Maple2.File.Parser/ServerTableParser.cs, Maple2.File.Parser/Xml/Table/Server/FishLure.cs
ParseFishLure() now returns (int Code, int Level, FishLure Lure) instead of (int Code, FishLure Lure). Updated M2dFeatureLocale selector to include additionalEffectLevel alongside additionalEffectCode.
Test Updates
Maple2.File.Tests/ServerTableParserTest.cs
Updated TestFishLure method to handle the new 3-tuple deconstruction pattern from ParseFishLure().

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested reviewers

  • AngeloTadeucci

Poem

🐰 A level now joins the lure's sweet call,
Three values dance where two did fall,
From sea to test, the tuple's grown,
The parser's gift, a level shown! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

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.
Title check ❓ Inconclusive The title 'Fix: FishLure' is vague and does not clearly communicate what was fixed or what the primary change is. It lacks specificity about the actual modification—adding a Level value to the ParseFishLure() return tuple. Consider using a more descriptive title such as 'Add Level field to FishLure parser return value' or 'Update ParseFishLure() to include Level in tuple return type'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
🧪 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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
Maple2.File.Parser/Xml/Table/Server/FishLure.cs (1)

9-9: Rename private field to camelCase to match repo convention.

Line 9 uses _lure; for non-static private fields this should be camelCase per repo rules.

✏️ Suggested naming update
-    [M2dFeatureLocale(Selector = "additionalEffectCode|additionalEffectLevel")] private IList<FishLure> _lure;
+    [M2dFeatureLocale(Selector = "additionalEffectCode|additionalEffectLevel")] private IList<FishLure> lure;

As per coding guidelines: "Use camelCase for private fields in C#".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Maple2.File.Parser/Xml/Table/Server/FishLure.cs` at line 9, The private field
_lure should be renamed to camelCase (e.g., lure) to follow repo conventions;
update the declaration "[M2dFeatureLocale(Selector =
\"additionalEffectCode|additionalEffectLevel\")] private IList<FishLure> _lure;"
to use "lure" and update every reference to _lure within the class
(constructors, methods, property accessors, serializers, and any attribute
targets) so the code compiles and preserves the attribute and type
(IList<FishLure>).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Maple2.File.Parser/Maple2.File.Parser.csproj`:
- Line 16: This release includes a breaking API change (ParseFishLure changed
from a 2-tuple to a 3-tuple), so update the package version from 2.4.4 to a
breaking-version (e.g. 3.0.0) in the PackageVersion element and ensure any
packaging/metadata/changelog entries reflect the major version bump so consumers
get the correct semver major release for the changed ParseFishLure signature.

---

Nitpick comments:
In `@Maple2.File.Parser/Xml/Table/Server/FishLure.cs`:
- Line 9: The private field _lure should be renamed to camelCase (e.g., lure) to
follow repo conventions; update the declaration "[M2dFeatureLocale(Selector =
\"additionalEffectCode|additionalEffectLevel\")] private IList<FishLure> _lure;"
to use "lure" and update every reference to _lure within the class
(constructors, methods, property accessors, serializers, and any attribute
targets) so the code compiles and preserves the attribute and type
(IList<FishLure>).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 161bb396-651a-48a3-a8cb-472066c21fc4

📥 Commits

Reviewing files that changed from the base of the PR and between 9e5b75d and 52ee02d.

📒 Files selected for processing (4)
  • Maple2.File.Parser/Maple2.File.Parser.csproj
  • Maple2.File.Parser/ServerTableParser.cs
  • Maple2.File.Parser/Xml/Table/Server/FishLure.cs
  • Maple2.File.Tests/ServerTableParserTest.cs

@AngeloTadeucci AngeloTadeucci merged commit e068b3f into MS2Community:master Apr 12, 2026
3 checks passed
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