-
-
Notifications
You must be signed in to change notification settings - Fork 179
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
[1.21] Add an Attribute formatting API for better control of attribute tooltips #1551
[1.21] Add an Attribute formatting API for better control of attribute tooltips #1551
Conversation
Docs need work, and the thing still needs to be tested, but the impl is here and it compiles.
Last commit published: ff9981d8b1511e67e5312d53cc1458df23d01862. PR PublishingThe artifacts published by this PR:
Repository DeclarationIn order to use the artifacts published by the PR, add the following repository to your buildscript: repositories {
maven {
name 'Maven for PR #1551' // https://github.com/neoforged/NeoForge/pull/1551
url 'https://prmaven.neoforged.net/NeoForge/pr1551'
content {
includeModule('net.neoforged', 'neoforge')
includeModule('net.neoforged', 'testframework')
}
}
} MDK installationIn order to setup a MDK using the latest PR version, run the following commands in a terminal. mkdir NeoForge-pr1551
cd NeoForge-pr1551
curl -L https://prmaven.neoforged.net/NeoForge/pr1551/net/neoforged/neoforge/21.1.85-pr-1551-attr-tooltips/mdk-pr1551.zip -o mdk.zip
jar xf mdk.zip
rm mdk.zip || del mdk.zip To test a production environment, you can download the installer from here. |
src/main/java/net/neoforged/neoforge/common/extensions/IAttributeExtension.java
Outdated
Show resolved
Hide resolved
src/main/java/net/neoforged/neoforge/client/util/TooltipUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/net/neoforged/neoforge/client/util/TooltipUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/net/neoforged/neoforge/client/util/TooltipUtil.java
Outdated
Show resolved
Hide resolved
cc'ing potentially relevant mod authors for comment: |
src/main/java/net/neoforged/neoforge/client/util/TooltipUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/net/neoforged/neoforge/client/util/TooltipUtil.java
Outdated
Show resolved
Hide resolved
@dhyces also pulling in ratatosk, other main alembic dev |
In terms of how I use attributes, the main concern I have is the ability to conditionally suppress attributes from the tooltip. My mod has item tooltips where holding control or shift displays a different set of info, and want attributes to show in 2 out of 3 of those modes (one with full info like they normally show, one with just the stats when worn in the proper slot). I currently accomplish this using the item tooltip event to suppress tooltips based on their translation keys, though I suspect this PR would make that task much more difficult. For my usecases, I ultimately need two things:
|
src/main/java/net/neoforged/neoforge/client/util/TooltipUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/net/neoforged/neoforge/client/util/TooltipUtil.java
Outdated
Show resolved
Hide resolved
The
I may be able to add per-slot logic to the event, though I worry about imposing a limitation on mods supplying attributes for non- |
I'll skip commenting a lot here because I don't actually use vanilla attributes ever since the mod was created. My tooltips are so large I just hide the vanilla tooltip and give a keybind to not hide, to well show the vanilla tooltip without my mod replacing it. I think this is more important for the other less overhaul-y rpg mod devs like Apotheosis.. Yeah you're the OP heh. I don't actaully know any other good rpg mods. What my mod also does is cut blank lines in tooltip if the tooltip gets too big.. which it does, even when i cut off the entire vanilla tooltip. I think forge was missing a scrollable tooltip mod last time I checked. Just problems that happen when you have: base, implicit, prefix, suffix, corruption stats on one item.. and a few other things I also have a system where pressing ALT, SHIFT, CTRL provide different functions to the tooltips. One makes my tooltips show stat ranges from the affix, one shows descriptions for stats, and the last one hides the tooltip and lets vanilla and mods do whatever. That's it from me. I initially unfollowed the post but got pinged by the quote.. Not sure how helpful this will be |
I think it’s better to allow mods who need this feature to have an option to enable it from code instead of having it fully controlled by config. For example, I have mods that need this merged tooltip to make my weapon tooltip look better. I think Apoth wants it as well. What about making config to be a tri-state (on/default/off), while default allows mods to turn them on? |
src/main/java/net/neoforged/neoforge/common/BooleanAttribute.java
Outdated
Show resolved
Hide resolved
src/main/java/net/neoforged/neoforge/common/extensions/IAttributeExtension.java
Outdated
Show resolved
Hide resolved
src/main/java/net/neoforged/neoforge/common/util/AttributeUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/net/neoforged/neoforge/common/util/AttributeUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/net/neoforged/neoforge/common/util/AttributeUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/net/neoforged/neoforge/common/util/AttributeUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/net/neoforged/neoforge/common/util/AttributeUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/net/neoforged/neoforge/client/event/GatherSkippedAttributeTooltipsEvent.java
Outdated
Show resolved
Hide resolved
src/main/java/net/neoforged/neoforge/client/event/GatherSkippedAttributeTooltipsEvent.java
Outdated
Show resolved
Hide resolved
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.
All looks good
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 looks good, for some reason my IDE isn't launching today and I don't feel like diagnosing it so I haven't actually tested it functions as advertised.
I've validated all the functionality I can by porting Apothic Attributes over to the PR publish artifact - everything looks clean on my end. If someone runs into a weird edge case (which is almost certain to happen), we'll fix it when it occurs. |
🚀 This PR has been released as NeoForge version |
Description
This PR supplies an implementation of Apothic Attributes' IFormattibleAttribute API, which allows attributes to supply custom formatting logic for their attribute modifiers (and modifier values), as well as giving mods better control over what information is displayed when attribute modifier tooltips are added.
This PR supersedes #1371.
Features
MutableComponent
(s), allowing attributes to map the raw numeric values to human-readable ones.a. This functionality is disabled by default, and must be enabled by a mod calling
NeoForgeMod.enableMergedAttributeTooltips()
.F3+H
is enabled to clarify the underlying modifier, regardless of customizations done by the attribute.Examples
Images from current implementation of various modifiers:
Default Settings:
With Advanced Tooltips:
With Merged Tooltips:
With Merged Tooltips Holding Shift: