jitterphysics2 icon indicating copy to clipboard operation
jitterphysics2 copied to clipboard

Implemented covariant Clone() method for all supported shapes

Open thygrrr opened this issue 2 months ago • 2 comments

Shape implementors may choose to implement Jitter2.Collision.Shapes.ICloneableShape, which requires a public Shape Clone() method. However -preferably-, they choose to implement Jitter2.Collision.Shapes.ICloneableShape<T>, where T is the implementors type, to benefit from covariance).

Both choices are optional.

I implemented this method for all builtin Shapes in the library; adapting existing Clone() methods where they were already present. I implemented Covariant returns (i.e. returning the descendant type) for all of them.

Covariant returns allow intuitive uses such as:

var box1 = new BoxShape(5, 6);
var box2 = box1.Clone(); //Is a BoxShape, and can be accessed as such, without prior casts.
Shape shape1 = box1.Clone(); //Equally legal.

I created some basic unit tests for the functionality.

CAVEATS:

Some tests couldn't be written in a stable way because the fields were not visible (private). Suggest making them visible to the Test Assembly, possibly setting their visibliity to private protected or internal if desired.

If desired, I can write additional tests ensure functionality is maintained, especially the more complex shapes, such as TriangleShape and ConvexHullShape, the latter already having had a Clone() method that was kept basically intact, may benefit from some test coverage. The primitive shapes are cloned in a very simple way.

Tests in this PR might break if IEquatable<Shape> or is planned to be implemented for any of the shapes; if that's the case I can write some specific reference equality code.

thygrrr avatar May 06 '24 15:05 thygrrr