-
Notifications
You must be signed in to change notification settings - Fork 359
Hook into fabric recipe sync. #4134
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
Conversation
mezz
left a comment
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.
Thank you for contributing!
| var recipeSerializer = new JeiShapedRecipe.Serializer(); | ||
| var registered = Registry.register(BuiltInRegistries.RECIPE_SERIALIZER, resourceLocation, recipeSerializer); | ||
| RecipeSerializers.register(() -> registered); | ||
| RecipeSynchronization.synchronizeRecipeSerializer(recipeSerializer); |
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.
JeiShapedRecipe do not exist on the server, so I think we can remove this
| for (var serializer : BuiltInRegistries.RECIPE_SERIALIZER.keySet()) { | ||
| if (serializer.getNamespace().equals(ResourceLocation.DEFAULT_NAMESPACE)) { | ||
| RecipeSynchronization.synchronizeRecipeSerializer(Objects.requireNonNull(BuiltInRegistries.RECIPE_SERIALIZER.getValue(serializer))); |
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.
I think it would be better to iterate over entries instead of the keySet, so that you don't need Objects.requireNonNull
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.
Originally did that, but this ended it slightly cleaner (andwith few less lookups I guess). Technically it can't be null anyway, just added that so intelij doesn't warn
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.
Okay checked and doing it the other way around still wants Objects#equireNonNull, as Registry#getKey can technically return null.
Fabric/src/main/java/mezz/jei/fabric/JustEnoughItemsClient.java
Outdated
Show resolved
Hide resolved
| ClientLifecycleHandler clientLifecycleHandler = new ClientLifecycleHandler(); | ||
|
|
||
| ClientRecipeSynchronizedEvent.EVENT.register((minecraft, synchronizedRecipes) -> { | ||
| Internal.setClientSyncedRecipes(RecipeMap.create(synchronizedRecipes.recipes())); |
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.
For the future, it may be nice if the event provided a shared RecipeMap, to avoid having multiple mods create it
With FabricMC/fabric-api#4926 being merged and released as Fabric API 0.137.0+1.21.10, JEI can now release updated version on Fabric.
Naturally the mod is needed on server to works (same as neoforge) to enable required recipes.
Changes are pretty simple, since fabric also just syncs Recipe objects.
Solves Fabric side of #4040