Skip to content

feat(engine): Add Spring Constraint #1572

Open
JoaoTRoxo wants to merge 9 commits intomainfrom
1237-implement-spring-constraint
Open

feat(engine): Add Spring Constraint #1572
JoaoTRoxo wants to merge 9 commits intomainfrom
1237-implement-spring-constraint

Conversation

@JoaoTRoxo
Copy link
Contributor

@JoaoTRoxo JoaoTRoxo commented Sep 3, 2025

Description

This pull request adds a new SpringConstraint system to the physics solver, allowing entities to be connected with spring-like behavior.

Checklist

  • Self-review changes.
  • Evaluate impact on the documentation.
  • Ensure test coverage.
  • Write new samples.
  • Add entry to the changelog's unreleased section.

@JoaoTRoxo JoaoTRoxo linked an issue Sep 3, 2025 that may be closed by this pull request
@github-actions github-actions bot added A-Engine B-Physics D-Good-First-Issue Easy but interesting P-Backlog This issue isn't a priority at all labels Sep 4, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2025

PR Preview Action v1.6.2

🚀 View preview at
https://GameDevTecnico.github.io/cubos/preview/pr-1572/

Built to branch gh-pages at 2025-09-26 18:55 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@codecov
Copy link

codecov bot commented Sep 4, 2025

Codecov Report

❌ Patch coverage is 0% with 49 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.66%. Comparing base (e2526be) to head (2a6839e).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...ne/src/physics/solver/spring_constraint/plugin.cpp 0.00% 40 Missing ⚠️
...gine/src/physics/constraints/spring_constraint.cpp 0.00% 8 Missing ⚠️
engine/src/physics/solver/plugin.cpp 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1572      +/-   ##
==========================================
- Coverage   51.72%   51.66%   -0.06%     
==========================================
  Files         499      503       +4     
  Lines       27241    27316      +75     
  Branches     2549     2551       +2     
==========================================
+ Hits        14091    14114      +23     
- Misses      13150    13202      +52     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/2)

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (2/2)

@JoaoTRoxo JoaoTRoxo force-pushed the 1237-implement-spring-constraint branch from 201e24b to 5860812 Compare September 5, 2025 16:32
@cubos-bot
Copy link

cubos-bot bot commented Sep 23, 2025

@RiscadoA will only be able to review this PR in 5 days.

@RiscadoA RiscadoA removed their request for review September 23, 2025 21:12
Copy link
Contributor

@fallenatlas fallenatlas left a comment

Choose a reason for hiding this comment

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

Thank you for working on this 🙏 The solver itself still needs some work to be consistent with the rest of the physics, I left a comment on this. The sample looks pretty cool. Good work.

{
/// @brief Relation which holds the information for a spring constraint between two bodies.
/// @ingroup physics-solver-plugin
struct SpringConstraint
Copy link
Contributor

Choose a reason for hiding this comment

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

it needs CUBOS_ENGINE_API after struct so that it is accessible in the dll for us to use in games

Copy link
Contributor

Choose a reason for hiding this comment

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

You did a sample just for the spring constraint so I think you can remove the changes you did to this file (if they are different move it to the other sample)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what this file is but is it supposed to have been commited?

Copy link
Contributor

Choose a reason for hiding this comment

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

same here

Copy link
Contributor

Choose a reason for hiding this comment

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

This solver is correct but it is using a different method than what we have for our solver overall. We use a soft constraint solver, TGS Soft as it was called at the time, this solver treats every constraint as a "spring" of sorts because it isn't stiff and allows a constraint to not be fully solved at every substep which greatly helps with stability of the simulation. There is a good resource where Catto explains soft constraints softly: https://box2d.org/files/ErinCatto_SoftConstraints_GDC2011.pdf if you want to know more about it (he also has many cool others).

An example of how this works is here: https://github.com/erincatto/box2d/blob/main/src/distance_joint.c, Catto implements the spring as part of the distance constraint, so you need to look where it says spring in the solver (and yes, we need all the warm starting and prepare and stuff). You can keep the spring a separate constraint, I'll leave that up to you, but, I think it would cool if our spring also had min and max lengths like our distance constraint has.

The worst part is really understanding how this stuff works. If you need anything explained more clearly, feel free to ask.

Co-authored-by: Tiago Antunes <78920440+fallenatlas@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Engine B-Physics D-Good-First-Issue Easy but interesting P-Backlog This issue isn't a priority at all

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement Spring Constraint

2 participants