Make dynamic registry element path include namespace #4180
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
|
An additional thing I will note is that fixing this realigns the behavior with the docs of |
The old method of prepending only applied in certain locations, and thus gives different results depending on the circumstance
|
Change seems fine to me, would it be possible to add some tests to help ensure that this doesnt break again in the future? Could maybe expand one of the existing dynmatic registry tests instead of creating a whole new one. I get that its a breaking change, but I think the best solution is to just go for it in 1.21.3 otherwise we will be waiting a long time to fix. If anyone disagrees please let your thoughts be known. |
|
I can add tests, sure. |
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.
Fixes #4179
Note that since this changes the loading location of modded dynamic registries, this is technically a breaking change to datapacks. However, with 1.21.2 being brand new, and that version already breaking datapack folder structures (with plurals becoming singular), I think now is the perfect time to fix this.