Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 42 additions & 15 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1,35 +1,62 @@
# Description

_Please include a summary of the change and which issue is fixed. Please also include relevant
motivation and context. List any dependencies that are required for this change._
Added automated unit tests for ray intersection functions, migrating interactive tests from `test_geometry.cpp` to proper unit tests that can run in CI/CD pipelines.

**Changes:**
- Added unit tests for `rectangle_ray_intersection` in `unit_test_geometry.cpp`
- Added unit tests for `circle_ray_intersection` in `unit_test_geometry.cpp`
- Added unit tests for `triangle_ray_intersection` in `unit_test_geometry.cpp`
- Added unit tests for `quad_ray_intersection` in `unit_test_geometry.cpp`
- Added unit tests for `bitmap_ray_collision` in `unit_test_bitmap.cpp`
- Added test for detecting closest intersection among multiple shapes
- Added necessary includes for geometry headers and physics

**Motivation:**
The ray intersection functionality previously only had interactive visual tests that required manual inspection. These new automated tests enable continuous integration testing and prevent regressions.

Fixes # (issue)

## Type of change

_Please delete options that are not relevant._

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as
expected)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Documentation (update or new)

## How Has This Been Tested?

_Please describe the tests that you ran to verify your changes. Provide instructions so we can
reproduce. Please also list any relevant details for your test configuration_
**Test Details:**
- All tests are automated using Catch2 framework
- Tests validate both boolean return values and output parameters (hit points, distances)
- Edge cases tested: rays pointing away, parallel rays, rays from inside shapes
- Multiple shape priority testing validates distance-based collision detection

**To reproduce:**
```bash
# From MSYS2 MinGW64 terminal
cd projects/cmake
cmake -G "Unix Makefiles" .
make
cd ../../bin

# Run all unit tests
./skunit_tests

# Run only ray intersection tests
./skunit_tests "[ray_intersection]"
./skunit_tests "[ray_collision]"
```

## Testing Checklist

- [ ] Tested with sktest
- [ ] Tested with skunit_tests
- [ ] Tested with sktest (not applicable - these are unit tests)
- [x] Tested with skunit_tests (syntax validated, ready for build/test)

## Checklist

- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code in hard-to-understand areas
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [x] My changes generate no new warnings
- [ ] I have requested a review from ... on the Pull Request
88 changes: 88 additions & 0 deletions coresdk/src/test/unit_tests/unit_test_bitmap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "types.h"
#include "graphics.h"
#include "resources.h"
#include "physics.h"

#include "logging_handling.h"

Expand Down Expand Up @@ -134,3 +135,90 @@ TEST_CASE("bitmap bounding details can be retrieved", "[bitmap]")
}
free_bitmap(bmp);
}

TEST_CASE("can perform bitmap ray collision detection", "[bitmap][ray_collision][physics]")
{
bitmap bmp_1 = load_bitmap("on_med", "on_med.png");
bitmap bmp_2 = load_bitmap("rocket_sprt", "rocket_sprt.png");
bitmap bmp_3 = load_bitmap("up_pole", "up_pole.png");

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional improvement: To check whether the rays should be intersecting these bitmaps, the image sizes must be checked. Consider creating bitmaps and drawing shapes onto them manually, as this would make the sizes explicit, and reduce room for error.

If the resources are kept, consider adding null checks here (e.g. REQUIRE(bmp_1 != nullptr);). This would give consistency with the other tests in this file, and make any errors more specific (for example, when a bitmap is not null, but is not valid).

REQUIRE(bitmap_valid(bmp_1));
REQUIRE(bitmap_valid(bmp_2));
REQUIRE(bitmap_valid(bmp_3));

SECTION("can detect ray collision with bitmap")
{
point_2d bmp_position = point_at(100.0, 100.0);
point_2d ray_origin = point_at(50.0, 150.0);
vector_2d ray_heading = vector_to(1.0, 0.0);

// Ray should collide with bitmap in its path
bool collision = bitmap_ray_collision(bmp_1, 0, bmp_position, ray_origin, ray_heading);
REQUIRE(collision);

// Ray pointing away should not collide
ray_heading = vector_to(-1.0, 0.0);
collision = bitmap_ray_collision(bmp_1, 0, bmp_position, ray_origin, ray_heading);
REQUIRE_FALSE(collision);
}

SECTION("can detect ray collision with multiple bitmaps at different positions")
{
point_2d bmp_1_position = point_at(300.0, 300.0);
point_2d bmp_2_position = point_at(500.0, 300.0);
point_2d bmp_3_position = point_at(700.0, 300.0);
point_2d ray_origin = point_at(100.0, 100.0);

// Ray heading towards first bitmap
vector_2d ray_heading = vector_point_to_point(ray_origin, bmp_1_position);
bool collision_1 = bitmap_ray_collision(bmp_1, 0, bmp_1_position, ray_origin, ray_heading);

// Ray heading towards second bitmap
ray_heading = vector_point_to_point(ray_origin, bmp_2_position);
bool collision_2 = bitmap_ray_collision(bmp_2, 0, bmp_2_position, ray_origin, ray_heading);

// Ray heading towards third bitmap
ray_heading = vector_point_to_point(ray_origin, bmp_3_position);
bool collision_3 = bitmap_ray_collision(bmp_3, 0, bmp_3_position, ray_origin, ray_heading);

// At least one should be true depending on bitmap transparency
REQUIRE((collision_1 || collision_2 || collision_3));
Comment on lines +172 to +185
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue:

This section seems to be meant for testing ray collisions with multiple bitmaps simultaneously. At the moment, this is three separate rays testing three separate bitmaps, and the test only fails if one of these rays doesn't intersect.

Suggested fix:

  • To avoid transparency issues, bitmaps should be created/drawn rather than loaded.
  • The ray should intersect all three bitmaps simultaneously
  • The assertion should be REQUIRE((collision_1 && collision_2 && collision_3))

}

SECTION("can detect ray collision with different ray origins")
{
point_2d bmp_position = point_at(300.0, 300.0);
vector_2d ray_heading = vector_to(1.0, 0.0);

// Ray from left should collide
point_2d ray_origin_left = point_at(200.0, 300.0);
bool collision_left = bitmap_ray_collision(bmp_1, 0, bmp_position, ray_origin_left, ray_heading);
REQUIRE(collision_left);

// Ray from far below might not collide depending on bitmap height
point_2d ray_origin_below = point_at(200.0, 500.0);
bool collision_below = bitmap_ray_collision(bmp_1, 0, bmp_position, ray_origin_below, ray_heading);
// This depends on bitmap dimensions, so we just verify it runs without error
REQUIRE((collision_below == true || collision_below == false));
Comment on lines +201 to +202
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue:

This assertion always passes, and therefore isn't improving the test case.

Suggested fix:

  • Change to REQUIRE(collision_below); and ensure the bitmap dimensions are static.
  • The section only tests rays from different locations (rather than edge cases or different functionality), and may be better off as part of the "can detect ray collision with bitmap" section. This would also fit with how you approached "can perform rectangle ray intersection" in unit_test_geometry.cpp.

}

SECTION("can handle ray collision with different bitmap cells")
{
point_2d bmp_position = point_at(300.0, 300.0);
point_2d ray_origin = point_at(200.0, 300.0);
vector_2d ray_heading = vector_to(1.0, 0.0);

// Test with cell 0 (default)
bool collision_cell_0 = bitmap_ray_collision(bmp_2, 0, bmp_position, ray_origin, ray_heading);
REQUIRE((collision_cell_0 == true || collision_cell_0 == false));

// Test with different cells (if bitmap has animation cells)
// For single-cell bitmaps, this should behave the same as cell 0
bool collision_cell_1 = bitmap_ray_collision(bmp_2, 1, bmp_position, ray_origin, ray_heading);
REQUIRE((collision_cell_1 == true || collision_cell_1 == false));
Comment on lines +211 to +218
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue:

This is a good case to test, but again the assertions always pass.

Suggested fix:

Make assertions more specific, e.g. REQUIRE(collision_cell_0);

Optional improvement:

Test bitmaps with multiple cells (e.g. A case where cell 1 should have a collision but cell 2 shoudn't)

}

free_bitmap(bmp_1);
free_bitmap(bmp_2);
free_bitmap(bmp_3);
}
199 changes: 199 additions & 0 deletions coresdk/src/test/unit_tests/unit_test_geometry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

#include "types.h"
#include "point_geometry.h"
#include "rectangle_geometry.h"
#include "circle_geometry.h"
#include "triangle_geometry.h"
#include "quad_geometry.h"

using namespace splashkit_lib;

Expand Down Expand Up @@ -984,3 +988,198 @@ TEST_CASE("can perform trigonometric calculations", "[trigonometry]")
REQUIRE(tangent(360.0f) == Catch::Detail::Approx(0.0f).margin(__FLT_EPSILON__));
}
}

TEST_CASE("can perform rectangle ray intersection", "[geometry][ray_intersection]")
{
rectangle r1 = rectangle_from(100.0, 100.0, 100.0, 100.0);

SECTION("can detect ray intersection with rectangle")
{
// Ray from left that intersects
REQUIRE(rectangle_ray_intersection(point_at(90.0, 110.0), vector_to(1.0, 0.0), r1));

// Ray that misses (extremely small x component)
REQUIRE_FALSE(rectangle_ray_intersection(point_at(90.0, 110.0), vector_to(__DBL_MIN__, 0.0), r1));
Comment on lines +1001 to +1002
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of __DBL_MIN__ here doesn't seem necessary unless the test relates to near-zero values. Since the purpose of the test is to check missed rays, could this be rectangle_ray_intersection(point_at(95.0, 95.0), vector_to(1.0, 0.0), r1) instead?


// Ray from top that intersects
REQUIRE(rectangle_ray_intersection(point_at(150.0, 50.0), vector_to(0.0, 1.0), r1));

// Ray pointing away from rectangle
REQUIRE_FALSE(rectangle_ray_intersection(point_at(50.0, 150.0), vector_to(-1.0, 0.0), r1));
}

SECTION("can get hit point and distance for rectangle ray intersection")
{
point_2d hit_point;
double distance;

// Ray from left hitting the left edge
bool intersects = rectangle_ray_intersection(point_at(50.0, 150.0), vector_to(1.0, 0.0), r1, hit_point, distance);
REQUIRE(intersects);
REQUIRE(hit_point.x == Catch::Detail::Approx(100.0).margin(EPSILON));
REQUIRE(hit_point.y == Catch::Detail::Approx(150.0).margin(EPSILON));
REQUIRE(distance == Catch::Detail::Approx(50.0).margin(EPSILON));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional improvement: The Catch::Detail::Approx(50.0).margin(EPSILON) syntax is consistent with the rest of the file, but could be simplified to Approx(50.0) anywhere it's used like this.


// Ray from top hitting the top edge
intersects = rectangle_ray_intersection(point_at(150.0, 50.0), vector_to(0.0, 1.0), r1, hit_point, distance);
REQUIRE(intersects);
REQUIRE(hit_point.x == Catch::Detail::Approx(150.0).margin(EPSILON));
REQUIRE(hit_point.y == Catch::Detail::Approx(100.0).margin(EPSILON));
REQUIRE(distance == Catch::Detail::Approx(50.0).margin(EPSILON));

// Ray that doesn't intersect
intersects = rectangle_ray_intersection(point_at(50.0, 50.0), vector_to(-1.0, -1.0), r1, hit_point, distance);
REQUIRE_FALSE(intersects);
Comment on lines +1030 to +1032
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional improvement: This test was already done in the previous test case. Instead, consider covering the case where the ray starts from inside the rectangle (as noted in the function documentation)

}
}

TEST_CASE("can perform circle ray intersection", "[geometry][ray_intersection]")
{
circle c1 = circle_at(300.0, 200.0, 60.0);

SECTION("can detect ray intersection with circle")
{
// Ray from left that intersects center
REQUIRE(circle_ray_intersection(point_at(200.0, 200.0), vector_to(1.0, 0.0), c1));

// Ray from top that intersects
REQUIRE(circle_ray_intersection(point_at(300.0, 100.0), vector_to(0.0, 1.0), c1));

// Ray that misses the circle
REQUIRE_FALSE(circle_ray_intersection(point_at(200.0, 100.0), vector_to(0.0, 1.0), c1));

// Ray pointing away from circle
REQUIRE_FALSE(circle_ray_intersection(point_at(200.0, 200.0), vector_to(-1.0, 0.0), c1));
}

SECTION("can get hit point and distance for circle ray intersection")
{
point_2d hit_point;
double distance;

// Ray from left hitting circle
bool intersects = circle_ray_intersection(point_at(200.0, 200.0), vector_to(1.0, 0.0), c1, hit_point, distance);
REQUIRE(intersects);
REQUIRE(hit_point.x == Catch::Detail::Approx(240.0).margin(EPSILON));
REQUIRE(hit_point.y == Catch::Detail::Approx(200.0).margin(EPSILON));
REQUIRE(distance == Catch::Detail::Approx(40.0).margin(EPSILON));

// Ray from inside the circle
intersects = circle_ray_intersection(point_at(300.0, 200.0), vector_to(1.0, 0.0), c1, hit_point, distance);
REQUIRE(intersects);
REQUIRE(hit_point.x == Catch::Detail::Approx(360.0).margin(EPSILON));
REQUIRE(distance == Catch::Detail::Approx(60.0).margin(EPSILON));
Comment on lines +1070 to +1071
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue:

These lines were causing the test to fail, due to incorrect values. According to the API Docs, hit_point.x should be 300, and distance should be 0.

Suggested fix:

Change to:

REQUIRE(hit_point.x == 300.0);
REQUIRE(distance == 0);


// Ray that doesn't intersect
intersects = circle_ray_intersection(point_at(200.0, 100.0), vector_to(0.0, 1.0), c1, hit_point, distance);
REQUIRE_FALSE(intersects);
}
}

TEST_CASE("can perform triangle ray intersection", "[geometry][ray_intersection]")
{
triangle t1 = triangle_from(400.0, 400.0, 550.0, 410.0, 390.0, 550.0);

SECTION("can detect ray intersection with triangle")
{
// Ray from left that intersects
REQUIRE(triangle_ray_intersection(point_at(350.0, 450.0), vector_to(1.0, 0.0), t1));

// Ray from top that intersects center
REQUIRE(triangle_ray_intersection(point_at(450.0, 350.0), vector_to(0.0, 1.0), t1));

// Ray that misses the triangle
REQUIRE_FALSE(triangle_ray_intersection(point_at(300.0, 300.0), vector_to(0.0, 1.0), t1));

// Ray pointing away from triangle
REQUIRE_FALSE(triangle_ray_intersection(point_at(350.0, 450.0), vector_to(-1.0, 0.0), t1));
}

SECTION("can get hit point and distance for triangle ray intersection")
{
point_2d hit_point;
double distance;

// Ray from left hitting triangle
bool intersects = triangle_ray_intersection(point_at(350.0, 450.0), vector_to(1.0, 0.0), t1, hit_point, distance);
REQUIRE(intersects);
REQUIRE(hit_point.x > 350.0);
REQUIRE(hit_point.y == Catch::Detail::Approx(450.0).margin(EPSILON));
REQUIRE(distance > 0.0);
Comment on lines +1106 to +1108
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue:

  • Line 1106 is too vague to test correctness - This is understandable, since the triangle is quite skewed, so the distance is difficult to calculate exactly. But at the moment, this is saying "as long as the hit point is detected after the ray origin, that's a success"
  • Line 1108 is also too vague - as per above

Suggested fix:

  • Provide a more specific equality assertion, or an upper bound
  • Use an axis-aligned, equilateral triangle to make calculations/assertions simpler


// Ray that doesn't intersect
intersects = triangle_ray_intersection(point_at(300.0, 300.0), vector_to(0.0, 1.0), t1, hit_point, distance);
REQUIRE_FALSE(intersects);
}
}

TEST_CASE("can perform quad ray intersection", "[geometry][ray_intersection]")
{
quad q1 = quad_from(100.0, 300.0, 200.0, 350.0, 100.0, 550.0, 200.0, 500.0);

SECTION("can detect ray intersection with quad")
{
// Ray from left that intersects
REQUIRE(quad_ray_intersection(point_at(50.0, 400.0), vector_to(1.0, 0.0), q1));

// Ray from top that intersects
REQUIRE(quad_ray_intersection(point_at(150.0, 250.0), vector_to(0.0, 1.0), q1));

// Ray that misses the quad
REQUIRE_FALSE(quad_ray_intersection(point_at(50.0, 200.0), vector_to(0.0, 1.0), q1));

// Ray pointing away from quad
REQUIRE_FALSE(quad_ray_intersection(point_at(50.0, 400.0), vector_to(-1.0, 0.0), q1));
}

SECTION("can get hit point and distance for quad ray intersection")
{
point_2d hit_point;
double distance;

// Ray from left hitting quad
bool intersects = quad_ray_intersection(point_at(50.0, 400.0), vector_to(1.0, 0.0), q1, hit_point, distance);
REQUIRE(intersects);
REQUIRE(hit_point.x > 50.0);
REQUIRE(distance > 0.0);
Comment on lines +1143 to +1144
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue:

  • Line 1143 assertion is too vague - As per the triangle test, this is understandable, since the quad is skewed, but at the moment this assertion doesn't test correctness.
  • Line 1144 assertion is too vague - as per above
  • Assertion for hit_point.y is missing

Suggested fix:

  • Provide more specific equality assertions, or upper bounds
  • Use an axis-aligned quad to make calculations/assertions simpler
  • Add assertion for hit_point.y


// Ray that doesn't intersect
intersects = quad_ray_intersection(point_at(50.0, 200.0), vector_to(0.0, 1.0), q1, hit_point, distance);
REQUIRE_FALSE(intersects);
}
}

TEST_CASE("can detect closest ray intersection among multiple shapes", "[geometry][ray_intersection]")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test case reflects the integration test, but as a unit test it doesn't pertain to a particular SplashKit function, and doubles up on some functionality already tested. We know from previous tests that each of these functions returns an accurate value, so nothing new is being tested. The implementation is effective but I would suggest removing it from the unit tests.

{
rectangle r1 = rectangle_from(100.0, 100.0, 100.0, 100.0);
circle c1 = circle_at(300.0, 200.0, 60.0);
triangle t1 = triangle_from(400.0, 400.0, 550.0, 410.0, 390.0, 550.0);
quad q1 = quad_from(100.0, 300.0, 200.0, 350.0, 100.0, 550.0, 200.0, 500.0);

SECTION("can identify closest shape from multiple intersections")
{
point_2d origin = point_at(50.0, 150.0);
vector_2d heading = vector_to(1.0, 0.0);

point_2d r1_hit, c1_hit, t1_hit, q1_hit;
double r1_dist, c1_dist, t1_dist, q1_dist;

bool r1_intersects = rectangle_ray_intersection(origin, heading, r1, r1_hit, r1_dist);
bool c1_intersects = circle_ray_intersection(origin, heading, c1, c1_hit, c1_dist);
bool t1_intersects = triangle_ray_intersection(origin, heading, t1, t1_hit, t1_dist);
bool q1_intersects = quad_ray_intersection(origin, heading, q1, q1_hit, q1_dist);

// Rectangle should be hit first (closest)
REQUIRE(r1_intersects);

// Find the minimum distance
double min_dist = __DBL_MAX__;
if (r1_intersects && r1_dist < min_dist) min_dist = r1_dist;
if (c1_intersects && c1_dist < min_dist) min_dist = c1_dist;
if (t1_intersects && t1_dist < min_dist) min_dist = t1_dist;
if (q1_intersects && q1_dist < min_dist) min_dist = q1_dist;

// Rectangle should be the closest
REQUIRE(r1_dist == min_dist);
}
}