Skip to content

Fix/potential energy reference height#315

Merged
mattqdev merged 2 commits into
physicshub:mainfrom
TechGenius-Karan:fix/potential-energy-reference-height
May 16, 2026
Merged

Fix/potential energy reference height#315
mattqdev merged 2 commits into
physicshub:mainfrom
TechGenius-Karan:fix/potential-energy-reference-height

Conversation

@TechGenius-Karan
Copy link
Copy Markdown
Contributor

📘 Pull Request Template – PhysicsHub

Thank you for contributing to PhysicsHub!
Please complete the sections below to help us review your pull request efficiently.


🔍 Description

Problem

getPotentialEnergy(gravity, referenceY) computes mass × g × (position.y − referenceY).

In BallGravity.jsx, referenceY was set to toMeters(p.height) — which
is the top of the canvas in Y-up coordinates (a large positive number).
Since the ball's Y position is always below the canvas top, this made PE
almost always a large negative number, which is physically incorrect.

Fix

Changed referenceY to size / 2 — the ball center's Y position when
resting on the floor. This means PE = 0 at ground level, which is the
standard physics convention and consistent with how BouncingBall.jsx
already handles it.

Closes #BUG-007 (if applicable)


✅ Checklist

Before requesting a review, please ensure that you have:

  • Verified that the project builds and runs locally (npm run dev)
  • Ensured no ESLint or TypeScript warnings/errors remain
  • Updated documentation, comments, or in-code explanations where needed
  • Verified responsiveness across devices (desktop, tablet, mobile)
  • Followed the CONTRIBUTING.md guidelines

🎨 Visual Changes (if UI-related)

IF CHANGES ARE RELATED TO SIMULATIONS PLEASE SEND A SHORT CLIP ABOUT IT
(OBLIGATORY)


📂 Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to change)
  • 📝 Documentation update
  • ♻️ Refactor / code quality improvement
  • 🎨 UI/UX enhancement
  • 🔒 Security improvement

🧩 Additional Notes for Reviewers

The drawLabel method in ForceRenderer hardcoded "N" (Newtons) as the
unit suffix for all vectors. In BallAcceleration, the acceleration
vector is drawn with F/m values (m/s²), so the label incorrectly
displayed "Acceleration (X.XN)".

Added a `unit` option to drawLabel so callers can override the default
"N". BallAcceleration now passes `{ unit: "m/s²" }` to drawVector,
fixing the label to correctly show "Acceleration (X.Xm/s²)".
getPotentialEnergy(gravity, referenceY) computes mass * g * (position.y - referenceY).
The previous referenceY of toMeters(p.height) is the canvas top in Y-up
coordinates, making PE almost always a large negative number.

Changed referenceY to size/2 (ball center Y when resting on the floor),
so PE correctly equals 0 at ground level — consistent with BouncingBall.
@mattqdev mattqdev merged commit 5655d5c into physicshub:main May 16, 2026
1 check passed
@mattqdev
Copy link
Copy Markdown
Collaborator

thank you for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants