Zylann / godot_heightmap_plugin

HeightMap terrain for Godot implemented in GDScript
Other
1.74k stars 159 forks source link

Grass patches disappearing when painting #106

Closed Zylann closed 4 years ago

Zylann commented 5 years ago

Godot 3.2 alpha1

While updating the demo project, I noticed detail layers have a tendancy to disappear while painting, with an internal Godot error printed in the console (which I forgot to pick up). Something about multimesh count not matching or something like that.

When trying again I could not find a way to reproduce it yet, so I'm writing this issue to remember that there might be a bug here.

The terrain looks fine otherwise when playing the game, and the detail map is properly saved.

Robbepop commented 4 years ago

I experience this bug frequently but without an actual error appearing. The solution is to paint another brush and save which makes all grass reappear, then you can simply undo the last brush placed. It is a dirty solution but somehow works for now. Hope this info was helpful.

Zylann commented 4 years ago

@Robbepop did you find a way to reproduce it reliably?

Robbepop commented 4 years ago

@Robbepop did you find a way to reproduce it reliably?

Sadly, no.

Zylann commented 4 years ago

According to @Wallace99, this error can happen:

servers/visual/visual_server_scene.cpp:726 - Condition "!is_geometry_instance(instance->base_type)" is true.

By the looks of it, this happens when calling VisualServer.instance_set_custom_aabb, and the error means that we are trying to use a RID which is not a GeometryInstance. MultiMeshInstance is one, so there are two possibilities:

I seem to have got a repro for the error: 1) Paint grass 2) Back away from the terrain until you can's see grass anymore 3) Change density in the inspector 4) Go back close to the terrain: error spams and chunks don't show up

ERROR: instance_set_custom_aabb: Condition "!is_geometry_instance(instance->base_type)" is true.
   At: servers/visual/visual_server_scene.cpp:726
Zylann commented 4 years ago

I pushed a fix in 2c2a73ecad6b963e31432b766f6a68c8d82c1e6f, let me know if it works.

What happened is that MultiMeshInstances I create keep weak references to their MultiMesh (which is shared within one HTerrainDetailLayer). I also pool them when they are not visible. However, changing density was replacing the old MultiMesh with a new one, which had the effect of freeing the old one (because its reference count went down to zero). That means all the MultiMeshInstances in the pool also lost their MultiMesh, so VisualServer didn't consider them anymore like geometry instances. When they get used again when coming closer, errors would then start happening. The fix was to not replace the MultiMesh, but modify it.