-
-
Notifications
You must be signed in to change notification settings - Fork 21.6k
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
Expose disable_render_loop property to GDScript #39541
Expose disable_render_loop property to GDScript #39541
Conversation
Maybe it is better to rename the property |
@samdze yeah it looks better indeed. I sticked with the existing variable for some reason 😛 |
4026d1b
to
97ed09f
Compare
servers/rendering_server.h
Outdated
@@ -47,6 +47,7 @@ class RenderingServer : public Object { | |||
static RenderingServer *singleton; | |||
|
|||
int mm_policy; | |||
bool render_loop_enabled; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be initialized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is implicitly initialized from main.cpp
, but should it be more explicit then? And if yes, should I do it on the header or on the constructor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be explicitly initialized here, in the header.
Disabling the render loop is an edge use case, so the default should be that it's enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Can you explain what's your use case? |
Sure. My use case is for a integration testing framework I'm developing, where a user "records" a gameplay session that saves all the input given, and a few screenshots. Running the test is running the game with the input supplied, and comparing the screenshots to see if they suffered many changes. Do to this, I would use this property to disable rendering and speed up a lot of the logic processing, and There's also the aforemention of being useful to get data for an AI/machine learning system, as noted #1731 and here |
97ed09f
to
d001a06
Compare
d001a06
to
46b2689
Compare
servers/rendering_server.cpp
Outdated
@@ -1895,6 +1895,10 @@ void RenderingServer::_bind_methods() { | |||
ClassDB::bind_method(D_METHOD("has_os_feature", "feature"), &RenderingServer::has_os_feature); | |||
ClassDB::bind_method(D_METHOD("set_debug_generate_wireframes", "generate"), &RenderingServer::set_debug_generate_wireframes); | |||
|
|||
ClassDB::bind_method(D_METHOD("is_render_loop_enabled"), &RenderingServer::is_render_loop_enabled); | |||
ClassDB::bind_method(D_METHOD("set_render_loop_enabled", "render_loop_enabled"), &RenderingServer::set_render_loop_enabled); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make the argument only enable
, otherwise it's too redundant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah sorry, forgot about that binding as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
46b2689
to
03879a5
Compare
Thanks! |
I'll link a proposal of mine so that this feature doesn't seem too corner case: godotengine/godot-proposals#104 (see "Use case: recording input states for playback" section). |
This can't be cherry-picked trivially as beyond the |
I thought as much. In that case I'll re-implement it for the 3.2 branch |
Exposes the
disable_render_loop
property frommain.cpp
to GDScript.I've put the new code on
VisualServer
as it felt the most logic class to put to.