Apply suggestions from code review

Co-authored-by: Thaddeus Crews <[email protected]>
Co-authored-by: A Thousand Ships <[email protected]>

Author: 3 people

SConstruct

@@ -356,13 +356,6 @@ if not env["platform"]:
     if env["platform"]:
         print(f"Automatically detected platform: {env['platform']}")
 
-# Library Support
-if env["library_type"] != "executable":
-    if env["platform"] not in ["linuxbsd", "macos", "windows"]:
-        print_error(f"Library builds not yet supported for {env['platform']}")
-        Exit(255)
-    env.Append(CPPDEFINES=["LIBGODOT_ENABLED"])
-
 # Deprecated aliases kept for compatibility.
 if env["platform"] in compatibility_platform_aliases:
     alias = env["platform"]
@@ -571,6 +564,13 @@ if not env["deprecated"]:
 if env["precision"] == "double":
     env.Append(CPPDEFINES=["REAL_T_IS_DOUBLE"])
 
+# Library Support
+if env["library_type"] != "executable":
+    if "library" not in env.get("supported", []):
+        print_error(f"Library builds unsupported for {env['platform']}")
+        Exit(255)
+    env.Append(CPPDEFINES=["LIBGODOT_ENABLED"])
+
 # Default num_jobs to local cpu count if not user specified.
 # SCons has a peculiarity where user-specified options won't be overridden
 # by SetOption, so we can rely on this to know if we should use our default.

core/extension/gdextension_function_loader.cpp

@@ -29,6 +29,7 @@
 /**************************************************************************/
 
 #include "gdextension_function_loader.h"
+
 #include "gdextension.h"
 
 Error GDExtensionFunctionLoader::open_library(const String &p_path) {
@@ -47,8 +48,7 @@ Error GDExtensionFunctionLoader::initialize(GDExtensionInterfaceGetProcAddress p
 	if (ret) {
 		return OK;
 	} else {
-		ERR_PRINT("GDExtension initialization function for '" + library_path + "' returned an error.");
-		return FAILED;
+		ERR_FAIL_V_MSG(FAILED, "GDExtension initialization function for '" + library_path + "' returned an error.");
 	}
 }
 

core/extension/gdextension_function_loader.h

@@ -39,17 +39,16 @@ class GDExtensionFunctionLoader : public GDExtensionLoader {
 	friend class GDExtensionManager;
 	friend class GDExtension;
 
-private:
 	String library_path;
 	GDExtensionInitializationFunction initialization_function = nullptr;
 
 public:
-	Error open_library(const String &p_path) override;
+	virtual Error open_library(const String &p_path) override;
 	Error initialize(GDExtensionInterfaceGetProcAddress p_get_proc_address, const Ref<GDExtension> &p_extension, GDExtensionInitialization *r_initialization) override;
 	void close_library() override;
 	bool is_library_open() const override;
 	bool has_library_changed() const override;
 	bool library_exists() const override;
 
-	void set_initialization_function(GDExtensionInitializationFunction initialization_function);
+	void set_initialization_function(GDExtensionInitializationFunction p_initialization_function);
 };

core/extension/gdextension_manager.cpp

@@ -121,7 +121,7 @@ GDExtensionManager::LoadStatus GDExtensionManager::load_extension(const String &
 GDExtensionManager::LoadStatus GDExtensionManager::load_extension_from_function(const String &p_path, GDExtensionConstPtr<const GDExtensionInitializationFunction> p_init_func) {
 	Ref<GDExtensionFunctionLoader> func_loader;
 	func_loader.instantiate();
-	func_loader->set_initialization_function((GDExtensionInitializationFunction) * (p_init_func.data));
+	func_loader->set_initialization_function((GDExtensionInitializationFunction)*p_init_func.data);
 	return load_extension_with_loader(p_path, func_loader);
 }
 

core/extension/godot_instance.cpp

@@ -29,6 +29,7 @@
 /**************************************************************************/
 
 #include "godot_instance.h"
+
 #include "core/extension/gdextension_manager.h"
 #include "main/main.h"
 #include "servers/display_server.h"

core/extension/godot_instance.h

@@ -32,15 +32,15 @@
 
 #include "core/extension/gdextension_interface.h"
 #include "core/object/class_db.h"
-#include "core/object/object.h"
 
 class GodotInstance : public Object {
 	GDCLASS(GodotInstance, Object);
 
-	static void _bind_methods();
-
 	bool started = false;
 
+protected:
+	static void _bind_methods();
+
 public:
 	GodotInstance();
 	~GodotInstance();

doc/classes/GDExtensionManager.xml

@@ -44,7 +44,7 @@
 			<param index="0" name="path" type="String" />
 			<param index="1" name="init_func" type="const GDExtensionInitializationFunction*" />
 			<description>
-				Loads an already-in-address-space extension by unique name and initialization function. The [param path] needs to be unique and start with libgodot://. Returns [constant LOAD_STATUS_OK] if successful.
+				Loads an extension already in address space by unique name and initialization function. The [param path] needs to be unique and start with libgodot://. Returns [constant LOAD_STATUS_OK] if successful.
 			</description>
 		</method>
 		<method name="reload_extension">

doc/classes/GodotInstance.xml

@@ -4,7 +4,7 @@
 		Provides access to an embedded Godot instance.
 	</brief_description>
 	<description>
-		GodotInstance represents a running Godot instance that is controlled from an outside codebase, without a perpetual main loop. It is created by the C API libgodot_create_godot_instance. Only one may be created per process.
+		GodotInstance represents a running Godot instance that is controlled from an outside codebase, without a perpetual main loop. It is created by the C API [code]libgodot_create_godot_instance[/code]. Only one may be created per process.
 	</description>
 	<tutorials>
 	</tutorials>
@@ -24,13 +24,13 @@
 		<method name="is_started">
 			<return type="bool" />
 			<description>
-				Returns true if this instance has been fully started.
+				Returns [code]true[/code] if this instance has been fully started.
 			</description>
 		</method>
 		<method name="iteration">
 			<return type="bool" />
 			<description>
-				Runs a single iteration of the main loop. Returns true if the engine is attempting to quit.
+				Runs a single iteration of the main loop. Returns [code]true[/code] if the engine is attempting to quit.
 			</description>
 		</method>
 		<method name="pause">
@@ -48,7 +48,7 @@
 		<method name="start">
 			<return type="bool" />
 			<description>
-				Finishes this instance's startup sequence. Returns true on success.
+				Finishes this instance's startup sequence. Returns [code]true[/code] on success.
 			</description>
 		</method>
 	</methods>

methods.py

@@ -88,8 +88,8 @@ def redirect_emitter(target, source, env):
     Emitter to automatically redirect object/library build files to the `bin/obj` directory,
     retaining subfolder structure. External build files will attempt to retain subfolder
     structure relative to their environment's parent directory, sorted under `bin/obj/external`.
-    If `redirect_build_objects` is `False`, or an external build file isn't relative to the
-    passed environment, or a file is being written directly into `bin`, this emitter does nothing.
+    If `redirect_build_objects` is `False`, an external build file isn't relative to the passed
+    environment, or a file is being written directly into `bin`, this emitter does nothing.
     """
     if not env["redirect_build_objects"]:
         return target, source

platform/linuxbsd/SCsub

@@ -13,11 +13,10 @@ common_linuxbsd = [
     "freedesktop_at_spi_monitor.cpp",
 ]
 
-libgodot_files = [
-    "libgodot_linuxbsd.cpp",
-]
-
-executable_files = ["godot_linuxbsd.cpp"]
+if env["library_type"] == "executable":
+    common_linuxbsd += ["godot_linuxbsd.cpp"]
+else:
+    common_linuxbsd += ["libgodot_linuxbsd.cpp"]
 
 if env["use_sowrap"]:
     common_linuxbsd.append("xkbcommon-so_wrap.c")
@@ -42,12 +41,12 @@ if env["dbus"]:
         common_linuxbsd.append("dbus-so_wrap.c")
 
 if env["library_type"] == "static_library":
-    prog = env.add_library("#bin/godot", common_linuxbsd + libgodot_files)
+    prog = env.add_library("#bin/godot", common_linuxbsd)
 elif env["library_type"] == "shared_library":
     env.Append(CCFLAGS=["-fPIC"])
-    prog = env.add_shared_library("#bin/godot", common_linuxbsd + libgodot_files)
+    prog = env.add_shared_library("#bin/godot", common_linuxbsd)
 else:
-    prog = env.add_program("#bin/godot", common_linuxbsd + executable_files)
+    prog = env.add_program("#bin/godot", common_linuxbsd)
 
 if env["debug_symbols"] and env["separate_debug_symbols"]:
     env.AddPostAction(prog, env.Run(platform_linuxbsd_builders.make_debug_linuxbsd))