-
Notifications
You must be signed in to change notification settings - Fork 0
Loot system improvment #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Loot system improvment #10
Conversation
Merge Master
1:1 translation
Added the ability to choose disenchant on loot if the bot is enchanting
…r117/mod-playerbots into Loot-System-Improvment
Centralized “who gets priority” into a generic fallback: if no bot in the group has the item as a mainspec upgrade (EQUIP/REPLACE + IsPrimaryForSpec), then offspec upgrades are allowed to keep NEED instead of being forced to GREED. Healer + Hit items are now handled only via IsPrimaryForSpec (healer treats SP/INT/Hit/no-regen as offspec), and the generic fallback decides whether they can still NEED when no DPS caster bot wants it. Fixed a compile issue by adding a forward declaration for IsPrimaryForSpec before group-level helper functions that call it.
Summary of ChangesHello @Celandriel, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive overhaul of the bot's loot rolling system, making their decisions more sophisticated and aligned with common loot etiquette. The changes aim to improve bot efficiency and fairness in group loot scenarios by providing detailed logic for 'Need', 'Greed', 'Pass', and 'Disenchant' choices based on various item and bot characteristics. This significantly enhances the bot's ability to manage inventory and gear progression within a group. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a significant improvement to the bot loot system, making it much more intelligent and configurable. It adds a lot of new logic to handle different item types, specs, and group situations. The changes are extensive and well-thought-out. I've found one high-severity bug in the legacy loot rolling logic and a couple of areas where the code could be refactored for better maintainability due to the complexity of the new functions. Overall, this is a great enhancement.
| } | ||
| } | ||
|
|
||
| static bool IsPrimaryForSpec(Player* bot, ItemTemplate const* proto) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The IsPrimaryForSpec function is very complex and long (almost 300 lines). It contains a large number of rules for different class/spec combinations. This makes it difficult to read, understand, and maintain. Please consider refactoring this function into smaller pieces. For example, the logic for each class or role (physical DPS, caster, tank) could be extracted into separate helper functions.
| } | ||
|
|
||
| RollVote LootRollAction::CalculateRollVote(ItemTemplate const* proto) | ||
| RollVote LootRollAction::CalculateRollVote(ItemTemplate const* proto, int32 randomProperty) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CalculateRollVote function is very long (over 200 lines) and handles many different loot rolling rules. To improve maintainability and readability, consider refactoring this function by extracting parts of the logic into smaller, well-named helper functions. For example, the logic for BoE/BoU items, duplicate items, unique items, and cross-armor upgrades could each be in their own function.
No description provided.