A huge change of `pyglet.shapes`

[zhengxyz123]

In this PR, I made the following changes:

1. Create pyglet.shapes2d
2. Add a new Catenary shape
3. Move pyglet.shapes to pyglet.shapes2d.drawable
4. Add pyglet.shapes2d.collidable, a module focus on 2D collision detection
5. Add an example(collision.py)
6. Some other changes

Besides, type hints are added to pyglet.shapes2d, and #857 has done the same thing.

In addition, I tried to use sphinx-recommended way to write the comments in pyglet.shapes2d:

:param int foo: bar

[Ross-TheBoss]

Here are my thoughts:

There is quite a lot of duplicated code as a result of this design.

• The ShapeBase has all the attributes/functions of the CollisionShapeBase, and some additional attributes/functions.
• The CollisionCircle, CollisionEllipse, CollisionRectangle and CollisionPolygon have the same properties as their shape counterparts.

I imagine those using the pyglet.shapes, may want to add collision detections to a drawable shape, and this would require having both a drawable shape and collidable shape. You would have to make sure any changes you make to the positioning/rotation of the drawn shape is also performed on the collidable shape.

Additionally, not all shapes which once had collision detection (by doing point in Shape), have a collision detection class (e.g., Sector, Line, BorderedRectangle, Triangle and Star).

I suggest instead of having colliable shapes classes, you make them functions. For example, you can have a point_in_circle(x, y, radius, point, rotation=0, anchor_x=0, anchor_y=0) function, a point_in_ellipse function and a point_in_rectangle function. These functions can be used by the drawable shape classes (via the contains method), but also by the user in case they want to perform collision detection, without having to draw a shape.

[zhengxyz123]

@Ross-TheBoss collision detection isn't mean point in shape entirely. They should also provide something like:

• Is part of a circle in a rectangle?
• Is part of a rectangle in a combination of circles?
• etc.

which are very difficult to implement and will be implemented later.

Only considering circle, ellipse and rectangle, we will have 6 different ways to test whether they are collided or not.

We also should confirm that a in b and b in a return a same value.

So in the above condition, using function isn't a wise decision.

A CollisionShape is very useful when it comes across a Sprite(not a Shape).

---

Additionally, not all shapes which once had collision detection.

• Sector: I imagined that it would be very complicated to realise collisions between shapes for it afterwards and pretended to claim that I had forgotten.
• Line: At the time, I was writing the code for point in Line as a different shape than a Rectangle to get better efficiency, but a Line is really a Rectangle.
• BorderedRectangle: Just Rectangle.
• Triangle is a polygon.
• Star: too difficult.

[pushfoo]

I'll try to post full comments on the code later today.

Re: Ross-TheBoss

There is quite a lot of duplicated code as a result of this design.

To some degree, this is unavoidable for any performance-optimized Python code. Inlining helps avoid the high cost of function calls and . lookups in Python.

The CollisionCircle, CollisionEllipse, CollisionRectangle and CollisionPolygon have the same properties as their shape counterparts.

This could be a good choice, see my next response:

You would have to make sure any changes you make to the positioning/rotation of the drawn shape is also performed on the collidable shape.

For the already-existing shapes or anything that's vertex-based, this is actually simple: we don't have to update anything because we can delegate vertex_list.position and properties to an underlying shape object. SAT should work with this for points.

Re: PR / zhengxyz123's comment

Some of this PR is very good, but may need to be split up like #/927 was. I'll try to post more comments on that in the next few hours. In the meantime, here's a recap of what I posted on Discord:

The fundamental question: Is it worth it to implement multiple collider types if we can get away with using polygons?

Remember:

1. pyglet leans toward being an OpenGL, input, and media abstraction layer rather than a full game engine
2. Physics engines and other binary-backed modules will probably do a better and faster job at handling shape collisions

which are very difficult to implement and will be implemented later. Sector: I imagined that it would be very complicated to realise collisions between shapes for it afterwards and pretended to claim that I had forgotten.

Composable colliders could address this:

# This is like any(), but for colliders
sector_collider = CompositeCollider(
  PolygonCollider(*triangle_params),
  CircleCollider(*circle_params)
)

However, this raises other questions:

1. Should we sort to place smallest first?
2. How would we define smallest? area? width? height?
3. How do we handle edge cases?
   1. Huge width / height, tiny area
   2. How do we handle sorting if a reference to an internal object is could be kept elsewhere and used to change it?

The easiest answer is to make the implementation limited and only support polygons.

[zhengxyz123]

@pushfoo I don't entirely argee with you. The most important reason is that using polygons is too inefficient.

Here is my solution: We(pyglet) just implement collision between polygons, but leave users a way to allow them to access more efficient algorithms(the CollisionShapeBase.is_collide method).

[shenjackyuanjie]

here is what I think.

• Is good to have ways to detect collision

• Also some simple shape collision is also verry useful and powerful in some UI usage.

• Too complex collision detect functionality will make the code too complex and some times with low performance.

So in a actual situation, if I need some simple UI collision detect(btw, why would you need that?), I do will use this code But if there is some really complex shape or too many of them, I will chose to use some physic engine to deal with it, such as this (in rapier)

[zhengxyz123]

Well, there are some situations and solutions:

1. Alex just wants to render something, he do not need collision system.
2. Ben wants to know whether a point is inside a shape, he could use (x, y) in Circle, which is provided by CollisionCircle.
3. Carel is writing a pong in pure pyglet for demonstration purpose, he could use the collision system and add some action when ball is collided with bats and walls.
4. Dan adds a lot of CollisionRectangles and wants collide them with each other, he can use a CollisionShapeManager(which should be implemented by himself for specific application).
5. shenjackyuanjie wants to write a game with so many collision objects and with high performance, a physical engine like pymunk could be used. Or he could use a game engine that is more tightly integrated with the physics engine, which would be easier.

So, for situation (3) and (4), a built-in collision system is useful.

And for situation (5), create inheritances of Collision* which work with physical engine well is a good idea. It's same as you want a BorderedRectangle with different color on border and inner content.

If you read DESIGN for how pyglet should be, a collision system was suggested about two decades ago. But if this collision system needs to be connected to a physics engine in order to work, instead of providing some simple function, a part of the users will not accept it. And those who want to use it in a more complex and efficient application through a physics engine, we can also make an example to show them how they should start.

In summary, there is nothing wrong with adding a collision system, and one can choose to use it directly or rewrite it for better performance.

[Ross-TheBoss]

I now understand why the drawable and collision shapes should be seperate. But now I'm wandering if they are so seperate that the collision detection shouldn't be included within the shapes2d module. Collision detection for a shape is not neccessarily exclusively used by the drawn shape. For example, the CollisionRectangle could used with UI elements (such as buttons) and Sprites or even different shapes as an approximation for efficiency.

In the design, it is suggested that collision detection is part of the mathematical euclid module. I suggest having the modules:

• collision.py for efficient collision detection between simple 2d (or even 3d) shapes.
• shapes.py for drawing 2d shapes, this could use the collision module to add some collision detection convience methods.
• model.py for drawing 3d models/shapes

[zhengxyz123]

It's also interesting that you think differently than I do.

But essentially, they are both 2D shapes, aren't they? It's just that one(specialise in drawing) is visible and the other one(specialise in collision detection) is not.

You should also be aware that DESIGN was written about two decades ago, and the current module structure is different from what it describes.

Differ from you, I suggest having the following modules:

• shapes2d
• shapes3d
  • drawable.py: cubes, balls, etc.
  • collide.py: for 3D collision.
• model.py: for drawing 3D models(it is not a shape).

3D is entirely different from 2D, especially in rendering and collision detecting(it's more complex than the 2D one). We should use a unique way to treat it, or your collision.py will be confusing.

This PR will be merged in the development branch for future feature. So we can discuss a suitable structure.

[pushfoo]

I'm still not sure of the current design for TypeA x TypeB specialized collision. I'm going to keep thinking about that in the coming days.

But now I'm wandering if they are so seperate that the collision detection shouldn't be included within the shapes2d module.

I could go either way on this. With the changes proposed in #939, putting them elsewhere could make sense since anything with a vertices property returning Sequence[Point2D] a can collide. This would not only be collider objects, but also sprites, drawable shapes, and custom classes.

It also allows doing other neat things. For example, you could do the following in a prototype without needing shaders:

1. Use a 3D shape or model as a player character, puzzle element, or both
2. Project its outline as 2D group of vertices
3. Perform 2D collision against sprites or other 2D shapes

It wouldn't be performant, but it would let you validate gameplay ideas quickly.

[zhengxyz123]

@pushfoo Thanks for your advice.

First, I agree with spliting the changes to customtypes.py into a new PR, but I'm not familiar with this. I need your help.

The algorithm implemented in poly_decomp.py will not be used unless a concave polygon is encountered, which is very rare. It's also very complex and I don't know how to optimise it.

I found that this algorithm just convert javascript to python without using python features.

Here is a neat way to build collidable shapes from drawable shapes:

CollisionPolygon.from_shape(...)
CollisionCircle.from_sprite(..., radius, anchor_position, rotation)

If we want to update data when the sprite changes it's position, more code should be added.

[zhengxyz123]

To save you the trouble of converting from rst format to the project's preferred Google style.

It seems that the rest of pyglet uses a different format and the docconvert wasn't work on some methods like Arc.__init__, BezierCurve.__init__ and Catenary.__init__.

The above was solved by removing _draw_mode=....

[zhengxyz123]

Conclusion before revision, polygonSlice wasn't used in the polygonQuickDecomp, which is used by pyglet to convert concave polygons to several convex polygons.

[zhengxyz123]

@pushfoo I have changed a lot of things based on your review.

But I'm confusing on this.

[pushfoo]

Thank you! I'll try to take a closer look at the changes in the next 2 days.

But I'm confusing on this.

The idea is to eliminate the calls to hasattr by replacing this line with a try::

        if hasattr(self, "get_polygon") and hasattr(other, "get_polygon"):

An AttributeError will be raised if either method is missing anyway, so there's no point in checking if they're there beforehand. Catching the AttributeError achieves the same thing.

The from e at the end attaches the original AttributeError info. This isn't strictly necessary, but it helps preserve the exact point the issue occurred at while still presenting the type you selected as more meaningful.

To my understanding, the speed boost from eliminating function calls is greatest for versions earlier than 3.11. The current design of this makes it harder to measure the impact of removing the function calls due to the GL context involved, but the old versions still have years of support left ahead of them. It's probably worth the odd looking syntax here.