Scenes with 3D MultiMeshes print errors on load

[0x0ACB]

Tested versions

4.3

System information

Windows 11

Issue description

https://github.com/godotengine/godot/blob/ee363af0ed65f6ca9a906f6065d0bd9c19dcf9e4/scene/resources/multimesh.cpp#L309 prints an error if the instance_count is set before set_transform_format. This will be true for any non empty 3D MultiMesh stored in a scene file.

Steps to reproduce

Add a 3D MultiMesh with some instances to a scene. Save scene. Reopen scene. <scene\resources\multimesh.cpp(309): MultiMesh::set_transform_format> Condition "instance_count > 0" is true.

Minimal reproduction project (MRP)

[tetrapod00]

Setting the format before the instance count is required, see here.

But this may still be a bug - should this be a case where a C++ error is surfaced?

Maybe transform_format's description should also have a note about the order it needs to be called, like instance_count does.

Edit: Sorry, I misread the issue!

[0x0ACB]

Setting the format before the instance count is required, see here.

But this may still be a bug - should this be a case where a C++ error is surfaced?

Maybe transform_format's description should also have a note about the order it needs to be called, like instance_count does.

Edit: Sorry, I misread the issue!

Means this is a more fundamental issue. There can be no hard initialization order requirements for properties since the initialization order during load is practically undefined. Either there needs to be a way to define property dependencies for the loader or properties need to be designed in a way that initialization order does not matter.

[clayjohn]

I can't reproduce this issue in a new scene by following the reproduction steps.

Can you upload an MRP that consistently reproduces the issue?

[0x0ACB]

Sure here you go. This one actually breaks completely: multimesh.zip

I just created the MultiMesh via the scatter populate tool

[clayjohn]

Thanks for that. Looking at your MRP I can see that instance_count is stored as the first property while in mine it is stored under transform_format.

So we need to figure out what would change the order of properties when saving

[0x0ACB]

I would say its best to not rely on the order at all or implement a way to enforce the ordering in the class definition. Relying on the ResourceLoader/Saver to just randomly save and load in the correct order does not seem like a robust design.

[aspectwaltz]

I noticed this as well on v4.3.stable.mono.official.77dcf97d8

when in a tool script where I instantiate a MultiMesh with new() and create the mesh with surface_tool's commit(). When I reopen a scene the following errors appear, one for each of the settings on the multimesh and for each multimesh in the scene.

ERROR: Condition "instance_count > 0" is true.
   at: set_transform_format (scene/resources/multimesh.cpp:309)
ERROR: Condition "instance_count > 0" is true.
   at: set_use_colors (scene/resources/multimesh.cpp:291)
ERROR: Condition "instance_count > 0" is true.
   at: set_use_custom_data (scene/resources/multimesh.cpp:300)

However, I was unable to reproduce this when I isolate scripts into in an MRP and it doesn't appear to happen always... Is this issue related? 87166

I should also note that these errors are only thrown in editor, but not during run project or release

[CreatedBySeb]

Still present on v4.5.dev.custom_build [209a446e3], marking as confirmed and requiring discussion since the solution is not clear. It does seem like #87166 mentioned by aspectwaltz is a similar issue. This issue has more actual discussion about the issue which makes me inclined to keep this one open instead. I've flagged this issue for Core triage since it seems like this may warrant a deeper solution based on the discussion so far.

There is a linked PR (#87205) in that issue that is pending review, but it tackles this specific case rather than the general problem. Is that still worth looking into?