fix(item-stats): improve random option selection and validation - #475
Conversation
Create a mutable list of available entries to avoid duplicates when selecting random item options. Remove locked preset entries from this list to ensure they are not reselected. Add a check to stop selection when no entries remain, logging an error for better diagnostics. Refactor offense stat validation by precomputing offense attribute flags and simulating the addition of a new stat to correctly enforce the offense line threshold for non-weapon, non-accessory, non-pet items.
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ItemStatsCalculator
Caller->>ItemStatsCalculator: RandomItemOption(entries, presets, ...)
ItemStatsCalculator->>ItemStatsCalculator: Initialize availableEntries from entries
loop For each preset
ItemStatsCalculator->>ItemStatsCalculator: Remove locked entries from availableEntries
end
loop While options needed and availableEntries not empty
ItemStatsCalculator->>ItemStatsCalculator: Select random entry from availableEntries
ItemStatsCalculator->>ItemStatsCalculator: Validate with IsValidStat
alt Valid
ItemStatsCalculator->>ItemStatsCalculator: Add to result, remove from availableEntries
else Invalid
ItemStatsCalculator->>ItemStatsCalculator: Remove from availableEntries
end
end
alt No availableEntries
ItemStatsCalculator->>ItemStatsCalculator: Log error with context
end
ItemStatsCalculator-->>Caller: Return selected options
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Maple2.Server.Game/Util/ItemStatsCalculator.cs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: format
🔇 Additional comments (4)
Maple2.Server.Game/Util/ItemStatsCalculator.cs (4)
520-521: Good solution to prevent infinite loops during selection.Creating a mutable copy of available entries is the correct approach to track which entries can still be selected, preventing the infinite loop scenario described in the PR objectives.
546-546: Excellent logging improvements for better diagnostics.The updates to use
Log.Logger.Errorand the addition of comprehensive error logging when no entries remain provide valuable diagnostic information. The error message includes all relevant context (ItemType, StatsType, Total) which will help with troubleshooting.Also applies to: 551-553
602-609: Excellent refactoring of offense stat validation logic.The precomputing of offense attribute flags and the simulation approach for checking the threshold are both improvements:
- The boolean flags make the logic more readable and reduce repeated attribute list lookups
- The pattern matching syntax for item type checking is cleaner
- Simulating the addition of the new stat by incrementing the count before the threshold check correctly models the expected behavior
This properly enforces the offense line threshold as described in the PR objectives.
529-529: 🛠️ Refactor suggestionAdd null check before removing entry from availableEntries.
Consider adding a null check to prevent issues if
FirstOrDefaultreturns a default entry:ItemOption.Entry entry = option.Entries.FirstOrDefault(e => e.BasicAttribute == basic); // Ignore any invalid presets, they will get populated with valid data below. AddResult(entry, statResult, specialResult); -availableEntries.Remove(entry); +if (entry.BasicAttribute != null || entry.SpecialAttribute != null) { + availableEntries.Remove(entry); +}Likely an incorrect or invalid review comment.
Create a mutable list of available entries to avoid duplicates when
selecting random item options. Remove locked preset entries from this
list to ensure they are not reselected. Add a check to stop selection
when no entries remain, logging an error for better diagnostics.
Refactor offense stat validation by precomputing offense attribute
flags and simulating the addition of a new stat to correctly enforce
the offense line threshold for non-weapon, non-accessory, non-pet items.
Summary by CodeRabbit
Bug Fixes
Refactor