Skip to content

Fix: Add enchant stats to character - #453

Merged
AngeloTadeucci merged 4 commits into
MS2Community:masterfrom
jf52637:JF
May 26, 2025
Merged

Fix: Add enchant stats to character#453
AngeloTadeucci merged 4 commits into
MS2Community:masterfrom
jf52637:JF

Conversation

@jf52637

@jf52637 jf52637 commented May 24, 2025

Copy link
Copy Markdown
Contributor

StatsManager.cs 文件中,添加了对装备强化属性的处理逻辑。新增了对 item.Enchantitem.Enchant.BasicOptions 的检查,以便将强化属性加到角色的属性值中,从而使角色的 GearScore 计算更加准确,考虑了装备的强化效果。Fixed the issue that enhancements were not implemented/applied to character attributes.(#452

Summary by CodeRabbit

  • New Features

    • Enhancement attributes from item enchantments and limit breaks are now included in character stats when equipping items.
  • Bug Fixes

    • Removed unnecessary debug logging related to critical damage stats.

在 `StatsManager.cs` 文件中,添加了对装备强化属性的处理逻辑。新增了对 `item.Enchant` 和 `item.Enchant.BasicOptions` 的检查,以便将强化属性加到角色的属性值中,从而使角色的 GearScore 计算更加准确,考虑了装备的强化效果。Fixed the issue that enhancements were not implemented/applied to character attributes.(MS2Community#452
@coderabbitai

coderabbitai Bot commented May 24, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

The AddEquips method in the StatsManager class was updated to include enhancement attributes from both the Enchant and LimitBreak properties of equipped items when calculating character stats. Additionally, a debug logging statement related to critical damage was removed.

Changes

File(s) Change Summary
Maple2.Server.Game/Manager/StatsManager.cs Extended AddEquips to add stat bonuses from Enchant.BasicOptions and LimitBreak.BasicOptions; removed debug log for critical damage in AddItemStats.

Poem

In the land where stat boosts grow,
Enchantments now join the show!
Equips and upgrades, side by side,
Player power amplified.
With every stat and bonus found,
The rabbit hops—enchantment-bound!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fe7f94f and 0caf261.

📒 Files selected for processing (1)
  • Maple2.Server.Game/Manager/StatsManager.cs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • Maple2.Server.Game/Manager/StatsManager.cs
✨ Finishing Touches
  • 📝 Generate Docstrings

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 853f8be and 31cfbc4.

📒 Files selected for processing (5)
  • Maple2.File.Ingest/Mapper/TableMapper.cs (1 hunks)
  • Maple2.Model/Metadata/Table/ScrollTable.cs (1 hunks)
  • Maple2.Server.Game/Manager/StatsManager.cs (3 hunks)
  • Maple2.Server.Game/PacketHandlers/ChangeAttributesHandler.cs (2 hunks)
  • Maple2.Server.Game/PacketHandlers/ChangeAttributesScrollHandler.cs (7 hunks)
🔇 Additional comments (12)
Maple2.Model/Metadata/Table/ScrollTable.cs (2)

32-35: LGTM! Clean metadata extension for item remake scrolls.

The new fields RollAttribute, RollValueType, and OnlyPet follow the existing pattern and naming conventions. The additions enhance the metadata structure to support more sophisticated scroll behavior.


37-41: Well-defined enum with logical progression.

The RollValueType enum has clear, meaningful names and follows a logical numerical progression. The values (None=0, Normal=1, HigherEnd=2) suggest increasing levels of enhancement.

Maple2.Server.Game/PacketHandlers/ChangeAttributesHandler.cs (2)

169-170: Consistent refactoring in HandleSelect method.

Good consistency applying the same GetGear() method change in both HandleChange and HandleSelect methods.


64-65:

Details

✅ Verification successful

Improved encapsulation with GetGear method.

The change from direct inventory lookup to session.Item.GetGear(itemUid) improves code encapsulation and readability. The fallback to pets inventory remains intact.

Verify that GetGear() method provides equivalent functionality to the original inventory lookup:


🏁 Script executed:

#!/bin/bash
# Description: Verify GetGear method implementation and ensure it handles the same scenarios as the original code

# Search for GetGear method definition
ast-grep --pattern $'GetGear($uid) {
  $$$
}'

# Also search for any GetGear method calls to understand usage patterns
rg -A 5 -B 2 "GetGear\(" 

Length of output: 7664


GetGear method verified – behavior matches and extends original lookup

The GetGear(long uid) implementation in Maple2.Server.Game/Manager/Items/ItemManager.cs first calls:

Inventory.Get(uid, InventoryType.Gear)

and then falls back to:

Equips.Gear.Values.FirstOrDefault(gear => gear.Uid == uid)

This covers both gear in the player’s inventory and equipped gear, meeting or exceeding the original direct inventory lookup. The refactoring improves encapsulation and preserves the pets fallback. No further changes needed.

Maple2.Server.Game/Manager/StatsManager.cs (1)

160-166: Critical enhancement: Enchantment attributes now properly applied to character stats.

This implementation correctly addresses the core PR objective by ensuring enchantment attributes contribute to character stats calculation. The code:

✅ Properly null-checks both item.Enchant and item.Enchant.BasicOptions
✅ Iterates through all basic enchantment options
✅ Adds values to the character's total stats using Values[kvp.Key].AddTotal(kvp.Value)
✅ Follows the existing code pattern in the same method

This fix ensures that equipment enhancements are accurately reflected in the character's GearScore and overall attribute calculations.

Maple2.File.Ingest/Mapper/TableMapper.cs (1)

892-895: Proper data mapping for new scroll metadata fields.

The field mappings correctly populate the new metadata properties:

  • RollAttribute: scroll.addOpKind == 1 - logical boolean conversion
  • RollValueType: (RollValueType) scroll.addOpValue - appropriate enum cast
  • OnlyPet: scroll.onlyPet - direct mapping

These mappings integrate seamlessly with the metadata extensions in ScrollTable.cs and provide the data foundation for enhanced scroll behavior.

Maple2.Server.Game/PacketHandlers/ChangeAttributesScrollHandler.cs (6)

1-1: LGTM: Standard library import added.

The addition of System.Diagnostics.CodeAnalysis is appropriate for nullable reference type support.


65-65: LGTM: Enhanced item retrieval to support pets.

The fallback to Pets inventory properly extends the functionality to handle pet items alongside gear items.


75-76: LGTM: Improved method signature for better data flow.

The addition of the output parameter eliminates duplicate metadata retrieval and improves code efficiency.


162-184: Excellent improvement: Atomic consumption with rollback.

This implementation properly handles the consumption of both items atomically, with appropriate rollback logic if either operation fails. This prevents partial state corruption and provides clear error messaging.


186-189: LGTM: Proper pet binding implementation.

The conditional binding logic correctly handles pet items with BindPet transfer type and appropriately notifies the client with an inventory update.


221-253: LGTM: Enhanced compatibility validation with pet support.

The method signature improvement and the addition of OnlyPet validation logic properly extend the compatibility checking to handle pet-specific scrolls. The validation logic correctly ensures pet-only scrolls are used exclusively on pet items.

Comment thread Maple2.Server.Game/PacketHandlers/ChangeAttributesScrollHandler.cs
@AngeloTadeucci

Copy link
Copy Markdown
Collaborator

Can you update your base branch so my changes don't show up?

@AngeloTadeucci AngeloTadeucci changed the title 增强角色属性计算逻辑 Fix: Add enchant stats to character May 26, 2025

@Zintixx Zintixx left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good! Although not implemented yet, you could go ahead and add the stats in the same way for limit break

            if (item.LimitBreak is not null) {
                foreach (KeyValuePair<BasicAttribute, BasicOption> kvp in item.LimitBreak.BasicOptions) {
                    Values[kvp.Key].AddTotal(kvp.Value);
                }
            }

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.

3 participants