Add mastery commands for player management and debugging - #562
Conversation
To better debug and test different mastery related features a new subcommand has been added to the player commands. This new subcommand allows for changing mastery exp/levels of the current player. **Usage:** `/player mastery <"exp"/"level"> <mastery> <exp/level>` **Masteries**: - `Fishing` - `Music` - `Mining` - `Gathering` - `Breeding` - `Farming` - `Blacksmithing` - `Engraving` - `Alchemist` - `Cooking` - `PetTaming` **Examples:** ``` # Sets the Fishing Mastery to a total of 20 exp /player mastery exp Fishing 20 # Sets the Fishing Mastery to level 2 /player mastery level Fishing 2 ```
WalkthroughAdds a new Mastery command group under PlayerCommand with two subcommands to set mastery experience directly or via level-derived experience. Commands update session mastery values; the level variant computes experience from TableMetadata’s MasteryRewardTable thresholds. Errors write exception messages and return non-zero exit codes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin as Admin/User
participant PC as PlayerCommand
participant MC as MasteryCommand
participant Sess as Session
participant Meta as TableMetadata
Admin->>PC: mastery exp <type> <exp>
PC->>MC: Route to MasteryExpCommand
MC->>Sess: Set Mastery[<type>] = <exp>
MC-->>Admin: ExitCode 0 or error (message on console)
Admin->>PC: mastery level <type> <level>
PC->>MC: Route to MasteryLevelCommand
MC->>Meta: Get MasteryRewardTable.Entries
MC->>MC: Find highest threshold <= level
MC->>Sess: Set Mastery[<type>] = derivedExp
MC-->>Admin: ExitCode 0 or error (message on console)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
Maple2.Server.Game/Commands/PlayerCommand.cs (2)
37-46: Argument description says “to add” but code sets an absolute value.The handler assigns the mastery experience directly, not incrementally. Update the help text to prevent confusion.
Apply this diff to clarify the argument description:
- var exp = new Argument<int>("exp", "Experience points to add."); + var exp = new Argument<int>("exp", "Absolute mastery experience (total).");
48-56: Validate non-negative exp before assignment.Setting negative mastery exp can corrupt state or trigger underflow in downstream logic. Reject negative values early.
Apply this diff inside the handler:
private void Handle(InvocationContext ctx, MasteryType masteryType, int exp) { try { + if (exp < 0) { + ctx.Console.Error.WriteLine("Exp must be >= 0."); + ctx.ExitCode = 1; + return; + } session.Mastery[masteryType] = exp; ctx.ExitCode = 0; } catch (SystemException ex) { ctx.Console.Error.WriteLine(ex.Message); ctx.ExitCode = 1; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
Maple2.Server.Game/Commands/PlayerCommand.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Maple2.Server.Game/Commands/PlayerCommand.cs (3)
Maple2.Server.Game/Session/GameSession.cs (3)
GameSession(36-852)GameSession(114-124)GameSession(742-742)Maple2.Model/Game/User/Mastery.cs (1)
Mastery(7-35)Maple2.File.Ingest/Mapper/TableMapper.cs (1)
MasteryRewardTable(764-778)
🔇 Additional comments (3)
Maple2.Server.Game/Commands/PlayerCommand.cs (3)
25-26: Mastery command registration is well-integrated.Hooking MasteryCommand into PlayerCommand alongside the existing subcommands is consistent with the file’s structure and permissioning (AdminPermissions.PlayerCommands).
28-33: Good encapsulation via nested command group.Keeping MasteryExpCommand and MasteryLevelCommand under a dedicated MasteryCommand mirrors the structure of the other player subcommands and keeps the surface area minimal.
48-51: Awaiting details from MasteryManager setter block for full context.
| var masteryCode = new Argument<MasteryType>("mastery", "MasteryType of the player."); | ||
| var exp = new Argument<int>("exp", "Experience points to add."); | ||
|
|
||
| AddArgument(masteryCode); | ||
| AddArgument(exp); | ||
| this.SetHandler<InvocationContext, MasteryType, int>(Handle, masteryCode, exp); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify MasteryType names vs PR examples; consider documenting accepted values or mapping synonyms.
Your PR text lists “Music, Gathering, Breeding, Engraving” but Model/Mastery properties use “Instrument, Foraging, Ranching, Handicrafts.” If the enum uses the latter names, System.CommandLine will reject the former. Either:
- Update PR examples to match the actual enum names; and/or
- Accept common synonyms by parsing a string argument and mapping to MasteryType.
Run this to confirm the enum members used at runtime:
If synonyms are needed and you want a lightweight change, I can propose a small mapper without touching other callsites. Want me to draft it?
Also applies to: 65-71
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the MasteryType enum declaration and members
rg -nPU '(?s)\benum\s+MasteryType\s*{.*?}' -C2 --glob '!**/bin/**' --glob '!**/obj/**' | sed -n '/enum MasteryType/,/}/p'Length of output: 143
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "🔍 Searching for any references to MasteryType in C# files..."
rg -n --glob '*.cs' '\bMasteryType\b' -C2 --glob '!**/bin/**' --glob '!**/obj/**' || true
echo
echo "🔍 Looking for a file named MasteryType.cs..."
fd --type f MasteryType.cs || true
# If it exists at root or in a discovered path, dump its declaration
if [ -f MasteryType.cs ]; then
echo
echo "📄 Contents of MasteryType.cs (first 200 lines):"
sed -n '1,200p' MasteryType.cs
fiLength of output: 21794
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Dump the contents of the MasteryType enum file for inspection
echo "📄 Showing the MasteryType enum declaration:"
sed -n '1,200p' Maple2.Model/Enum/MasteryType.csLength of output: 451
Document or Align MasteryType Inputs
Please ensure that the command-line examples and handlers use the exact enum identifiers defined in Maple2.Model.Enum.MasteryType (Unknown, Fishing, Music, Mining, Gathering, Breeding, Farming, Blacksmithing, Engraving, Alchemist, Cooking, PetTaming). Currently, System.CommandLine will only accept those names (e.g. “Music”) and numeric values, not the game-facing synonyms (“Instrument”, “Foraging”, “Ranching”, “Handicrafts”).
• Update your PR’s documentation/examples in
- Maple2.Server.Game/Commands/PlayerCommand.cs (constructor overloads at lines 40–46 and 65–71)
to list the actual enum values, for instance:
- // Example: mastery Music exp 100
+ // Example: mastery Music exp 100• If you’d like to support synonyms, replace the raw Argument<MasteryType> with Argument<string> (or keep both) and map input strings to the correct enum before handling. For example:
- var masteryCode = new Argument<MasteryType>("mastery", "MasteryType of the player.");
+ var masteryInput = new Argument<string>("mastery", "MasteryType (e.g. Music, Instrument).");
var exp = new Argument<int>("exp", "Experience points to add.");
AddArgument(masteryInput);
AddArgument(exp);
- this.SetHandler<InvocationContext, MasteryType, int>(Handle, masteryCode, exp);
+ this.SetHandler<InvocationContext, string, int>((ctx, input, e) =>
+ {
+ var mastery = ParseMastery(input);
+ Handle(ctx, mastery, e);
+ }, masteryInput, exp);
+ static MasteryType ParseMastery(string s) =>
+ s.ToLowerInvariant() switch
+ {
+ "music" or "instrument" => MasteryType.Music,
+ "gathering" or "foraging" => MasteryType.Gathering,
+ "breeding" or "ranching" => MasteryType.Breeding,
+ "engraving" or "handicrafts" => MasteryType.Engraving,
+ _ when Enum.TryParse<MasteryType>(s, true, out var m) => m,
+ _ => throw new ArgumentException($"Unknown mastery '{s}'", nameof(s))
+ };Let me know if you’d like a fuller PR to add this mapper.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Maple2.Server.Game/Commands/PlayerCommand.cs around lines 40–46 (and also
apply to the constructor overloads at lines 65–71), the command currently
declares Argument<MasteryType> which leads to confusion because
System.CommandLine only accepts the enum identifiers (Unknown, Fishing, Music,
Mining, Gathering, Breeding, Farming, Blacksmithing, Engraving, Alchemist,
Cooking, PetTaming) and numeric values; update the argument help text/examples
to list those exact enum names, or change the signature to accept
Argument<string> (or add a second string argument) and perform a mapping from
input synonyms (e.g., "Instrument" -> "Music", "Foraging" -> "Gathering",
"Ranching" -> "Breeding", "Handicrafts" -> "Blacksmithing") to the MasteryType
enum before invoking Handle; finally ensure invalid inputs produce a clear error
and update the constructor documentation/examples accordingly.
| private void Handle(InvocationContext ctx, MasteryType masteryType, int level) { | ||
| try { | ||
| int exp = 0; | ||
| if (session.TableMetadata.MasteryRewardTable.Entries.TryGetValue(masteryType, out IReadOnlyDictionary<int, MasteryRewardTable.Entry>? masteryRewardMetadata)) { | ||
| exp = masteryRewardMetadata.OrderByDescending(mastery => mastery.Key).FirstOrDefault(mastery => level >= mastery.Key).Value.Value; | ||
| } | ||
| session.Mastery[masteryType] = exp; | ||
| ctx.ExitCode = 0; |
There was a problem hiding this comment.
Avoid NullReference when no threshold ≤ level; handle missing metadata and invalid levels.
If there’s no mastery grade ≤ requested level, FirstOrDefault returns a default KVP and .Value.Value will throw. Also, missing mastery metadata currently results in silently setting exp=0. Handle both explicitly and fail fast.
Apply this diff to make the computation robust and user-friendly:
private void Handle(InvocationContext ctx, MasteryType masteryType, int level) {
try {
- int exp = 0;
- if (session.TableMetadata.MasteryRewardTable.Entries.TryGetValue(masteryType, out IReadOnlyDictionary<int, MasteryRewardTable.Entry>? masteryRewardMetadata)) {
- exp = masteryRewardMetadata.OrderByDescending(mastery => mastery.Key).FirstOrDefault(mastery => level >= mastery.Key).Value.Value;
- }
- session.Mastery[masteryType] = exp;
- ctx.ExitCode = 0;
+ if (!session.TableMetadata.MasteryRewardTable.Entries.TryGetValue(masteryType, out IReadOnlyDictionary<int, MasteryRewardTable.Entry>? masteryRewardMetadata)) {
+ ctx.Console.Error.WriteLine($"Mastery metadata not found for {masteryType}.");
+ ctx.ExitCode = 1;
+ return;
+ }
+
+ if (level < 1) {
+ ctx.Console.Error.WriteLine($"Invalid mastery level: {level}. Must be >= 1.");
+ ctx.ExitCode = 1;
+ return;
+ }
+
+ // Highest grade threshold <= requested level
+ int applicableGrade = masteryRewardMetadata.Keys.Where(k => k <= level).DefaultIfEmpty(-1).Max();
+ if (applicableGrade < 0 || !masteryRewardMetadata.TryGetValue(applicableGrade, out var entry)) {
+ ctx.Console.Error.WriteLine($"No mastery grade threshold found for level {level} ({masteryType}).");
+ ctx.ExitCode = 1;
+ return;
+ }
+
+ session.Mastery[masteryType] = entry.Value;
+ ctx.ExitCode = 0;
} catch (SystemException ex) {
ctx.Console.Error.WriteLine(ex.Message);
ctx.ExitCode = 1;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private void Handle(InvocationContext ctx, MasteryType masteryType, int level) { | |
| try { | |
| int exp = 0; | |
| if (session.TableMetadata.MasteryRewardTable.Entries.TryGetValue(masteryType, out IReadOnlyDictionary<int, MasteryRewardTable.Entry>? masteryRewardMetadata)) { | |
| exp = masteryRewardMetadata.OrderByDescending(mastery => mastery.Key).FirstOrDefault(mastery => level >= mastery.Key).Value.Value; | |
| } | |
| session.Mastery[masteryType] = exp; | |
| ctx.ExitCode = 0; | |
| private void Handle(InvocationContext ctx, MasteryType masteryType, int level) { | |
| try { | |
| if (!session.TableMetadata.MasteryRewardTable.Entries.TryGetValue(masteryType, out IReadOnlyDictionary<int, MasteryRewardTable.Entry>? masteryRewardMetadata)) { | |
| ctx.Console.Error.WriteLine($"Mastery metadata not found for {masteryType}."); | |
| ctx.ExitCode = 1; | |
| return; | |
| } | |
| if (level < 1) { | |
| ctx.Console.Error.WriteLine($"Invalid mastery level: {level}. Must be >= 1."); | |
| ctx.ExitCode = 1; | |
| return; | |
| } | |
| // Highest grade threshold <= requested level | |
| int applicableGrade = masteryRewardMetadata.Keys | |
| .Where(k => k <= level) | |
| .DefaultIfEmpty(-1) | |
| .Max(); | |
| if (applicableGrade < 0 | |
| || !masteryRewardMetadata.TryGetValue(applicableGrade, out var entry)) { | |
| ctx.Console.Error.WriteLine($"No mastery grade threshold found for level {level} ({masteryType})."); | |
| ctx.ExitCode = 1; | |
| return; | |
| } | |
| session.Mastery[masteryType] = entry.Value; | |
| ctx.ExitCode = 0; |
Overview
To better debug and test different mastery related features a new subcommand has been added to the player commands. This new subcommand allows for changing mastery exp/levels of the current player.
Usage:
/player mastery <"exp"/"level"> <mastery> <exp/level>Masteries:
FishingMusicMiningGatheringBreedingFarmingBlacksmithingEngravingAlchemistCookingPetTamingExamples:
Not sure if this is needed or others find this helpful. If not feel free to close this Pull Request :)
Summary by CodeRabbit