- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 776
 
Full Java tooltip parity (as much as that is possible) #5692
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
base: master
Are you sure you want to change the base?
Full Java tooltip parity (as much as that is possible) #5692
Conversation
…ps, fix ominous bottle tooltip
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.
this is another insane PR, the 3rd or 4th registry refactor? xd
I'm sceptical about how useful it is to have occasionally duplicate/conflicting tooltips. Might be worth exploring whether we can use the optional pack to yeet them entirely?
Also, performance of this could be... interesting. Same as maintainability; but with components being now as far as they are I'd assume this wouldn't change too much
Haven't done a proper review yet, mainly because I'd like to test it to see how much this would actually be "confusing"; and to see if there's possibilities to cook with the optional pack!
        
          
                core/src/main/java/org/geysermc/geyser/item/tooltip/TooltipProviders.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
      | // TODO tooltips for default components? | ||
| public static void addNameTooltips(TooltipContext context, DataComponents componentPatch, Consumer<String> adder) { | ||
| // TODO | ||
| } | 
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.
Are these (more or less) already being added by the Bedrock client?
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.
| return is(tag, itemStack.asItem()); | ||
| } | ||
| 
               | 
          ||
| 
               | 
          
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.
# Conflicts: # core/src/main/java/org/geysermc/geyser/translator/item/ItemTranslator.java
This PR intends to fully implement Java tooltip partity, or as much as that is possible.
This PR fixes or will fix the following issues:
minecraft:tropical_fish_patterncomponent only show up on tropical fish buckets.minecraft:instrumentcomponent only show up on goat horns.minecraft:can_place_onandminecraft:can_breakcomponents (new Java parity).... among other issues as well. This PR also adds new functionality to
MinecraftLocale, to load thedeprecated.jsonlanguage file (introduced in Java 1.21.2), which contains removed and replaced language keys, and applies it to loaded locales. This ensures the proper translation keys are resolved.Along with these changes, this PR also refactors the loading of data-driven registries.
JavaRegistryKeys now support parsing and translating MCPL'sHolders to objects loaded by Geyser, when possible. Some refactors to loaded registry objects, such asJukeboxSongandGeyserInstrumenthave also been made, to accommodate for the new way of building tooltips.Of course, this PR also overhauls the building of tooltips in the
ItemTranslatorclass, and spreads this over multiple classes to increase code readability.This PR is still unfinished and there are some things left to do. Here are some things worth noting:
Here are some images to illustrate a couple of the new tooltip changes (left Java, right bedrock. Apologies for the difference in screenshot size):