RandomMcSomethin / fallingleaves

MIT License
43 stars 20 forks source link

Multi-Part Leaf Models #21

Closed GrandPappyJay closed 3 years ago

GrandPappyJay commented 3 years ago

I have created a multi-part leaf model (Doku inspired) with no tint to use in the decaying blockstate "persistant:false, distance:7"... The mod only drops gray leaves in this instance... When I combine the same models into one then the mod will drop colored leaves correctly... Are multi-part models supported?

Fourmisain commented 3 years ago

I'm not familiar with multi-part models at all, can you link the broken and working versions you were talking about so that I can reproduce/debug the issue?

The part of code which is responsible for extracting the "true" color from the leaf block is this:

https://github.com/RandomMcSomethin/fallingleaves/blob/bfd825983b1a17058c8e9ff052b7e0d58ff41799/src/main/java/randommcsomethin/fallingleaves/util/LeafUtil.java#L47-L56

So it reads the block model for the given state, checks if a down quad has a tint and if so, it'll mix the texture color with the block color, otherwise it'll just use the texture color (which in this case is gray edit: or is it?).

Most likely the issue is the down quad part, for some reason.

The confusing part is that there's only 1 model - which makes sense, since this is already baked, but I'm not sure exactly why this wouldn't work with multi-part models.

GrandPappyJay commented 3 years ago

Unzip the attached file... There are two blockstate files included for "acacia_leaves", one working and one not working... Rename one or the other to make the pack work, currently formatted for 1.17... I tried to label everything as best I could to help you path to and from...

In the not working version I am using the same multi-part model arrangement for both the tinted and not-tinted models... the bug only seems to effect the not-tinted models... tinted models work just fine... even in the multi-part model setup...

In the working version I only included the not-tinted combined model... the model is just the individual model pieces joined into one model... the bug is not present with the model like this...

*Edit - Sorry I left some unnecessary model files... Just clutter, should be no effect... Ignore all of the "1a", "1b", "2a", "2b" model files... They aren't used...

Faling_Leaves_Testing.zip

Fourmisain commented 3 years ago

Okay, I think I figured out the issue and some possible solutions.

The BakedModel that is returned is a MultipartBakedModel, which has a (protected) components field which basically maps BlockStates to BakedModels, just like how it is defined with "when" and "apply".

Now, model.getSprite() does not return the component corresponding to the current block state but it simply returns the very first component, which in this case uses the "block/caliburn/leaves/acacia/leaves_base" model and thus the vanilla acacia leaves texture, which is gray. That (and the fact that the block is not tinted) is why the falling leaves are gray.

This should be fixable by walking through the components, finding one corresponding to the current block state and using its texture. model.getQuads(state, ...) actually already does this, so one way to fix the issue should be to copy this behavior.

While trying to find ways to access multipart stuff I found a much more direct way to access the Sprite's texture (it has a protected images field), which is nice.

More interestingly, I found that BakedQuads have a getSprite() too and since that is already filtered by state I just tried using that. It chose output, which is acacia_leaves_bottom_long.png. It's not an ideal choice (I really want it to pick the bottom quad from the actual block if possible) but it does have the correct color!

After some further testing, model.getQuads(state, Direction.DOWN, random) doesn't seem to return the bottom quad from the block at all which is both concerning and confusing! Not only would it be useful for extracting textures but it's already used for tint, so I'll have to figure out what's going on there...

Fourmisain commented 3 years ago

I've (kinda) figured out why model.getQuads(state, Direction.DOWN, random) wouldn't get the down face, it's because your leaves_base.json (in master_no_tint but also in master_tint) has this:

            "faces": {
                "north": {"uv": [0, 0, 16, 16], "texture": "#leaves", "cullface": "north"},
                "east": {"uv": [0, 0, 16, 16], "texture": "#leaves", "cullface": "east"},
                "south": {"uv": [0, 0, 16, 16], "texture": "#leaves", "cullface": "south"},
                "west": {"uv": [0, 0, 16, 16], "texture": "#leaves", "cullface": "west"},
                "down": {"uv": [0, 0, 16, 16], "texture": "#leaves", "cullface": "up"}
            }

The down face should have "cullface": "down" instead of "up". Right now, if you place an opaque block on top of the leaves, the bottom face will disappear: cullface (I've disabled the other parts in this screenshot)

And for some reason I don't understand, when the downface has "cullface": "up", it can only be found with model.getQuads(..., Direction.UP, ...) as well - that might be a MC bug.

The good news is that after fixing the json file, I can directly read the texture from the first bottom quad, which (in this case at least) is indeed the block bottom quad and the issue should be fixed - just need to clean up the code.

Fourmisain commented 3 years ago

Just released 1.7.2 on CurseForge, let me know if it's working for you. (Just make sure to fix the json files first.)

GrandPappyJay commented 3 years ago

Great! As soon as CurseForge will let me download it I'll check it out... Thank you so much for your help!

Fourmisain commented 3 years ago

Ah, right, approval might take a while, you can get it from Modrinth as well!

GrandPappyJay commented 3 years ago

Works perfectly for me! Thanks again...