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.

Sign up for GitHub

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

Jump to bottom

Make viewport message dependent on framerate not physics step #87631

Merged
merged 1 commit into from
Feb 3, 2024
Merged

Make viewport message dependent on framerate not physics step #87631

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion editor/plugins/node_3d_editor_plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2836,7 +2836,7 @@ void Node3DEditorViewport::_notification(int p_what) {
last_message = message;
}

message_time -= get_physics_process_delta_time();
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it a matter of replacing get_physics_process_delta_time() with get_process_delta_time() here, without moving the code around?

Copy link
Contributor Author

@ryevdokimov ryevdokimov Jan 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It could be, but I wanted to keep it consistent with #87621, since canvas_item_editor_plugin.cpp doesn't have a notification for process. I thought it would be strange to make one just for this functionality, when either/or would work.

Edit: The fact that anything is in physics process that's unrelated to physics is the real strange thing.

Copy link
Contributor Author

@ryevdokimov ryevdokimov Feb 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Although this does beg the question of why the stuff in canvas plugin is in physics process and not just process?

Edit: And stuff in Node3D plugin as well. It all kind of seems arbitrary and possibly has some other unknown side effects. What does any of this stuff have to do with physics? We can change the physics step, so it's not like the default of 60 is some kind of requirement for anything.

Copy link
Contributor Author

@ryevdokimov ryevdokimov Feb 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I'll need to dig into this whole issue. It seems like it has the potential to affect several things. For now, I've applied your suggestion, since it makes a lot more sense given what I said above.

Copy link
Member

Choose a reason for hiding this comment

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

I've opened a separate issue for this: #87885

message_time -= get_process_delta_time();
if (message_time < 0) {
surface->queue_redraw();
}
Expand Down
Loading