Godot: 'Show Collision Shapes' does not update when CollisionShapes are resized via Animation

Created on 16 Mar 2019  路  6Comments  路  Source: godotengine/godot

Godot version: 3.1 stable

OS/device including version: Windows 10 64-bit 1809

Issue description:

After I checked 'Visible Collision Shapes' in the Debug menu and ran a scene where CollisionShapes are being resized by an Animation, the Visible CollisionShape did not update its size even though the CollisionShape itself had been resized by the animation.

The expected behaviour is that the Visible CollisionShape's size updates when the CollisionShape is resized.

Steps to reproduce:

  1. Download reproduction project
  2. Open 'Explosion.tscn'
  3. There is an AnimationPlayer that resizes the Sphere Collider (StaticBody/CollisionShape)
  4. Check 'Visible Collision Shapes' in the Debug menu
  5. Run scene
  6. Observe that Visible CollisionShape does not update its size even though the CollisionShape's radius has been increased (which can be seen as the StaticBody flung the Broken Cube apart, thus the CollisionShape's radius increased

Minimal reproduction project:

upload.zip

bug junior job core

Most helpful comment

Should be pretty easy to fix.
There is the CollisionShape::_create_debug_shape() method:
https://github.com/godotengine/godot/blob/3f631c2567429f00f98fa15a4d49116789dcf4a6/scene/3d/collision_shape.cpp#L202
.. but it is called only in `NOTIFICATION_ENTER_TREE:
https://github.com/godotengine/godot/blob/3f631c2567429f00f98fa15a4d49116789dcf4a6/scene/3d/collision_shape.cpp#L89-L95

What needs to be done is connecting to the shape's "changed" signal, similar to CollisionShape2D:
https://github.com/godotengine/godot/blob/3f631c2567429f00f98fa15a4d49116789dcf4a6/scene/2d/collision_shape_2d.cpp#L43-L46
https://github.com/godotengine/godot/blob/3f631c2567429f00f98fa15a4d49116789dcf4a6/scene/2d/collision_shape_2d.cpp#L166-L167

Things of note:

  • Do not update the shape immediately. Instead, use call_deferred to actually call the function and set a boolean to avoid calling it twice.
  • Make sure to disconnect from the "changed" signal of the old shape :smiley:.

All 6 comments

Should be pretty easy to fix.
There is the CollisionShape::_create_debug_shape() method:
https://github.com/godotengine/godot/blob/3f631c2567429f00f98fa15a4d49116789dcf4a6/scene/3d/collision_shape.cpp#L202
.. but it is called only in `NOTIFICATION_ENTER_TREE:
https://github.com/godotengine/godot/blob/3f631c2567429f00f98fa15a4d49116789dcf4a6/scene/3d/collision_shape.cpp#L89-L95

What needs to be done is connecting to the shape's "changed" signal, similar to CollisionShape2D:
https://github.com/godotengine/godot/blob/3f631c2567429f00f98fa15a4d49116789dcf4a6/scene/2d/collision_shape_2d.cpp#L43-L46
https://github.com/godotengine/godot/blob/3f631c2567429f00f98fa15a4d49116789dcf4a6/scene/2d/collision_shape_2d.cpp#L166-L167

Things of note:

  • Do not update the shape immediately. Instead, use call_deferred to actually call the function and set a boolean to avoid calling it twice.
  • Make sure to disconnect from the "changed" signal of the old shape :smiley:.

I tried to do the code, but I found that: 1, the shape class inherit from Shape did't emit change signal but the classes inherit from Shape2D do, 2 It seems that changed signal should be trigger when the shape data changed, changing the scale of it didn't trigger it in fact

The scale should be already handled by adding the debug shape as a child of the collision shape.
The emission of the changed signal should be doable by just adding the line to each of the set_ methods in the Shape classes.

Should we add it at every class's _set_update method, just like Shape2d ? why don't we just write it in parent class and inherit it ?

https://github.com/godotengine/godot/blob/df7d3708c5b535c3696943322a14ec19a175e30c/scene/resources/shape.cpp#L62-L69
there are mesh cache in get_debug_cache too, should I
1 . move _set_update to Shape parent class , so I can clear cache easily when mesh changes

  1. just remove debug_mesh cache

The debug_mesh_cache is fine, it should stay. Without it, multiple CollisionShape-s reusing the same Shape will have to each recalculate its mesh, which is far from ideal.

Was this page helpful?
0 / 5 - 0 ratings