Skip to content

Conversation

mosemister
Copy link
Member

@mosemister mosemister commented Jan 3, 2025

API | Impl

Adds key to recipe

@mosemister
Copy link
Member Author

First implementation PR for Sponge so if there is any PR standards then please comment below.

As this PR itself, I couldn't think of a situation whereby a recipe wouldn't have a key, but as I don't play modded Minecraft and this is the first time looking at the minecraft code, I played it safe with a optional

@aromaa
Copy link
Member

aromaa commented Jan 3, 2025

Recipes are part of registry so they follow the pattern where only the registry knows its key, not the item itself, so this is by design.

The problem is that we are missing recipes from RegistryTypes because we were not accounting for reloadable registries which the SpongePowered/Sponge#4145 is trying to resolve. Internally vanilla has layers for their registries where different layer builds upon another layer. I don't remember all the details but there were some concerns around the bootstrapping.

@mosemister
Copy link
Member Author

Recipes are part of registry so they follow the pattern where only the registry knows its key, not the item itself, so this is by design.

The problem is that we are missing recipes from RegistryTypes because we were not accounting for reloadable registries which the SpongePowered/Sponge#4145 is trying to resolve. Internally vanilla has layers for their registries where different layer builds upon another layer. I don't remember all the details but there were some concerns around the bootstrapping.

Thanks for the information. Ill change the implementation to just make it so you can call RegistryManager methods again without exception (the original point of me digging - just found that recipe had keys along the way)

@mosemister mosemister closed this Jan 3, 2025
@mosemister mosemister deleted the api-12_recipe-fix branch January 3, 2025 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants