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

test: linearVelocity deteriorates with time steps over 0.2s #84

Closed
wants to merge 4 commits into from

Conversation

adil192
Copy link

@adil192 adil192 commented Aug 6, 2023

Description

Added a (failing) test that bodies with no linear damping shouldn't slow down over time.

If you'd like me to close this PR and just make a regular bug report issue, please let me know :)

Checklist

  • The title of my PR starts with a Conventional Commit prefix (fix:, feat:, docs: etc).
  • I have read the Contributor Guide and followed the process outlined for submitting PRs.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • [-] I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • [-] I have updated/added relevant examples in examples.

Breaking Change

  • [-] Yes, this is a breaking change.
  • No, this is not a breaking change.

Related Issues

adil192/ricochlime#1

Other info

`flutter doctor --verbose`
[✓] Flutter (Channel stable, 3.10.6, on macOS 14.0 23A5286i darwin-x64, locale en-GB)
    • Flutter version 3.10.6 on channel stable at /Users/adilhanney/development/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision f468f3366c (4 weeks ago), 2023-07-12 15:19:05 -0700
    • Engine revision cdbeda788a
    • Dart version 3.0.6
    • DevTools version 2.23.1

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.2)
    • Android SDK at /Users/adilhanney/Library/Android/sdk
    • Platform android-33, build-tools 33.0.2
    • Java binary at: /usr/local/opt/openjdk/bin/java
    • Java version OpenJDK Runtime Environment Homebrew (build 20.0.1)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.3.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14E300c
    • CocoaPods version 1.12.1

[✗] Chrome - develop for the web (Cannot find Chrome executable at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome)
    ! Cannot find Chrome. Try setting CHROME_EXECUTABLE to a Chrome executable.

[!] Android Studio
    • Android Studio at /Users/adilhanney/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-0/223.8836.35.2231.9923731/Android Studio Preview.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    ✗ Unable to find bundled Java version.
    • Try updating or re-installing Android Studio.

[!] Android Studio
    • Android Studio at /Users/adilhanney/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-0/223.8836.35.2231.10023527/Android Studio Preview.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    ✗ Unable to find bundled Java version.
    • Try updating or re-installing Android Studio.

[✓] VS Code (version 1.80.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.70.0

[✓] Connected device (1 available)            
    • macOS (desktop) • macos • darwin-x64 • macOS 14.0 23A5286i darwin-x64

[✓] Network resources
    • All expected network resources are available.

! Doctor found issues in 3 categories.

@spydon
Copy link
Member

spydon commented Aug 7, 2023

Have you checked that the body doesn't fall asleep when you do such a large timestep?

@adil192
Copy link
Author

adil192 commented Aug 7, 2023

Have you checked that the body doesn't fall asleep when you do such a large timestep?

The test still fails when I set allowSleep to false.

I've found with 11524c5 that the test fails when the time step is bigger than 0.2s (not including 0.2s).

I tried to find and replace all 0.2 literals in the forge2d repo with 0.3 but this didn't help.

@adil192 adil192 changed the title test: constant velocity with no linear damping test: linearVelocity deteriorates with time steps over 0.2s Aug 7, 2023
@spydon
Copy link
Member

spydon commented Aug 8, 2023

Does this also happen on lower velocities and the same timesteps? Maybe it hits some upper velocity limit per timestep

@adil192
Copy link
Author

adil192 commented Aug 8, 2023

Good idea. I changed the velocity from 10 to 5 and the timestep error occured at 0.401 instead of 0.201

@spydon
Copy link
Member

spydon commented Aug 8, 2023

If you set this to something higher, does it still happen?

@adil192
Copy link
Author

adil192 commented Aug 8, 2023

/// The maximum linear velocity of a body. This limit is very large and is used
/// to prevent numerical problems. You shouldn't need to adjust this.
double maxTranslation = 2.0;

Looks like this maxTranslation value is to blame

Edit: Looks like we commented at the same time; changing the value to e.g. 4.0 passes the test successfully

@spydon
Copy link
Member

spydon commented Aug 8, 2023

Since it is that one, I would just zoom in for the rendering and use lower velocities :)
https://www.iforce2d.net/b2dtut/gotchas#speedlimit

@adil192
Copy link
Author

adil192 commented Aug 8, 2023

I could simply increase this value but I don't think that actually solves the problem, but just increases the maximum time step before we see unexpected behaviour (e.g. when the user minimises and then reopens the app, the time step will be several seconds).

It seems like we should be multiplying maxTranslation by the time step to allow it to scale properly, perhaps renaming the value to maxTranslationPerSecond or rather just maxVelocity?

@adil192
Copy link
Author

adil192 commented Aug 8, 2023

Oh okay I'll try that

@adil192
Copy link
Author

adil192 commented Aug 8, 2023

Even when I set the zoom to 10000000000.0 it's still too slow for my bullets. (After every collision, I check to see if the speed is <10% of what it's supposed to be, which is almost always true in debug mode).
Do you know if there's a way to change the maxTranslation value without forking forge2d?
Or maybe we could preserve the maxTranslation value, but if we hit this limit, leave the velocity as it is?

@adil192
Copy link
Author

adil192 commented Aug 8, 2023

Also my previous point about minimising and reopening the app or browser tab still stands, see this screen recording where the bullets are at almost zero velocity after returning to the tab
https://github.com/flame-engine/forge2d/assets/21128619/e0b420af-1e13-4a4c-a1ab-a9e0f787f052

@spydon
Copy link
Member

spydon commented Aug 8, 2023

Even when I set the zoom to 10000000000.0 it's still too slow for my bullets. (After every collision, I check to see if the speed is <10% of what it's supposed to be, which is almost always true in debug mode). Do you know if there's a way to change the maxTranslation value without forking forge2d? Or maybe we could preserve the maxTranslation value, but if we hit this limit, leave the velocity as it is?

That sounds very strange, are you using flame_forge2d?

Also my previous point about minimising and reopening the app or browser tab still stands, see this screen recording where the bullets are at almost zero velocity after returning to the tab https://github.com/flame-engine/forge2d/assets/21128619/e0b420af-1e13-4a4c-a1ab-a9e0f787f052

You have to either pause the engine or just discard too big dts when that happens.

@spydon
Copy link
Member

spydon commented Aug 8, 2023

Do you know if there's a way to change the maxTranslation value without forking forge2d?

That variable is mutable, you should be able to just change it directly.

@adil192
Copy link
Author

adil192 commented Aug 8, 2023

That sounds very strange, are you using flame_forge2d?

Yep I'm using flame_forge2d: ^0.14.1

You have to either pause the engine or just discard too big dts when that happens.

I feel like this should be part of the physics engine, e.g. if the dt is bigger than some threshold, break it up into 0.1s increments.
Edit: I'm making a PR in the flame_forge2d repo

@adil192
Copy link
Author

adil192 commented Aug 8, 2023

Do you know if there's a way to change the maxTranslation value without forking forge2d?

That variable is mutable, you should be able to just change it directly.

import 'package:forge2d/src/settings.dart' as physics_settings;
physics_settings.maxTranslation = Bullet.speed;

This works perfectly albeit with this warning:

Import of a library in the 'lib/src' directory of another package.
Try importing a public library that exports this library, or removing the import.dart[implementation_imports](https://dart-lang.github.io/linter/lints/implementation_imports.html)

@adil192 adil192 closed this Aug 13, 2023
@adil192 adil192 deleted the constant-velocity-test branch August 13, 2023 04:15
spydon pushed a commit to flame-engine/flame that referenced this pull request Sep 21, 2023
When the game is backgrounded and then resumed, the update function is called with a very large dt time step.

This might cause the physics engine to hit the maximum translation per timestep, causing moving bodies to slow down to almost-zero velocities.

This PR makes the game pause if the game is backgrounded. If you'd like to change this behaviour, set the pauseWhenBackgrounded flag to true (default) or false.

class MyGame extends FlameGame {
  MyGame() {
    pauseWhenBackgrounded = false;
  }
}

This is currently not working on the web, see flutter/flutter#53107.

You can view previous discussion about this here: flame-engine/forge2d#84.
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