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

Change timers process callback to physics #1228

Merged
merged 3 commits into from
Sep 18, 2022

Conversation

Xwdit
Copy link
Contributor

@Xwdit Xwdit commented Sep 16, 2022

Changed all timer process callbacks to physical frames so that they are framerate independent. This prevents dialogue from causing problems with certain game features that require precise timing. (e.g. replay system that can accelerate/decelerate)

This is based on a recent PR to SceneTreeTimer: godotengine/godot#63026

Copy link
Collaborator

@Jowan-Spooner Jowan-Spooner left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Looks clean and logical to me.

@coppolaemilio
Copy link
Collaborator

I think it makes sense to offer this as a feature, but you don't always want the timers to be based on frames, some times you need to sync with time, so an option should be added to select which one you want.

Do you think you can make that change to the PR?
It would require a new setting and check that setting for the timers to be set either by frame or time.

@Xwdit
Copy link
Contributor Author

Xwdit commented Sep 17, 2022

I think it makes sense to offer this as a feature, but you don't always want the timers to be based on frames, some times you need to sync with time, so an option should be added to select which one you want.

Do you think you can make that change to the PR? It would require a new setting and check that setting for the timers to be set either by frame or time.

Even if the timer process callback is changed to physics frame, it still be sync with time in seconds (based on delta time of physics frame), So it shouldn't cause any problems to regular users.

But I still added the corresponding setting for some special cases, or for someone doesn't like timing in physical frames. (Idle process callback is the default value)

We may also be able to instructing users to enable physical frame timing in the documentation to solve issues caused by erratic idle timing?

@@ -65,3 +66,7 @@ func custom_testing_scene_selected(path:String):
%TestingSceneLabel.text = path
ProjectSettings.set_setting('dialogic/editor/test_dialog_scene', path)
ProjectSettings.save()

func _on_physics_timer_button_toggled(button_pressed:bool):
Copy link
Collaborator

Choose a reason for hiding this comment

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

-> void:

return get_project_setting('dialogic/timer/process_in_physics', false)


static func update_timer_process_callback(timer:Timer):
Copy link
Collaborator

Choose a reason for hiding this comment

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

-> void:

@@ -175,7 +175,7 @@ func load_batch(data):

func _on_batch_loaded():
if _batches.size() > 0:
await get_tree().create_timer(0.01).timeout
await get_tree().create_timer(0.01, true, DialogicUtil.is_physics_timer()).timeout
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might not be a problem to do this here, but as this is in the editor, not in game, I'm not sure if it's necessary/good practice.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I think I will revert this one since it is in-editor

@coppolaemilio coppolaemilio merged commit d3626a3 into dialogic-godot:main Sep 18, 2022
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.

3 participants