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

[Platformer] Add a frame rate independent movement mode. #3417

Merged
merged 8 commits into from Jan 3, 2022

Conversation

D8H
Copy link
Collaborator

@D8H D8H commented Dec 31, 2021

This uses a Verlet integration.

It also fix the ledge grabbing: when the character is on exactly on right Y, it can now grab.
It add tests to check that the trajectory is the same on different frame rate for jump/fall and walking.

The existing tests have been adapted to this new mode. Only the basic jump test uses the legacy mode. I think it allows to check that the legacy mode works without having to maintain 2 test suites. But, if you want to reduce regression risk during the transition, I can add the old suite in a legacy folder.

I sometimes made tests less constraint. This is to only check what is really matter for the test to help readability and make them easier to maintain.

@D8H D8H requested a review from 4ian as a code owner December 31, 2021 01:26
@4ian
Copy link
Owner

4ian commented Jan 1, 2022

So cool! I'll check that tomorrow :)

Copy link
Owner

@4ian 4ian left a comment

Choose a reason for hiding this comment

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

Overall looks great!

behavior._requestedDeltaY -= previousJumpSpeed * timeDelta;

// Fall
// This is arbitrary. It used to not be obvious.
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure I understand this comment?

// Here, if we had pressed Right the character would have been falling.
runtimeScene.renderAndStep(1000 / 60);
expect(object.getBehavior('auto1').isOnLadder()).to.be(true);
// Now, it fells.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
// Now, it fells.
// Now, it falls.

// The condition is a legacy thing.
// There is no actual reason not to fall at 1st frame.
// Before a refactoring, it used to not be this obvious.
if (!this._jumpingFirstDelta) {
Copy link
Owner

Choose a reason for hiding this comment

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

I indeed can't remember why I added this 🤔

@4ian 4ian merged commit ae6a77d into 4ian:master Jan 3, 2022
@4ian
Copy link
Owner

4ian commented Jan 3, 2022

Thank you for working on 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.

None yet

2 participants