Rename the addAlias parameter names
This should help reduce confusion with the method for people
Simply put, it is a tiny, few second PR just to make the param names a bit more clear on the surface at a glance when a beginner looks at the method. The hope is it would help reduce possible confusion at no cost to us.
Especially as if a modder accidentally flips the param values, it could take a while before they realize their mistake. Even though one can argue the javadoc is clear enough, I don't seem harm in doing further clarity
- [ ] Publish PR to GitHub Packages
As ~~the cause~~ my original joke about always misunderstanding these parameters seems to be the cause of this PR, I thought I'd drop in my perspective: I recognize that the wording is quite clear for some people, but for some reason it causes inordinate confusion for me and every time I've used it recently I've gotten it wrong.
The problem (that I have) with the existing language:
Adds an alias that maps from the name specified by
fromto the name specified byto
For me, I find it possible to read this two ways with equal weight:
- From the name of the existing object specified by
from, to the "alias" that doesn't exist,to. - The inverse of the above -- from the "alias" specified in
fromto the existing object specifiedto.
The suggested oldName and newName do come from a perspective of "we've renamed an object" which certainly seems to be the primary use case but isn't necessarily the entire use case.
My initial suggestion had been "alias" and "existing", and I felt that this made it quite clear that one of the objects was the registered object that actually exists, while the other is clearly an "alias".
Basically, some way to demonstrate that "this parameter is the concrete, registered object" and "this parameter is the alias for that object" is what would be very useful for my weird brain.
PR build check fails due to whitespace in the javadoc; please run the applyAllFormatting task.
@neoforged/bots run applyAllFormatting
I personally find the use of registeredName (and specifically it's ResourceLocation) to be much clearer than previous efforts.
Added overload that takes a Holder. I can revert this commit if this is not desired. But could just be a nice helper for people that want to feed it the holder instead of manually extracting or copying the resourcelocation destination
@TelepathicGrunt, this PR introduces breaking changes. Unfortunately, this project is not accepting breaking changes right now. Please revert them before this PR can be merged. Last checked commit: 8687e669ab2c14a96065d346d4c1a0b8de133642.
neoforge (:neoforge)
net/neoforged/neoforge/registries/IRegistryExtensionaddAlias(Lnet/minecraft/resources/ResourceLocation;Lnet/minecraft/core/Holder;)V: ❗ Method was made abstract
I really support the Holder<T> override (in whatever pattern is decided upon), as it concretely provides answers to the questions that confuse me: the alias ResourceLocation is the name that doesn't exist which will be created and point towards the Holder, which is the concrete object that does exist.