Add a CustomShape override, for changing shape based on blockstates.

[Tcat2000]

Description

Adds a callback to change the shape of a block (in addition to .box as default) based on blockstate values.

Example Script

StartupEvents.registry("block", event => {
    event.create('example')
    ...
    .box(5, 16, 5, 11, 14, 11) // is still used to set default shape used if the callback is not set, does not
                               //  run event.box, as the shape, and as the shape for block model generation.
        .shape(event => { // used to create a callback for the shape
        	console.info(event.getStateValue("age"));
        	if(event.getStateValue("age") == 0) event.box(4, 16, 5, 11, 12, 11);
        	if(event.getStateValue("age") == 1) event.box(4, 16, 4, 11, 10, 11);
        	if(event.getStateValue("age") == 2) event.box(3, 16, 3, 12, 8, 12);
        	if(event.getStateValue("age") == 3) event.box(3, 16, 3, 13, 6, 13);
        	if(event.getStateValue("age") == 4) event.box(2, 16, 2, 14, 4, 14);
        })

[ChiefArug]

I do not think this is the way to implement this.
Shape code is VERY hot, and you should cache with it as much as possible, something that scripters cannot be relied on to do (your example doesn't even cache).

Instead I think the best way to implement this would be a generate shapes callback that fires once for each blockstate permutation, then KubeJS caches the results of that and returns them based on the blockstate passed in.

[Tcat2000]

Ok, I will work on this, 2 questions, 1, do I / should I, 1 add this on other game versions of the mod, and 2, what would it take to get verified on the discord, I'd like to chat and ask for help, but I don't have a phone number to use.

[Tcat2000]

I did put a log line in the shape callback, it it was called a lot.

[Tcat2000]

Ok, I rewrote the system to be more performant.
New example script

StartupEvents.registry("block", event => {
    event.create('example')
    ...
        .property(BlockProperties.AGE_4)
        .box(5, 16, 5, 11, 14, 11) // is still used to set default shape used if the callback is not set, does not
                               //  run event.box, as the shape, and as the shape for block model generation.
        .box({"age": 0}, 5, 16, 5, 11, 12, 11)
        .box({"age": 1}, 4, 16, 4, 12, 10, 12)
        .box({"age": 2}, 3, 16, 3, 13, 8, 13)
        .box({"age": 3}, 3, 16, 3, 13, 6, 13)
        .box({"age": 4}, 2, 16, 2, 14, 4, 14)

[Tcat2000]

All of the possible combinations are saved to a map, and in the getShape(...) method, it simply gets all of the properties, and passes them to map.get(), and returns the result.

[Tcat2000]

found an issue, working on fixing

[ChiefArug]

Looks pretty good as is, just some suggestions.

[ChiefArug]

Mods/Vanilla typically use arrays to store shapes as each blockstate property value can be simplified down to an index.
I don't think that is feasible here, but it should be an IdentityHashMap and use BlockStates (because only one BlockState instance exists per combination of property values) as keys so that is has slightly better performance from use == rather than equals().

[Tcat2000]

Ok, I will look into that

[Tcat2000]

Can you provide more details? I don't understand.

[ChiefArug]

Oh whoops, I was reffering to the other shapeMap, here: https://github.com/KubeJS-Mods/KubeJS/pull/902/files#diff-0f8f0bc1e2c3614534f9f872111eca48e7d4e71d68c067e5db4e51506975e32bR102

Making that a Map<BlockState, VoxelShape> then when you initialize it making a new IdentityHashMap<>().

[Tcat2000]

Like, how do I get all of the different blockstates, I played around with the code, but could not find how to do this. Or just pass on this one, how do I get this pr reviewed to add it to the real thing? Is there anything I need to do?

[ChiefArug]

There should be methods on Block for getting blockstates. Like I said, this comment was meant to be on the other shapeMap in the BasicBlockJS class where you do actually have access to those.

As for getting this added to KJS that could happen anytime Lat or Max feels like it, but that likely wont happen while there are pending reviews from other people. Sometimes PRs can stay open for a while with no activity as Lat doesn't touch Github much and he generally deals with feature PRs.

[Tcat2000]

Alright

[ChiefArug]

Map has a getOrElse/getOrDefault method (named something like that) that you can use instead of a null check.
That does mean that if the stored shape is empty then that will result in the block having no box, which I think is okay, but it does lead to it being hard to remove...

If filtering for empty is going to happen it should happen when its added to the map (and probably error instead of being silently ignored), not when its grabbed from the map.

[Tcat2000]

Ya know, I was doing some testing, and i got a crash because it was null, I think the only reason it can be null, is a different issue, that I am working on, but getOrDefault will prevent a crash if something goes wrong.

[ChiefArug]

Number (the superclass of Double, Float ect) has a doubleValue() method that can be used here.

[Tcat2000]

Ok, I was thinking this is a bad way to do it, just was not sure of an alternative.

[Tcat2000]

github does not like you renaming branches with prs open