Minecraftforge: OBJ BlockState Models

[cjm721]

Minecraft Version: 1.14.4

Forge Version: 28.0.74

Steps to Reproduce:

1. Reference OBJ model from block state file
2. Start Game
3. Game attempts to load filename.obj.json

Description of issue:
ModelBakery attempts to load all models from /models/.json which means it cannot load OBJ models.

Can somewhat get around this by manually calling OBJLoader for each model and then overwriting the registry.

[williewillus]

I'm working on a successor to PR #6090 which should fix this (next week probably)

[williewillus]

nvm the above, I've gotten stuck on issue after issue and the whole thing just needs to get blown up and redone

[gigaherz]

Should be fixed as of 28.1.40 / 5e9380a

[ZeroNoRyouki]

Still happening in 28.1.61:

[gigaherz]

Are you using a Forge Blockstates file? Custom model (including OBJ) won't load from vanilla blockstates jsons. I'm working on a new model loading system, so it's not worth patching a ton of vanilla code to make things work a bit closer to how they were in 1.12.

[ZeroNoRyouki]

Yes. I've tested it with a vanilla and a forge blockstate and the result was the same. In both cases ModelBakery don't forward the model loading to forge (objloader.accept is never called) and try to load the model as a vanilla json

Ps: yes, I'm registering my domain with objloader (in the client setup event)

[gigaherz]

Are you calling OBJLoader.INSTANCE.addDomain(MODID); ?

[ZeroNoRyouki]

Yes

[gigaherz]

Well all I can say is,

If you want to check the repository of my Survivalist mod, where I have the code of the WIP 1.14 port as see in the image...

(PS: Ignore the rogue comma in the json file, I removed some unrelated stuff from the file to make the situation clear. it does work without that comma btw)

[ZeroNoRyouki]

uhm I've checked your code but you are using a custom model & loader for your rack block and it's that custom model that, in the end, load the OBJ file by calling ModelLoaderRegistry::getModelOrMissing() that in turn use ObjLoader to load the model. So your OBJ is not, strictly, loaded from the (forge) blockstate.

I think I found where the problem is.

When ModelBakery::getUnbakedModel() is called to process the MRL of the blockstate it use the field_217848_D set to keep track of what it is processing and what still need to be processed: it add the MRL of the blockstate there and loop on every value in that set by calling ModelBakery::loadBlockstate() on each of them:

field_217848_D = {HashSet@16386} size = 1
0 = {ModelResourceLocation@16383} "extremereactors:basic_reactorcasing#"

The problem is that ModelBakery::loadBlockstate() add every model it found referenced in the blockstate to that field_217848_D field (by calling ModelBakery::putModel()), including the .obj model:

field_217848_D = {HashSet@16386} size = 2
0 = {ResourceLocation@18797} "extremereactors:block/reactor/basic/test_cube_100x100x100mm.obj"
1 = {ModelResourceLocation@16383} "extremereactors:basic_reactorcasing#"

So when the first call to loadBlockstate() return, getUnbakedModel() keep looping and call loadBlockstate() again passing in the RL of the obj model. And this is were ModelBakery try to open the .obj.json file and we end up with a FileNotFoundException

[ZeroNoRyouki]

Hi,
I've tested it again using your updated blockstate (in my original blockstate the obj file was referenced by both the "default" and "variants" blocks but that should not change anything in theory):

{
    "forge_marker": 1,
    "defaults": {
        "model":"extremereactors:block/reactor/basic/rack.obj"
    },
    "variants":{
      "":{
         "y":90
      }
   }
}

but the code path and result didn't change:

field_217848_D = {HashSet@16487} size = 2
0 = {ModelResourceLocation@16483} "extremereactors:basic_reactorcasing#"
1 = {ResourceLocation@17085} "extremereactors:block/reactor/basic/rack.obj"

Z

PS: I've checked forge .64 but the new commits do not touch ModelBakery or other relevant code

[gigaherz]

@ZeroNoRyouki When I said I tested it, I meant it. I didn't just lie to you and think that the custom model working means obj work. The reason the file had a typo was that I chose to remove the unused data I had in the file.

You are correct in saying that getUnbakedModel will fail, that's why Forge Blockstates does not use it.

The bug is, interestingly, much simpler and has a silly explanation.

This works:

This does not:

If you can't see the difference, it's that the version that doesn't work, doesn't have "textures" or "custom" -- meaning it's a vanilla-compatible blockstate variant, so it skips the forge processing... which skips models.

This is why it was always working for me, but not for you: you aren't using retexturing or custom data, which means the code path does not trigger forge's extra processing.

This IS a mistake on my part, I made the incorrect assumption that only forge "extended variants" needed model injection.

However I don't know if we want to include every possible variant in the model injection process, it seems... overkill. Maybe it's best if modders just use an empty "textures":{} block? (Empty block doesn't do it, it has to contain at least one entry which makes it less elegant)

[ZeroNoRyouki]

Just to clarify... the test I did last night was performed looking at the code I cloned from your repo and that matched the pic you posted here initially. I noticied that you updated the blockstate pic here only this morning so I rerun the test. That's it. I wasn't implying anything... And yes, I can read :)

Adding the semi-empty texture block fixed it. Since textures are specified in the .mtl file I was assuming this was not required. And like you said, empty blocks are not that elegant.

It's my understanding that every variant could have it's own model so I would say that every variant should be included in the model injection.

Since the texture block is required apparently, I would say that one forge blockstate without it should be marked invalid and not offloaded to vanilla. But if it worked this way since the first release of forge blockstate there may be too many invalid blockstate files around to change things...

[gigaherz]

It is not invalid. The texture block is not meant to be required. It's a mistake (invalid assumption) in the changes I made in #6154. Using a "textures" block or "custom" block is a workaround, not the intended solution.