Improvement: Added Tooltips that Explain the Skill When Viewing a Person in the Personnel Tab or Purchasing/Improving Skills #6839
Conversation
…son in the Personnel Tab or Purchasing/Improving Skills ### Dev Notes We frequently get contacted by users querying what one skill or another does. With the introduction of both aging effects, SPAs that affect skills, traits, _and_ attribute scores this was only going to get worse. I went ahead and created a set of tooltips that should significantly reduce confusion. Now, whenever a player is purchasing (or improving) a skill, or viewing a character in Person View, simply hovering over the skill will show not just what the skill does but what modifiers are affecting it.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6839 +/- ##
============================================
+ Coverage 11.69% 11.71% +0.02%
- Complexity 6656 6668 +12
============================================
Files 1100 1100
Lines 141324 141383 +59
Branches 21885 21891 +6
============================================
+ Hits 16525 16568 +43
- Misses 123101 123115 +14
- Partials 1698 1700 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces skill tooltips in both the Person View and personnel table popup menus to help users understand skill effects, modifiers, and relevant attributes in gameplay.
- Added tooltip generation in PersonViewPanel and PersonnelTableMouseAdapter
- Extended Skill and SkillType logic to incorporate attribute and reputation modifiers
- Updated file headers with appropriate legal notices
Reviewed Changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| MekHQ/src/mekhq/gui/view/PersonViewPanel.java | Updated skill display to show tooltips and adjusted final skill value computation using new attributes and reputation modifiers. |
| MekHQ/src/mekhq/gui/adapter/PersonnelTableMouseAdapter.java | Added reputation adjustments and tooltips in menu items, with a conditional check for artillery skills. |
| MekHQ/src/mekhq/campaign/personnel/skills/SkillType.java | Added resource key normalization for flavor text and updated flavor text generation. |
| MekHQ/src/mekhq/campaign/personnel/skills/Skill.java | Updated skill value and level computations to include attribute and SPA modifiers; introduced an HTML formatted tooltip method. |
| MekHQ/src/mekhq/campaign/personnel/skills/Attributes.java | Added a helper method to compute attribute modifiers. |
Files not reviewed (2)
- MekHQ/resources/mekhq/resources/Skill.properties: Language not supported
- MekHQ/resources/mekhq/resources/SkillType.properties: Language not supported
Comments suppressed due to low confidence (1)
MekHQ/src/mekhq/gui/adapter/PersonnelTableMouseAdapter.java:2941
- The variable 'skill' is used in this scope but has not been declared. Consider retrieving the Skill instance (e.g., via person.getSkill(typeName)) before calling getTooltip.
String tooltip = wordWrap(skill.getTooltip(person.getOptions(), person.getATOWAttributes(), adjustedReputation));
| * @author Illiani | ||
| * @since 0.50.06 | ||
| */ | ||
| public String getTooltip(PersonnelOptions options, Attributes attributes, int adjustedReputation) { |
There was a problem hiding this comment.
[nitpick] Consider adding explicit separators or additional line breaks between the tooltip components (aging, SPA modifiers, and attribute modifiers) to improve readability, in case the resource strings do not already include them.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Resource string covers this
# Conflicts: # MekHQ/src/mekhq/gui/adapter/PersonnelTableMouseAdapter.java
|
Removed 'For New Dev Cycle' tag. Updated to be suitable for 50.06 |
|
its alot of new code, i'd be happier if at least the newly added code in Skills was fully covered in tests instead of just 30% of it. |
|
It really isn't a lot of code if you consider that the past majority of it is in guy components or resource bundles. |
|
However, as much as I don't want to write even more tests, if there is specific things you think need testing lemme know. As it is, I tested what I thought needed to be tested. |
…ort Weapons, and Melee Weapons skills.
# Conflicts: # MekHQ/unittests/mekhq/campaign/personnel/skills/SkillTypeTest.java
# Conflicts: # MekHQ/resources/mekhq/resources/Skill.properties # MekHQ/resources/mekhq/resources/SkillType.properties # MekHQ/src/mekhq/campaign/personnel/skills/Skill.java # MekHQ/src/mekhq/campaign/personnel/skills/SkillType.java # MekHQ/src/mekhq/gui/view/PersonViewPanel.java
|
This PR was required by #6894 as that has been merged and this is approved, I'm going to go ahead and merge this. |
Dev Notes
We frequently get contacted by users querying what one skill or another does. With the introduction of both aging effects, SPAs that affect skills, traits, and attribute scores this was only going to get worse.
I went ahead and created a set of tooltips that should significantly reduce confusion. Now, whenever a player is purchasing (or improving) a skill, or viewing a character in Person View, simply hovering over the skill will show not just what the skill does but what modifiers are affecting it.