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

Deprecate RenderingServer's has_feature and Features enum #87035

Merged

Conversation

Mickeon
Copy link
Contributor

@Mickeon Mickeon commented Jan 10, 2024

Closes #87034

This PR deprecates the RenderingServer's has_feature method...
... Which is not to be confused with the RenderingServer's own has_os_feature...
... Which is not to be confused with OS's has_feature... Ok.

The method, and its corresponding enum have never, never been used for anything at all, ever since they existed in 3.0. In fact, they were so forgotten that no one bothered to touch-up the description for 4.0.

@Mickeon Mickeon requested a review from a team as a code owner January 10, 2024 10:33
@Mickeon Mickeon force-pushed the deprecate-RenderingServer-has-feature branch from 2a45688 to b64146d Compare January 10, 2024 10:41
@akien-mga
Copy link
Member

Makes sense.

I would suggest also enclosing the actual enum and method definitions in #ifndef DISABLE_DEPRECATED so they're compiled out when using that option, and it makes it clear in code that they should be removed when we can.

diff --git a/servers/rendering/rendering_server_default.cpp b/servers/rendering/rendering_server_default.cpp
index 1e6a311cc2..f7620ad5dc 100644
--- a/servers/rendering/rendering_server_default.cpp
+++ b/servers/rendering/rendering_server_default.cpp
@@ -287,9 +287,11 @@ void RenderingServerDefault::set_default_clear_color(const Color &p_color) {
 	RSG::viewport->set_default_clear_color(p_color);
 }
 
+#ifndef DISABLE_DEPRECATED
 bool RenderingServerDefault::has_feature(Features p_feature) const {
 	return false;
 }
+#endif
 
 void RenderingServerDefault::sdfgi_set_debug_probe_select(const Vector3 &p_position, const Vector3 &p_dir) {
 	RSG::scene->sdfgi_set_debug_probe_select(p_position, p_dir);
diff --git a/servers/rendering/rendering_server_default.h b/servers/rendering/rendering_server_default.h
index dac1275957..11ec670280 100644
--- a/servers/rendering/rendering_server_default.h
+++ b/servers/rendering/rendering_server_default.h
@@ -1021,7 +1021,9 @@ public:
 	virtual Color get_default_clear_color() override;
 	virtual void set_default_clear_color(const Color &p_color) override;
 
+#ifndef DISABLE_DEPRECATED
 	virtual bool has_feature(Features p_feature) const override;
+#endif
 
 	virtual bool has_os_feature(const String &p_feature) const override;
 	virtual void set_debug_generate_wireframes(bool p_generate) override;
diff --git a/servers/rendering_server.cpp b/servers/rendering_server.cpp
index 2d61e47062..27e677bce0 100644
--- a/servers/rendering_server.cpp
+++ b/servers/rendering_server.cpp
@@ -3362,7 +3362,6 @@ void RenderingServer::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("get_default_clear_color"), &RenderingServer::get_default_clear_color);
 	ClassDB::bind_method(D_METHOD("set_default_clear_color", "color"), &RenderingServer::set_default_clear_color);
 
-	ClassDB::bind_method(D_METHOD("has_feature", "feature"), &RenderingServer::has_feature);
 	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);
 
@@ -3380,9 +3379,6 @@ void RenderingServer::_bind_methods() {
 	BIND_ENUM_CONSTANT(RENDERING_INFO_BUFFER_MEM_USED);
 	BIND_ENUM_CONSTANT(RENDERING_INFO_VIDEO_MEM_USED);
 
-	BIND_ENUM_CONSTANT(FEATURE_SHADERS);
-	BIND_ENUM_CONSTANT(FEATURE_MULTITHREADED);
-
 	ADD_SIGNAL(MethodInfo("frame_pre_draw"));
 	ADD_SIGNAL(MethodInfo("frame_post_draw"));
 
@@ -3392,6 +3388,13 @@ void RenderingServer::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("create_local_rendering_device"), &RenderingServer::create_local_rendering_device);
 
 	ClassDB::bind_method(D_METHOD("call_on_render_thread", "callable"), &RenderingServer::call_on_render_thread);
+
+#ifndef DISABLE_DEPRECATED
+	ClassDB::bind_method(D_METHOD("has_feature", "feature"), &RenderingServer::has_feature);
+
+	BIND_ENUM_CONSTANT(FEATURE_SHADERS);
+	BIND_ENUM_CONSTANT(FEATURE_MULTITHREADED);
+#endif
 }
 
 void RenderingServer::mesh_add_surface_from_mesh_data(RID p_mesh, const Geometry3D::MeshData &p_mesh_data) {
diff --git a/servers/rendering_server.h b/servers/rendering_server.h
index 8b17463b4d..a491efc64c 100644
--- a/servers/rendering_server.h
+++ b/servers/rendering_server.h
@@ -1612,12 +1612,15 @@ public:
 	virtual Color get_default_clear_color() = 0;
 	virtual void set_default_clear_color(const Color &p_color) = 0;
 
+#ifndef DISABLE_DEPRECATED
+	// Never actually used, should be removed when we can break compat.
 	enum Features {
 		FEATURE_SHADERS,
 		FEATURE_MULTITHREADED,
 	};
 
 	virtual bool has_feature(Features p_feature) const = 0;
+#endif
 
 	virtual bool has_os_feature(const String &p_feature) const = 0;
 

@Mickeon Mickeon force-pushed the deprecate-RenderingServer-has-feature branch from b64146d to b82a629 Compare January 10, 2024 10:57
@Mickeon Mickeon requested a review from a team as a code owner January 10, 2024 10:57
@Mickeon
Copy link
Contributor Author

Mickeon commented Jan 10, 2024

Applied all of the changes mentioned above. Thank you.

@Mickeon Mickeon force-pushed the deprecate-RenderingServer-has-feature branch from b82a629 to 63a08f2 Compare January 10, 2024 11:26
@Mickeon
Copy link
Contributor Author

Mickeon commented Jan 10, 2024

Should be all good now.

Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Makes sense to me

@akien-mga akien-mga merged commit bf4fb98 into godotengine:master Jan 11, 2024
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@Mickeon Mickeon deleted the deprecate-RenderingServer-has-feature branch January 11, 2024 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants