Improve tag loading and load non-vanilla tags as well #4849
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Camotoy
reviewed
Jul 16, 2024
core/src/main/java/org/geysermc/geyser/session/cache/TagCache.java
Outdated
Show resolved
Hide resolved
Camotoy
reviewed
Jul 16, 2024
core/src/main/java/org/geysermc/geyser/session/cache/tags/TagRegistry.java
Outdated
Show resolved
Hide resolved
…y#getVanillaTags return original map
…rics # Conflicts: # core/src/main/java/org/geysermc/geyser/entity/type/living/animal/PigEntity.java # core/src/main/java/org/geysermc/geyser/entity/type/living/animal/RabbitEntity.java # core/src/main/java/org/geysermc/geyser/entity/type/living/animal/StriderEntity.java # core/src/main/java/org/geysermc/geyser/entity/type/living/animal/horse/CamelEntity.java # core/src/main/java/org/geysermc/geyser/item/enchantment/Enchantment.java # core/src/main/java/org/geysermc/geyser/session/cache/RegistryCache.java
|
A lot has changed now, generics have been implemented for type safety and some stuff has moved around or been added. Pretty happy with how it looks now, but this definitely still needs testing. |
Camotoy
reviewed
Oct 21, 2024
core/src/main/java/org/geysermc/geyser/session/cache/tags/HolderSet.java
Outdated
Show resolved
Hide resolved
Camotoy
reviewed
Oct 21, 2024
core/src/main/java/org/geysermc/geyser/session/cache/tags/HolderSet.java
Outdated
Show resolved
Hide resolved
|
Both of @Camotoy's reviews have been addressed in the latest commit. I've also done some basic gameplay testing and let Geyser make a printout of all loaded tags to confirm both vanilla- and non-vanilla tags loaded successfully. |
Camotoy
reviewed
Oct 22, 2024
core/src/main/java/org/geysermc/geyser/session/cache/tags/HolderSet.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/geysermc/geyser/session/cache/tags/HolderSet.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/geysermc/geyser/session/cache/tags/Tag.java
Outdated
Show resolved
Hide resolved
onebeastchris
approved these changes
Oct 22, 2024
core/src/main/java/org/geysermc/geyser/session/cache/tags/GeyserHolderSet.java
Outdated
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR improves the way Geyser loads tags sent by the server, which allows both vanilla and non-vanilla tags to be loaded. It also makes it easier to add new registries to load tags for. Loading all tags for a registry, and not just vanilla ones, is important for datapack support and matches the behaviour of the Java client. Datapacks can define custom tags and then use these elsewhere, such as in custom enchantments.
To start with, the PR adds a new
JavaRegistryKey<T>record, which is used to define a Java registry (can be both hardcoded or data-driven). The record doesn't store the contents of a registry (this is done byRegistryCacheor other classes for hardcoded registries), rather, it stores its registry key, and optionally a network serializer- and deserializer for translating between network IDs and registry objects.JavaRegistryKey<T>is used to define registries that Geyser should load data-driven objects (inRegistryCache) or tags (inTagCache) for. Instances of this record are created and stored in the newJavaRegistriesclass, which also has methods to allow for easy creation of the registry keys.Along with this, the PR also adds a new
Tag<T>record, which stores aJavaRegistryKey<T>and aKey- the first being the registry of the tag and the second being the actual tag key.TagCachenow uses theTag<T>record for tag lookup. The old way of looking tags up inint[][]arrays has been replaced by a simpleMap<Tag<?>, int[].TagCachenow loads all registries listed inJavaRegistries, as long as they satisfy the requirements for loading tags - that is, they must have a network serializer- and deserializer registered to be able to translate between network IDs and registry objects, which is required for the functionality ofTagCache.Along with the changes to tag loading, this PR also adds a new
HolderSetclass, which makes loading and using Minecraft's holder sets easier within Geyser, and has a better integration with Geyser's tag cache.Basic gameplay testing, as well as confirming both vanilla- and non-vanilla tags load by letting Geyser make a printout of loaded tags, has been done.