-
Notifications
You must be signed in to change notification settings - Fork 23
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
Mod initialization functions #141
Conversation
I think we should have a simple, descriptive |
Maybe creating an interface for mod dependencies handlers. That way you NOVA would check the hashmap if it contains any classes bound to mod id, That way a mod dev could write something like:
Dependencies should be handled in method called for every mod before So in mod annotated class a method:
These are in no way ideal and their names should probably changed. Also, I |
I agree that the initialization stages are ambiguous. The thing we have to allow is for cases as follows: Mod B uses Mod A block as recipe This will fail if Mod B loads after A. One option I propose is to lazy evaluate everything. In other words, all registration is stored and not used until the game needs it. |
Well, in the register methods you are telling the NOVA Wrapper what to register, instead of when. I would say register runs before forge preInit, and then NOVA registers stuff to forge at the appropriate time. |
@RX14 agree. And actually we might as well make registration happen in constructor, since it also prevent init being called twice... |
Shovelling everything into a constructor sounds strange... |
On the other hand, mods are less likely to have to store refs to the BlockManager etc. |
@RX14 Yeah I agree. Will think into that because then all mod classes will be one big constructor :) I also want to try make mod construction less annotation based. |
And I mean, ain't nothing stopping ya from calling alternate methods in a On Sat, 25 Jul 2015 2:11 pm Calclavia notifications@github.com wrote:
|
I still think it's possible to scan the classpath for stuff to register, and register it. That cuts down on the registration we need to do. Factories etc. still needed for custom args. |
It's still nicer to have more control and less "magic" happening. I prefer to rely less on ASM. |
Yeah, it should be possible completely to generate block types dynamically. On Sat, 25 Jul 2015 9:46 pm Calclavia notifications@github.com wrote:
|
I never said get rid of the current register system, but to autodetect things that could be registered and register them. Maybe still a bit too much magic. Integration-based registers? |
Like registering a block that only exists if another mod exists. On Sat, 25 Jul 2015 9:51 pm Chris Hobbs notifications@github.com wrote:
|
That would be in the mod post init hooks that I mentioned before. |
Fine. On Sat, 25 Jul 2015 9:52 pm Chris Hobbs notifications@github.com wrote:
|
Adding a note here for when I implement this:
|
Bleh. My proposal: add a way for mods to specify arbitrary loading stages. Let's say NOVA-Core adds an "initializeBlocks" stage. If you were to initialize blocks, then, you'd create a loading stage "MyMod:blocks" which happens before "initializeBlocks". That way, you could also introduce arbitrary loading stages which happen nowhere in particular, like "BuildCraft:initializeBlueprintSchematics". Then another mod could make their own loading state which happens right after that. And create a dependency tree out of it which allows for a lot more granularity than anything else. It might be slightly overengineered, however. Needs a way to make it user-friendly. |
@asiekierka That could be easily done with an annotation, I guess. For example: @Initialize("blocks", "nova:blocks") // or
@Initialize("blocks", "nova:blocks, after=OtherMod:blocks") // or
@Initialize("blocks", "nova:blocks", after="OtherMod:blocks") // or
@Initialize("blocks", "after=OtherMod:blocks") // This would throw a dependency error if
// OtherMod:blocks doesn't actually exist.
public void initializeModBlocks() { ... } Where this function could be referenced as Though I'm not sure I would support this way of doing things. You could end up with some pretty nasty dependency problems, because modders could arbitrarily choose to do things whenever. Registering everything at construction and then actually initializing things when it seems most fitting for the wrapper seems to make the most sense to me. |
What advantages does this have over the previous single stage loading that I suggested. It simply seems more complex to me. |
@RX14 - more granularity, I guess. What if you need to replace a block after it is registered? Etc. It's just an idea. |
This seems pretty nice imo, along with the previous register all before On Wed, 29 Jul 2015 7:12 pm Adrian Siekierka notifications@github.com
|
@copygirl We could (instead of using annotations), have methods that have the corresponding manager arguments.
Then again, I'm not a big fan of using lots of annotations. In fact, I'm trying to avoid too many annotations as they voids any possibly to do things dynamically (also because it's going to be painful to implement annotations in NOVA JS). We could simply do method registration into the GlobalEvent handler, that gets called when the corresponding init stage is required.
|
@Victorious3 all I am saying is for managers to have an eventbus too, with registration events. The event might be cancellation for very easy, explicit, functional style registration filtering. |
So my decision is to try lazy loading, see if that works well. PROGRESS:
TODO: #196 |
Suggestion: We can do named event handlers. events
.on(BlockEvent.Register.class)
.withName(this.modID)
.before(otherModID)
.after(otherModID)
.after(otherModID)
.bind(this::registerBlocks); This method of registration is enforced as such: Methods to register blocks will only exist on the block register event. |
@calclavia this is good base stuff, but having a more simple abstraction over this would be nice. |
@RX14 Such as? Any example code you have in mind? |
No idea, sorry. Implement this for now, and we can decide on better abstractions later. |
What about the lazy initialization idea? |
@copygirl It's difficult to handle dependencies with lazy init... Lazy init example code for your mod's main class: public ModName(BlockManager blockManager){
blockManager.register(...);
blockManager.register(...);
blockManager.register(...);
} After that, NOVA handles when the blocks are actually registered with Forge. How do we handle if mod A wants to register blocks after mod B? I feel like the event system is nicer, unless you can come up with a good suggestion. |
👍 for named event stuff |
@anti344 Working on it right now... Topological sorting doesn't seem lightweight. |
Current coverage is
|
Improve mod initialization functions
@@ -91,4 +95,86 @@ public void testRemovalByObject() { | |||
|
|||
assertThat(event.toString()).isEqualTo("A"); | |||
} | |||
|
|||
@Test | |||
public void testNamedPriority1() { |
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.
testNamedPriorityBefore
Looking at the example code.
I think this is a bad design choice for a couple of reasons:
There should either be methods for handling specific initialization code (config, blocks, items, recipes, ...) or even have it done automatically using reflection, and let the wrapper decide the proper time to do everything.