Body.copy() with multiple shapes attached to a body

[GuillaumeBroggi]

The copy of a body attached to multiple shapes raises an AssertionError: The shape's body must be added to the space before (or at the same time) as the shape. when following the usage shown in the examples.

The only workaround I have found is to get a new body and only copy the shapes (see below).

import pymunk

s = pymunk.Space()
b1 = pymunk.Body(1, 1)
b1.position = 1, 2

shape1 = pymunk.Circle(b1, 4)
shape2 = pymunk.Circle(b1, 6)
s.add(b1, shape1, shape2)

# Work, with a new object
b2 = pymunk.Body()
shape_copies = []
for shape in b1.shapes:
    shape_copies.append(shape.copy())
    shape_copies[-1].body = b2

s.add(*shape_copies, b2)

# Do not work
b3 = b1.copy()
shape_copies = []
for shape in b1.shapes:
    shape_copies.append(shape.copy())

s.add(*shape_copies, b3)

# Do not work
shape_copies = []
for shape in b1.shapes:
    shape_copies.append(shape.copy())

s.add(*shape_copies, shape_copies[0].body)

[viblo]

Oh.. It seems to work properly if the whole space is copied. But not if the shape or body is copied separately..

In the your last case it looks like the issue is that the two shapes copies have different bodies, but only one of them is added to the space which is why it breaks.

I will have to investigate a bit

[GuillaumeBroggi]

In the last case the shapes are attached to the same body, so the I was (maybe naively :) ) expecting that the shape copies would reference the same body object.

Instead, it seems that the copy is done sequentially and each shape is attached to its own copy of the initial body:

print(hex(id(b1)))

shape_copies = []
for shape in b1.shapes:
    shape_copies.append(shape.copy())
    print(hex(id(shape_copies[-1].body)))
    
s.add(*shape_copies, shape_copies[0].body)

0x7fa74d0fdf50 # b1 0x7fa74cde4c90 # first copy of b1 0x7fa74cde5290 # second copy of b1

Thus both body copies should be added to the space, but the shapes will not be part of the same body anymore. A workaround could be to replace the body of the additional shape copies with the one from the first shape copy, as below, but it means that we may be doing a lot of body copying for nothing:

shape_copies = []
for shape in b1.shapes:
    shape_copies.append(shape.copy())

for shape in shape_copies[1:]:
    shape.body = shape_copies[0].body

s.add(*shape_copies, shape_copies[0].body)

[viblo]

Thinking about this some more, Im not sure if this is a bug or feature :)

Copy a space is "easy", or at least the expectation of what should happen is easy (except for maybe callback functions and other special cases). Everything inside should be copied.

But for single bodies or shapes Im not sure:

• A body does not have a direct reference to the shapes to avoid circular dependency. It only has a weakref to it for convenience (its the shape that is attached to a body and not other way around). So if its copied, its natural that only the body is copied without the shapes that are attached to it. The same applies to constraints the body is attached to.
• A shape has a body, when its copied then the body is also copied over.
• But two shapes with the same body.. ? When the first shape is copied over body is also copied. But when second shape is copied, then its very messy if it should attach itself to the first copied shapes' body (since it would mean that some state would be needed to keep track of all copies and originals). And there are other corner cases here, like if you modify the body in between..
• Neither shapes nor bodies that are copied will have the space set on the copy.
• And what about the complex case when there's constraints as well.

I guess the copy could follow all references between bodies, shapes and constraints so if you copy one thing then everything else is included. However, this is quite complicated since creation of the copies has to happen in the right order or it will break..

Given all this Im not sure I would have an API for it at all except to copy space unless it didnt already exist..

[GuillaumeBroggi]

From a user perspective, it would make sense to have a "ready to use" copied object, with all referenced objects.

But I see that it would be very challenging from the API perspective, so I guess it's not an idea to consider :smile:

As an alternative, could it make sense to have an option to only copy the shape (and not the referenced body for instance) so no unnecessary copying happens when working with multiple shapes? Assuming that these body extra copies have an impact on performances.