-
-
Notifications
You must be signed in to change notification settings - Fork 189
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
Add EntityType DeferredRegister #1854
base: 1.21.x
Are you sure you want to change the base?
Add EntityType DeferredRegister #1854
Conversation
Last commit published: d77dd19f2f82624ef4bb7db5a8cb733434f9b4a0. PR PublishingThe artifacts published by this PR:
Repository DeclarationIn order to use the artifacts published by the PR, add the following repository to your buildscript: repositories {
maven {
name 'Maven for PR #1854' // https://github.com/neoforged/NeoForge/pull/1854
url 'https://prmaven.neoforged.net/NeoForge/pr1854'
content {
includeModule('net.neoforged', 'neoforge')
includeModule('net.neoforged', 'testframework')
}
}
} MDK installationIn order to setup a MDK using the latest PR version, run the following commands in a terminal. mkdir NeoForge-pr1854
cd NeoForge-pr1854
curl -L https://prmaven.neoforged.net/NeoForge/pr1854/net/neoforged/neoforge/21.4.59-beta-pr-1854-feat-annoying-entitytype-generics/mdk-pr1854.zip -o mdk.zip
jar xf mdk.zip
rm mdk.zip || del mdk.zip To test a production environment, you can download the installer from here. |
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.
Looks fine. Wonder how many specialized registers we'll end up with 🤔
/** | ||
* Specialized DeferredRegister for {@link EntityType EntityTypes}. | ||
*/ | ||
public static class EntityTypes extends DeferredRegister<EntityType<?>> { |
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 wonder if it should be called Entities
to match DataComponents
above.
* @return A {@link DeferredHolder} which reflects the data that will be registered. | ||
* @param <E> the type of the entity | ||
*/ | ||
public <E extends Entity> DeferredHolder<EntityType<?>, EntityType<E>> registerType(String name, EntityType.EntityFactory<E> factory, MobCategory category, UnaryOperator<EntityType.Builder<E>> builder) { |
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 wonder if it should be called registerEntityType
to match registerComponentType
above.
Adds a
DeferredRegister
subclass forEntityType
s to fix the potential generic inference issues when there are multiple constructors present and the entity factory is constructed within a lambda. This also auto resolves the builder by supplying theResourceKey
in thebuild
method.The following will now resolve correctly when using the
registerType
method:Technically,
var
will still throw an exception as it will once again infer that the type of the entity isEntity
if there is more than one constructor. This could be fixed by just having theEntityType
in the constructor just take in a wildcard rather than be bounded, but that is not best practice, so I will be consideringvar
a non-issue.