Skip to content
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

Open
wants to merge 10 commits into
base: 1.20.4
Choose a base branch
from

Conversation

karatebiscuit
Copy link

@karatebiscuit karatebiscuit commented Apr 3, 2024

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 a cube_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:

public static class ModelGenerator extends FabricModelProvider
{
    private static final Block CHEESE_LOG = // ... 
    public ModelGenerator(FabricDataOutput output) { super(output); }

    @Override
    public void generateBlockStateModels(BlockStateModelGenerator blockStateModelGenerator)
    {
        // We can create the block's model & simple state like so:
        blockStateModelGenerator.blockStateCollector.accept(new SimpleBlockStateSupplier(CHEESE_LOG, new Identifier("modid", "cheese_log")));
        blockStateModelGenerator.modelCollector.accept(new Identifier("modid", "cheese_log"),
            () -> new FabricBlockModelSupplier("cube_column") // The parent type is the parameter.
                .addTextureData(new FabricTextureMap("end", "side")
                    .set(new Identifier("modid", "block/cheese_log_end"), new Identifier("modid", "block/cheese_log_side")))
                .get());
    }

    @Override
    public void generateItemModels(ItemModelGenerator itemModelGenerator)
    {
        // ...
    }
}

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.

{
  "parent": "minecraft:block/cube_column",
  "textures": {
    "end": "modid:block/cheese_log_end",
    "side": "modid:block/cheese_log_side"
  }
}
{
  "parent": "modid:block/cheese_log"
}

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:

// Model Generator code...
protected void simpleItem(ItemModelGenerator generator, String ID)
{
	generator.writer.accept(new Identifier("modid", "item/" + ID),
			() -> new FabricBlockModelSupplier("modid", ID)
					.get());
}

@Override
public void generateItemModels(ItemModelGenerator itemModelGenerator) {
	simpleItem(itemModelGenerator, "myblockitem")
}

This generates a simple Item Model JSON like so:

{
  "parent": "modid:block/myblockitem"
}

This is because of the FabricBlockModelSupplier's support for custom model types and it's immediate initialization of the "parent" JSON.

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.
@karatebiscuit karatebiscuit deleted the patch-1 branch April 4, 2024 06:09
@karatebiscuit karatebiscuit restored the patch-1 branch April 4, 2024 06:10
@karatebiscuit karatebiscuit reopened this Apr 4, 2024
@anscg
Copy link

anscg commented Apr 4, 2024

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
@karatebiscuit karatebiscuit changed the title Add a FabricBlockModelSupplier Add a FabricBlockModelSupplier and FabricTextureMap for easier data generation with Block Models. Apr 4, 2024
Add SimpleBlockStateSupplier for no-variant based blockstates (because that apparently isn't something built-in already??)
@karatebiscuit karatebiscuit changed the title Add a FabricBlockModelSupplier and FabricTextureMap for easier data generation with Block Models. Add a FabricBlockModelSupplier, FabricTextureMap and SimpleBlockStateSupplier for easier data generation with Block Models & States. Apr 4, 2024
@apple502j
Copy link
Contributor

@hellvet3 Hmm, I'm sure vanilla has them?

@karatebiscuit
Copy link
Author

karatebiscuit commented Apr 6, 2024

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 SimpleBlockStateSupplier it's mainly to generate BlockState files like this which (as far as I know) are just not creatable on it's own, for some reason:

{
    "variants": {
        "": {
            "model": "modid:block/myblockmodel"
        }
    }
}

Whilst I'd understand SimpleBlockStateSupplier not being merged, I do think a FabricBlockModelSupplier could seriously help because there isn't really a class vanilla or from the Fabric API that acts like this - the Block Model generation has almost if not nothing for it.

* {@see FabricBlockModelSupplier}
*/

public class FabricTextureMap {
Copy link
Contributor

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?

Copy link
Author

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.

Copy link
Author

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.

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?

Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants