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

Getting negative delta values for _process #26887

Closed
trommlbomml opened this issue Mar 10, 2019 · 32 comments · Fixed by #35617
Closed

Getting negative delta values for _process #26887

trommlbomml opened this issue Mar 10, 2019 · 32 comments · Fixed by #35617

Comments

@trommlbomml
Copy link

trommlbomml commented Mar 10, 2019

Godot version:
Godot 3.1. RC 1

OS/device including version:
Windows 10 Home 64-bit, Geforce GTX 860m

Issue description:
I already found some similar issue, but it was related to android only. Therefore I named the issue similar but the error also seems to occur on windows machines.

I noticed my script crashes because of a division by zero (Stripped down to relevant part):

func _process(delta):
	if abs(angle) > PI * 4 * delta:
		thisRotation = current.slerp(target, max_radians_delta / abs(angle))
	else:
		thisRotation = current.slerp(target, 1)

For my case, angle = 0. If delta is negative the first code path is executed and results in a division by zero error. This happens sometimes when I start my game and do not move my character.

If this is relevant: I use the bullet physics with KinematicBody and StaticBodyies.

Minimal reproduction project:
I would provide the project if not easy to reproduce, I think a simple print if delta < 0 is sufficient.

@matthew1006
Copy link
Contributor

I also have this problem with 3.1 stable on Windows 7.
delta_-0 007
delta_0

@nathanfranke
Copy link
Contributor

nathanfranke commented Apr 17, 2019

Reproducable on Ubuntu 18.04.2 LTS (Kernal x86_64 Linux 4.18.0-17-generic)
GeForce GTX 960

Godot: v3.1.stable.mono.official

The issue only seems to come during very high stress (In my example, a ton of high poly spheres are spawned)

GodotTestProject.zip

@Faless
Copy link
Collaborator

Faless commented Oct 3, 2019

To anyone that can reproduce the original issue
can you try this branch?
https://github.com/Faless/godot/tree/spike/clock_info
Is it still happening there?

The patch just tries to avoid some unnecessary math, limiting potential overflows and hopefully minimizing numerical errors:
master...Faless:spike/clock_info

(there is also a 3.1 version of this patch: https://github.com/Faless/godot/tree/spike/clock_info_3.1 )

@nathanfranke
Copy link
Contributor

nathanfranke commented Oct 16, 2019

I can still reproduce this in all branches. More data below

Ubuntu 18.04.2 LTS
Kernal x86_64 Linux 4.18.0-17-generic
GeForce GTX 960

@Faless branch Standard: Bug is Reproducible
Master Standard, Master Mono: Bug is Reproducible

Seems to happen somewhat randomly, and behavior can be changed based on background processes and performance

Updated test project:
This project uses GDScript rather than C# so that the bug is testable in the Standard version.
GodotTestProject.zip

@Faless
Copy link
Collaborator

Faless commented Oct 18, 2019 via email

@Faless
Copy link
Collaborator

Faless commented Oct 18, 2019 via email

@nathanfranke
Copy link
Contributor

nathanfranke commented Oct 18, 2019

Motherboard: Gigabyte H170-Gaming 3
CPU: Intel i5 6600k

Not at home to check Vsync enabled, but I always get 60fps using it
Without Vsync I get hundreds of FPS, however the bug seems to only appear during lag spikes or bad performance

@Faless
Copy link
Collaborator

Faless commented Oct 18, 2019 via email

@nathanfranke
Copy link
Contributor

No problem edited my response already

@Faless
Copy link
Collaborator

Faless commented Oct 20, 2019 via email

@nathanfranke
Copy link
Contributor

I cannot reproduce this bug with VSync enabled

@Faless
Copy link
Collaborator

Faless commented Oct 21, 2019

I cannot reproduce this bug with VSync enabled

I see, and am I guessing right that when you get negative deltas it's actually an incredibly small negative value? (i.e. negative zero -0.00000)

@nathanfranke
Copy link
Contributor

nathanfranke commented Oct 21, 2019

Correct. The values are quite small. Here is a log

0.005
0.07277458
0.01940706
0.032334
0.01548033
-0.0005547218

Therefore it is likely unrelated to the Android bug that was fixed (mentioned above)

@nathanfranke
Copy link
Contributor

nathanfranke commented Jan 25, 2020

I found a very easy way to reproduce this. I am assuming it only happens on Unix since the problem is likely in OS_Unix::get_ticks_usec which calls SceneTree::idle with a negative p_time

Here is the script that has <0 delta consistently after 1-2 seconds

extends Node

func _process(delta : float):
	if delta < 0:
		print(delta)
	
	if randf() < 0.25: # Inconsistent lag spikes
		stress()

func stress():
	for i in range(100000):
		randi()

TimeBug.zip

@Zylann
Copy link
Contributor

Zylann commented Jun 3, 2020

@zmanuel well in my case I didnt really have much of a choice, it's in calculation of a derivative. delta=0 here would mean no time elapsed, so... I just returned at the beginning of the function, which then might as well not have been called. And if I had code that luckily did nothing, then still no point running it. And if delta is negative, I believe it should also not run either, because where would a "positive" value come from then? Such cases make no sense, unless you are inclined to implement your game simulation for every possible value of delta, be it negative or zero, that's quite a big ask^^"

@ghost
Copy link

ghost commented Jun 4, 2020

@Zylann That is an interesting point. If you get back a 0 in delta, it is does represent there is/was no time in which to do anything, and anything multiplied by delta 0 is going to do nothing. So why call the function? I am wondering if there are some cases with 0 delta where you'd still want to execute something not dependent on time, but wouldn't want to locate it in the physics process loop.

If the goal is to not surprise people (oh no, game was running fine for weeks, but today crashed because of divide by 0?!) or add extra work to the developer (maintaining code for guarding against 0 in dozens to hundreds of places), it probably shouldn't execute delta <= 0.0, and just be documented it might not execute at all if it's out of time.

@zmanuel
Copy link
Contributor

zmanuel commented Jun 6, 2020

@avencherus A legitimate case where you would want _process to still be called on zero timesteps is if you want to implement your own framerate statistics, you'd miss render frames. A slightly more contrived example would be old-school fixed timesteps; some game may decide it works best if it engages v-sync and just assumes to get _process-calls 60 times per second. Sure, that means it gets slowdown if frames are missed or the framerate drops to 30 fps, but in some cases that may be preferable to the alternative. I remember reading in a very old shmup review something along the lines of "The game slows down when the action gets too much, but then again, that helps to get through the tougher spots." Ultimately, what I guess I'm saying is that control over the number of physics steps and delta arguments should be given to the game, with reasonable defaults.

@Zylann Negative timesteps are out of the question. Even if you implement your timestep physically accurately, negative steps just reverse the mechanical time, not thermodynamic time. Take your basic fake friction update:
velocity = velocity * exp(-lambda * delta)
with some positive friction strength lambda. Normally, it decreases the velocity and is benign and stable, but if you feed that negative delta, it increases it uncontrollably. For positive delta, it is furthermore perfectly fine to approximate the above update to
velocity = velocity / (1 + lambda * delta)
to avoid a possibly expensive exp call (I guess in GDScript, that does not matter much), and that formula breaks hard at delta < -1/lambda.

Regarding your derivative, you may already be getting problems at small timesteps. I assume you're doing something along the lines

old_value = value
.... (calculate value in some complicated way)
derivative = (value - old_value)/delta
old_value = value

Since old_value and value are likely to be very close, that runs into the old numeric problem of cancellation and elimination of significant digits. The lowest allowed delta argument in the old version of the PR was 1E-6; (single precision) floats are only accurate to about 1E-8, which only leaves your derivative with an accuracy of about 1/100, provided it is of the same size as value, approximately. If it's smaller (say, an object moving at 1 m/s, 100 m away), you're completely out of accuracy.
If possible, and I'm aware it is not always possible, it's better to first calculate the derivative, then update the value with
value = value + derivative * delta
That still has accuracy problems if derivative * delta is much smaller than value, but they don't show as much. And of course, naive application of this leads to exploding physics.
TL;DR: Bailing out on non-positive delta is a valid soluiton :)

@Calinou Calinou changed the title Getting negative delta values for _process on Windows Getting negative delta values for _process Jul 10, 2020
zmanuel added a commit to zmanuel/godot that referenced this issue Oct 31, 2020
Three attack points, all after the regular calculations:
1. Prevent negative physics timestep counts. They could occur if
   physics_jtter_fix is changed at runtime.
2. idle_step is not allowed to go below zero. That could happen
   on physics_jitter_fix changes or heavily fluctuating performance.
3. Prevent that the idle_step modification breaks the promise
   that Engine.get_physics_interpolation_fraction() is between
   0 and 1 by doing more physics steps than the base system wants.

Fixes godotengine#26887
@hhyyrylainen
Copy link

I was just wondering where I got some NaNs showing up in a Label, and after looking into it, I noticed it happens when some of my game logic inside _Process is called with exactly 0 delta. Funnily enough negative delta seems to work fine with the calculations I'm doing. Based on that I found this issue.
This is happening on Linux using 3.2.3.stable.mono.official. My code is in C#

Just want to let you know that I'd much prefer if Godot would only pass delta > 0 to my code, so that I don't have to look through all of my _Process methods looking for places where I need to bail if delta is <= 0. Even if that would be an option that needs to be explicitly enabled.

@felixscheffer
Copy link

felixscheffer commented Jan 7, 2021

I am getting a delta of -0,005539333 on Win10 using 3.2.4-beta4_mono. Code is in C#

So I don't think it's specific to Unix. As delta is supposed to be the time since the previous frame I don't think negative values make any sense.

@fractilegames
Copy link

I hit this same problem in Godot 3.2.4rc3 (on Xubuntu 20.10 x86_64).

There was some discussion on whether zero is a valid value for delta at all. If I'm not mistaken, one more case where zero delta is expected is when using small (or zero) value in Engine.time_scale.

My code currently handles zero deltas but I wasn't prepared for negative values.

@zmanuel
Copy link
Contributor

zmanuel commented Mar 14, 2021

A fix has been ready for merging for quite some time now, any ideas how we can actually get it merged?

@Calinou
Copy link
Member

Calinou commented Mar 14, 2021

@zmanuel We can't merge it in 3.2 right now as 3.2.4 is nearing release (to avoid regressions), but I guess it can be merged in master first. I don't merge pull requests on the main Godot repository though.

@zmanuel
Copy link
Contributor

zmanuel commented Mar 16, 2021

I understand. Who can I bother and when would be the right time to go nagging? A week or so after 3.2.4 is out?

@Calinou
Copy link
Member

Calinou commented Mar 17, 2021

I understand. Who can I bother and when would be the right time to go nagging? A week or so after 3.2.4 is out?

akien-mga is the one who merges pull requests, and I think you can ask him once 1-2 weeks have passed after 3.3's release.

Note: The next Godot release will be 3.3 instead of 3.2.4.

@akien-mga akien-mga added this to the 4.0 milestone Mar 17, 2021
zmanuel added a commit to zmanuel/godot that referenced this issue Mar 21, 2021
Three attack points, all after the regular calculations:
1. Prevent negative physics timestep counts. They could occur if
   physics_jtter_fix is changed at runtime.
2. idle_step is not allowed to go below 1/8th of the input step.
   That could happen on physics_jitter_fix changes or heavily
   fluctuating performance.
3. Prevent that the idle_step modification breaks the promise
   that Engine.get_physics_interpolation_fraction() is between
   0 and 1 by doing more physics steps than the base system wants.

Fixes godotengine#26887

Co-authored-by: Hugo Locurcio <hugo.locurcio@hugo.pro>
lawnjelly pushed a commit that referenced this issue Sep 6, 2021
Three attack points, all after the regular calculations:
1. Prevent negative physics timestep counts. They could occur if
   physics_jtter_fix is changed at runtime.
2. idle_step is not allowed to go below 1/8th of the input step.
   That could happen on physics_jitter_fix changes or heavily
   fluctuating performance.
3. Prevent that the idle_step modification breaks the promise
   that Engine.get_physics_interpolation_fraction() is between
   0 and 1 by doing more physics steps than the base system wants.

Fixes #26887

Co-authored-by: Hugo Locurcio <hugo.locurcio@hugo.pro>
zmanuel added a commit to zmanuel/godot that referenced this issue Sep 22, 2021
Three attack points, all after the regular calculations:
1. Prevent negative physics timestep counts. They could occur if
   physics_jtter_fix is changed at runtime.
2. idle_step is not allowed to go below 1/8th of the input step.
   That could happen on physics_jitter_fix changes or heavily
   fluctuating performance.
3. Prevent that the idle_step modification breaks the promise
   that Engine.get_physics_interpolation_fraction() is between
   0 and 1 by doing more physics steps than the base system wants.

Fixes godotengine#26887

Co-authored-by: Hugo Locurcio <hugo.locurcio@hugo.pro>
Cherry-Pick from 6be702b.
sairam4123 pushed a commit to sairam4123/godot that referenced this issue Nov 10, 2021
Three attack points, all after the regular calculations:
1. Prevent negative physics timestep counts. They could occur if
   physics_jtter_fix is changed at runtime.
2. idle_step is not allowed to go below 1/8th of the input step.
   That could happen on physics_jitter_fix changes or heavily
   fluctuating performance.
3. Prevent that the idle_step modification breaks the promise
   that Engine.get_physics_interpolation_fraction() is between
   0 and 1 by doing more physics steps than the base system wants.

Fixes godotengine#26887

Co-authored-by: Hugo Locurcio <hugo.locurcio@hugo.pro>
Cherry-Pick from 6be702b.
lekoder pushed a commit to KoderaSoftwareUnlimited/godot that referenced this issue Dec 18, 2021
Three attack points, all after the regular calculations:
1. Prevent negative physics timestep counts. They could occur if
   physics_jtter_fix is changed at runtime.
2. idle_step is not allowed to go below 1/8th of the input step.
   That could happen on physics_jitter_fix changes or heavily
   fluctuating performance.
3. Prevent that the idle_step modification breaks the promise
   that Engine.get_physics_interpolation_fraction() is between
   0 and 1 by doing more physics steps than the base system wants.

Fixes godotengine#26887

Co-authored-by: Hugo Locurcio <hugo.locurcio@hugo.pro>
Cherry-Pick from 6be702b.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.