Conversation
|
Marking as draft since it can't be stacked properly. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Critical benchmark and ray-intersection regressions, plus unresolved path-contour handling issues, block approval.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (4)
What changed in this PR
Promotes PathComponent into Flame’s core geometry API and adds path-to-polygon conversion, concave ray intersections, and related ecosystem updates.
Changes:
- Adds path sampling, transformations, and
fromPathconstructors. - Extends shape and hitbox APIs with
isSolidsupport. - Updates tests, examples, documentation, reusable fixtures, and benchmarks.
| File | Reviewed change / note |
|---|---|
packages/flame/test/geometry/polygon_ray_intersection_test.dart |
Adds convex, concave, and vertex ray-intersection tests. |
packages/flame/test/geometry/polygon_component_from_path_test.dart |
Tests polygon construction from paths. |
packages/flame/test/geometry/path_component_test.dart |
Tests PathComponent sizing and filtering. |
packages/flame/test/extensions/rect_test.dart |
Tests offset bounds. |
packages/flame/test/extensions/path_test.dart |
Tests path sampling and transformations. |
packages/flame/test/extensions/aabb_test.dart |
Tests vertex AABB conversion. |
packages/flame/test/experimental/geometry/shapes/polygon_test.dart |
Tests experimental polygon path conversion. |
packages/flame/test/collisions/screen_hibox_test.dart |
Tests path hitbox containment. |
packages/flame/lib/src/geometry/shape_intersections.dart |
Updates polygon intersection documentation. |
packages/flame/lib/src/geometry/shape_component.dart |
Adds constructor-level isSolid support. |
packages/flame/lib/src/geometry/rectangle_component.dart |
Forwards isSolid. |
packages/flame/lib/src/geometry/polygon_ray_intersection.dart |
Adds concave ray parity handling. Critical, 1 vote: vertex crossings can be double-counted, regressing corner raycasts. |
packages/flame/lib/src/geometry/polygon_component.dart |
Adds fromPath and isSolid. Moderate, 1 vote: open contours can form unintended triangles or fail. |
packages/flame/lib/src/geometry/path_component.dart |
Adds the standard path component. Moderate: open contours can fail hitbox construction (2 votes); contour ordering uses diagonal length instead of area (2 votes); AABB filtering can remove valid concave contours (1 vote); nested contours are incompletely compared (1 vote). |
packages/flame/lib/src/geometry/circle_component.dart |
Forwards isSolid. |
packages/flame/lib/src/extensions/rect.dart |
Adds offset-bound construction. |
packages/flame/lib/src/extensions/path.dart |
Adds path utilities. Nit, 1 vote: unused import creates an unnecessary barrel cycle. |
packages/flame/lib/src/extensions/offset.dart |
Adds offset-list conversions. |
packages/flame/lib/src/extensions/aabb.dart |
Adds vertex-bound construction. |
packages/flame/lib/src/experimental/geometry/shapes/polygon.dart |
Adds Polygon.fromPath. Moderate, 1 vote: open contours are silently closed or can assert. |
packages/flame/lib/src/collisions/hitboxes/rectangle_hitbox.dart |
Forwards isSolid. |
packages/flame/lib/src/collisions/hitboxes/polygon_hitbox.dart |
Adds path construction support. |
packages/flame/lib/src/collisions/hitboxes/circle_hitbox.dart |
Forwards isSolid. |
packages/flame/lib/geometry.dart |
Exports PathComponent. |
packages/flame/lib/components.dart |
Exports PathComponent. |
packages/flame/benchmark/README.md |
Documents new benchmarks. |
packages/flame/benchmark/ray_intersection_benchmark.dart |
Adds ray-intersection benchmarks. Critical, 1 vote: hitboxes mounted directly under FlameGame fail because a PositionComponent ancestor is required. |
packages/flame/benchmark/path_contour_benchmark.dart |
Adds contour sampling benchmarks. |
packages/flame/benchmark/path_collision_benchmark.dart |
Adds path collision benchmarks. |
packages/flame/benchmark/main.dart |
Registers benchmarks. |
packages/flame_test/test/test_paths_test.dart |
Tests reusable path fixtures. |
packages/flame_test/lib/test_paths.dart |
Exposes reusable test paths. |
packages/flame_test/lib/src/test_paths.dart |
Defines reusable path fixtures. |
examples/pubspec.yaml |
Adds path-related dependencies. |
examples/lib/stories/input/gesture_hitboxes_example.dart |
Demonstrates path gesture hitboxes. Moderate, 1 vote: the path is sampled redundantly by a discarded hitbox and PathComponent. |
examples/lib/stories/experimental/shapes.dart |
Demonstrates path-derived experimental shapes. |
examples/lib/stories/collision_detection/raytrace_example.dart |
Adds path raytrace shapes. |
examples/lib/stories/collision_detection/rays_in_shape_example.dart |
Adds interactive path ray tests. |
examples/lib/stories/collision_detection/raycast_max_distance_example.dart |
Adds path raycast targets. |
examples/lib/stories/collision_detection/raycast_light_example.dart |
Adds path lighting targets. |
examples/lib/stories/collision_detection/raycast_example.dart |
Adds path raycast targets. |
examples/lib/stories/collision_detection/multiple_shapes_example.dart |
Adds collidable path shapes. |
examples/lib/commons/rounded_rect_component.dart |
Adds rounded rectangle rendering. |
examples/lib/commons/paths.dart |
Adds shared path helpers. |
examples/lib/commons/paths_creation_mixin.dart |
Adds reusable path creation behavior. |
doc/flame/components/shape_components.md |
Documents path-based shape components. |
doc/flame/collision_detection.md |
Documents concave polygon hitboxes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
PathComponent now lives in src/geometry and it is exported by components.dart: all affected examples now import it from there.
The path_component_test uses the TestPaths to verify both dimensions match and hitboxes filtering.
From Copilot: path.contours includes open contours, and walkContourAt returns only the endpoints for a line. This unconditionally constructs a PolygonHitbox for every contour, while PolygonComponent requires at least three vertices, so a valid Path with an open contour fails when hitboxes load. Skip or reject non-polygon contours explicitly and document the requirement.
5e0386e to
6055c4f
Compare
| /// The default paint used to render hitboxes. | ||
| static Paint hitboxStroke = Paint() | ||
| ..color = const Color(0xffffffff) | ||
| ..style = .stroke; |
There was a problem hiding this comment.
We should follow the same standard for painting this as the other shape components, like PolygonComponent.
| required Path path, | ||
| this.sampling = 1.0, | ||
| this.tolerance, | ||
| this.hitboxesPriority, |
There was a problem hiding this comment.
This should not live here, the hitboxes should handle their own priority.
| super.key, | ||
| super.paint, | ||
| super.paintLayers, | ||
| bool renderShape = true, |
There was a problem hiding this comment.
This shouldn't be here, since it doesn't exist on PolygonComponent.
| @@ -16,8 +15,11 @@ | |||
| with CollisionCallbacks, CollisionPassthrough { | |||
| PathComponent({ | |||
There was a problem hiding this comment.
Follow PolygonComponent to see which arguments that should exist on here, remember this is no longer an example helper when moved in here, so it needs to be consistent with the rest of the code.
| this.addHitboxes = false, | ||
| this.loadHitboxes = true, | ||
| this.renderHitboxes = false, | ||
| this.filterHitboxes = true, | ||
| this.hitboxesPaint, |
| ..renderShape = renderHitboxes, | ||
| ]; | ||
| // Create a hitbox for each path contour with at least three vertices. | ||
| List<PolygonHitbox> _createHitboxes() { |
There was a problem hiding this comment.
The component should not create hitboxes for itself, we probably need a PathHitbox that can fill this up when added to it.


Description
The
PathComponentclass introduced in PR #4048 was moved fromexamples/lib/commonsintolib/src/geometryin the core package, on suggestion from @spydon; all affected examples have been updated accordingly. There's also documentation and a test forPathComponent.I have added the missing
granularityparameter in the constructor, as well as thehitboxesPriorityparameter. For convenience, I've also added therenderShapeparameter (stilltrueby default).Checklist
docsand added dartdoc comments with///.examplesordocs.Breaking Change?
Related Issues