Open jdrueckert opened 2 years ago
Hello, Please assign this to me
@jdrueckert could you assign this to me if it is still open?
Hi @vishalsingh2972, we typically don't assign issues to people until they at least provide a preliminary PR. So I suggest you take a stab at the issue and provide a PR with some initial changes and then we can assign it to you =)
Hi, is this issue still open?
@xbdhshshs Yes it is.
Great, then can I have a go at it? I've worked with Java and SpringBoot before but haven't contributed to open source. I will go through the contributing guidelines and the motivation stated above thoroughly, but if you could guide me to the specific files/directories to be worked with, that would be really helpful...
@jdrueckert
@xbdhshshs sure, you don't have to ask whether you can have a go at something :wink: To get started around here, I'd recommend having a look at our Contributor Quick Start Guide. And if you have questions you can always come talk to us on our Discord.
For the specific issue, I believe all relevant resources are already linked in the issue description. So feel free to onboard with the above mentioned guide, give the proposed solution for this issue a go and open a PR with your work (ideally as soon as possible, so we can have a look and comment on whether or not this goes into the right direction, etc.).
Thanks for the reply😁. I'll follow the guide and get on with it the issue asap.
Hello @jdrueckert, I'll try and have a poke at this topic if it's ok
@spookynutz sure, go ahead
Hello @jdrueckert,
I created a first draft of the manager but it is not complete yet since I have some interrogations. I added comments on my Draft PR since I thought it would be a better place to have the discussions. If you could have a look and help me clarify so that I can proceed with the rest. Thank you !
Motivation
Dependency injection via
@In
is a widely used mechanism throughout our code-base. However, it doesn't seem to be supported for all kinds of classes yet.While it is implemented for world generators, it is not yet implemented for world-gen plugins. If used for these, it can/will lead to NPEs due to fields not being properly initialized before being accessed.
Example
Typically, the pattern to get a dependency injected looks like this
If this fails,
blockmanager
will benull
and result in an NPE when being accessed.The current workaround for this issue is to get the dependency from
CoreRegistry
instead:Proposal
It would be nice to support dependency injection for world-gen plugins as well. The implementation would probably look similar to how it is already implemented for world generators.
The idea is to do the following:
@RegisterPlugin
, for instance the CaveRasterizer or the LakeProvider<class>.getAnnotation(RegisterPlugin.class)
<class>.getConstructor(SimpleUri.class).newInstance(<annotation>.id())
InjectionHelper.inject(<loadedPlugin>)
Additional notes
Related Issues: