Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Instancing of a scene does not instance a new ConcavePolygonShape in a collision shape #40887

Open
Tracked by #45333
goatchurchprime opened this issue Jul 30, 2020 · 5 comments

Comments

@goatchurchprime
Copy link

Godot version:
3.2.2.stable

OS/device including version:
ubuntu 18.04

Issue description:
If you use a scene to procedurally generate static bodies using the SurfaceTool and the shape.set_faces() function, then it comes as a surprise to learn that each time you instance a new one it over-writes the triangles that are assigned to the concave collision shape

This is a very difficult bug to discover, particularly when your procedural shapes are reasonably similar and there's something about the physics that doesn't look right.

The work-around is easy. Put $CollisionShape.shape = ConcavePolygonShape.new() in the _ready() function of the static body. The current behavior of not applying the deep-copy to this Collision object when the scene is instanced is not functionally useful. This might be a special case, however. Maybe the other pre-configured shapes (box, cylinder) are sufficiently constrained that they can be shared, because it is not their purpose to have procedural geometry inserted into them.

Animated gif shows two procedurally generated ramp static bodies with cubes rolling down them, where the collision shape of the steeper ramp has overwritten the collision shape of the shallow ramp.
out

Steps to reproduce:
See below.

Minimal reproduction project:
collisionclonebug.zip

@hoontee
Copy link
Contributor

hoontee commented Jul 31, 2020

The Shape is a shared resource. Modifying it directly will apply the changes to all nodes that reference it.

You must use $CollisionShape.shape = $CollisionShape.shape.duplicate() or $CollisionShape.shape = ConcavePolygonShape.new().

Set resource_local_to_scene to true.

@goatchurchprime
Copy link
Author

The problem is that the ConvexPolygonShape is not like a normal resource. It is used as a container that always starts out as empty. The shape only gets created by feeding it a list of triangles. https://docs.godotengine.org/en/stable/classes/class_concavepolygonshape.html

When you instance a scene, all the container nodes are cloned, so when I add new elements to a subnode in one instance those elements do not appear in the subnode of another instance. The same principle should apply to this node.

With the current implementation, anyone who does not write $CollisionShape.shape = ConcavePolygonShape.new() for the second instantiation of a scene containing one of these "resources" has created a bug in their code that is very difficult to spot. It would be better if every instantiation of ConcavePolygonShape from a scene returned an error if used or was null so that people knew that they always had to write $CollisionShape.shape = ConcavePolygonShape.new().

@hoontee
Copy link
Contributor

hoontee commented Aug 3, 2020

I'm sorry, I actually gave you bad information due to me being blatantly unaware of resource_local_to_scene's purpose...
Please confirm that setting resource_local_to_scene to true fixes your issue:
image

@goatchurchprime
Copy link
Author

Yes, that works perfectly.

Can that flag definitely be set to true by default? It's proven to be difficult for both of us to find, and I can't think of a use-case for to be set to false for these procedurally generated objects.

@Zireael07
Copy link
Contributor

Shapes are shared by default for performance reasons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants