search

rvandoosselaer / Blocks

A block (voxel) engine for jMonkeyEngine
BSD 3-Clause "New" or "Revised" License
41 stars 14 forks source link

Disable default blocks, shapes and types registeration #61

Closed Ali-RS closed 3 years ago

Ali-RS commented 3 years ago

Hi @rvandoosselaer

I would suggest not register default blocks, types, shapes in the registry constructor and let the user code call this if he wants. For example, in my case, I do not use the default ones and I am adding my own blocks.

https://github.com/rvandoosselaer/Blocks/blob/4528e175f8b23232605a37ae014326dd3a9378e7/src/main/java/com/rvandoosselaer/blocks/BlockRegistry.java#L32-L34

https://github.com/rvandoosselaer/Blocks/blob/4528e175f8b23232605a37ae014326dd3a9378e7/src/main/java/com/rvandoosselaer/blocks/ShapeRegistry.java#L33-L35

https://github.com/rvandoosselaer/Blocks/blob/4528e175f8b23232605a37ae014326dd3a9378e7/src/main/java/com/rvandoosselaer/blocks/TypeRegistry.java#L52-L57

I know I can use the clear() method to remove them but I want them to not get registered in the first place :) (I mean loading all those textures,... would be a waste If I am going to clear them after)

Please let me know if you agree with this proposal and I will submit a PR if you want.

rvandoosselaer commented 3 years ago

I am convinced that by default, the basic shapes, types and blocks should be loaded. The lesser boilerplate code that is needed for a new user to get up and running the better.

It's possible to clear all the registries after initialization. In this case you will load, clear and reload all the registries but this is unnecessary waste of time and resources as you already mentioned. An option could be to add a new initializeWithoutDefaults(assetmanager) method to the BlocksConfig singleton, that will in turn create the registries but without registering default entries.

Would this be an option, or do you see any other possibilities?

Ali-RS commented 3 years ago

Agreed :+1:

or better to do BlocksConfig.initialize(assetmanager, loadDefaults) and then the existing method (BlocksConfig.initialize(assetmanager)) should delegate the call to above method with loadDefaults set to true.

I can submit a PR if you like

rvandoosselaer commented 3 years ago

That's ok for me. Would be awesome if you can create a PR for it.

Ali-RS commented 3 years ago

PR is ready: #62

rvandoosselaer commented 3 years ago

I'll release a new version with this change in the coming days.