Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

collide is aware of fixed positions of the nodes #225

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rioj7
Copy link

@rioj7 rioj7 commented Dec 30, 2023

If you have fixed position nodes (fx,fy !== null) that possibly overlap, the collision keeps these nodes fixed and we don't collide with a shadow version at d.x + d.vx, d.y + d.vy

@rioj7
Copy link
Author

rioj7 commented Dec 30, 2023

this will fix #213

I have recreated the tests from test/collide-test.js in a browser by implementing a very simple test runner.

The original code also fails the test forceCollide collides nodes and the 2 jiggle tests.

In the jiggle test forceCollide jiggles equal positions there are the lines

  assert.strictEqual(a.vx, -b.vx);
  assert.strictEqual(a.vy, -b.vy);

the test result says:

forceCollide jiggles equal positions
0.4397129164975401  0.4397129164975401 - values not strict equal
0.4082309018184576  0.4082309018184576 - values not strict equal

they are 16 decimal places equal, most likely the last bits (1 or 2) of the mantissa are different.
The assert should also compare with a delta.

The test forceCollide jiggles in a reproducible way assumes the random number generator produces the same number sequence for each test. Is this also true if you change the order of the tests?

If I try the modified code it has the same results for the test forceCollide collides nodes as the original code.
It should be because there are no nodes with fixed xy in the test and the modification only does something different for nodes with a fixed x-or-y.

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

Successfully merging this pull request may close these issues.

1 participant