-
Notifications
You must be signed in to change notification settings - Fork 399
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 a FabricBlockModelSupplier, FabricTextureMap and SimpleBlockStateSupplier for easier data generation with Block Models & States. #3685
base: 1.20.4
Are you sure you want to change the base?
Conversation
Created the FabricBlockModelSupplier for data generation, makes making the [modid:block/myblock.json] model data easier.
Makes the FabricBlockModelSupplier more readable than initially, and also updates the constructor, enforcing a parent type to be specified to restrict possible errors.
For some reason it wasn't referred to as FabricBlockModelSupplier but BlockModelSupplier in the class, so changed it to FabricBlockModelSupplier.
Makes the code more readable (in general, as well as for people in-editor) and inline with the other fabric code.
LGTM! |
…ouble bracket indentation, or outer variables. Also easier to work with due to the use of Tuples as variables for FabricTextureMap. FabricBlockModelSupplier now requires a FabricTextureMap parameter when .addTextureData() is called. A provided example of a cube_all block model has also been provided in DataGeneratorTestEntrypoint
Add SimpleBlockStateSupplier for no-variant based blockstates (because that apparently isn't something built-in already??)
@hellvet3 Hmm, I'm sure vanilla has them? |
Vanilla has some capabilities from what I know - but as a whole it's lacking a lot of features. What I showed was a class that could actually easily generate block model jsons, because there isn't many if not any built in classes for that already - but from what I can see anything that does actually generate Block Models aren't very accessible, customisable, etc. As for the {
"variants": {
"": {
"model": "modid:block/myblockmodel"
}
}
} Whilst I'd understand |
* {@see FabricBlockModelSupplier} | ||
*/ | ||
|
||
public class FabricTextureMap { |
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.
Could you explain the difference between this and vanilla TextureMap?
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.
Can't actually remember why I wrote that, I will probably have to exclude it from the commits - I'll look into it soon though as I'm currently away.
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.
The difference between the two appears to be that the FabricTextureMap
will just return the raw HashMap because it is much easier to access that way, the vanilla TextureMap
appears to have no direct way to actually get the entries from a separate class, the existence of FabricTextureMap
is to make it easier to create inline without the use of double-bracket indentation (since some don't know of it) for HashMaps, although initially it was just taking in a HashMap.
This is how we get the entries for a FabricTextureMap
:
HashMap<String, Identifier> textureMap = fabricTextureMap.get();
You can't get the entries from a TextureMap
- in fact, you can't access any of the inputted data without at least having one bit of pre-existing data. I'm contemplating whether to just revert back to the HashMap being a parameter or keeping it as FabricTextureMap
, since some could argue it is technically redundant.
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.
couldn't this be a simple interface injection to add a method to do so?
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.
Yeah, it doesn't really matter to me which implementation we go for (HashMap parameter, interface injection, etc.) - I'm mainly focused on the FabricBlockModelSupplier
and the SimpleBlockStateSupplier
, the existence of FabricTextureMap
isn't necessarily required, although helpful. It's more like syntax sugar.
Currently, data generation is very new and restricted. In this case, block model generation is very limited - being incredibly inaccessible and anything even slightly more complex than a
cube_all
model requires custom classes to be built for generating them. A FabricBlockModelSupplier would fix this issue, where you could easily simply provide the parent type and a (see below) new class - FabricTextureMap with the texture name and location, which can then be turned into a resulting JSON. As an example, creating acube_column
model can be easily made, like so - the SimpleBlockStateSupplier allows for no-variant based blockstates to be generated easily, because this isn't a built-in feature for fabric at the moment:Which would then be outputted as this, and if done correctly should also generate a basic block parent item model, as well - however some methods won't produce Item Models. Unsure as to the reason of this, looking into it.
The creation of a FabricTextureMap not only could potentially help in the long run with other future classes requiring something similar but also, as a whole, it's much easier to use than a raw HashMap - it allows for the code to be more readable than initially (especially without double bracket indentation) and also reduces the line count and should also help reduce external variables being used when people wouldn't use double bracket indentation for the HashMap.
As a whole, I think these additions could greatly help the data generation part of fabric, although more specifically attributed towards Block Models - although the FabricBlockModelSupplier could work perfectly fine with Item Models, as well - for example, this is a snippet of how I had recently used it as a custom class for one of my mods more recently:
This generates a simple Item Model JSON like so:
This is because of the FabricBlockModelSupplier's support for custom model types and it's immediate initialization of the "parent" JSON.