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

[GDExtension] Add config option to specify custom library lookup paths. #88049

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+89 −4
Open

[GDExtension] Add config option to specify custom library lookup paths. #88049

Show file tree
Hide file tree
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
8 changes: 8 additions & 0 deletions core/config/project_settings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ String ProjectSettings::get_resource_path() const {
return resource_path;
}

String ProjectSettings::get_main_pack_path() const {
return main_pack_path;
}

String ProjectSettings::get_imported_files_path() const {
return get_project_data_path().path_join("imported");
}
Expand Down Expand Up @@ -477,6 +481,10 @@ bool ProjectSettings::_load_resource_pack(const String &p_pack, bool p_replace_f
return false;
}

if (main_pack_path.is_empty()) {
main_pack_path = p_pack.get_base_dir();
}

//if data.pck is found, all directory access will be from here
DirAccess::make_default<DirAccessPack>(DirAccess::ACCESS_RESOURCES);
using_datapack = true;
Expand Down
2 changes: 2 additions & 0 deletions core/config/project_settings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ class ProjectSettings : public Object {

RBMap<StringName, VariantContainer> props; // NOTE: Key order is used e.g. in the save_custom method.
String resource_path;
String main_pack_path;
HashMap<StringName, PropertyInfo> custom_prop_info;
bool using_datapack = false;
bool project_loaded = false;
Expand Down Expand Up @@ -176,6 +177,7 @@ class ProjectSettings : public Object {
String get_project_data_dir_name() const;
String get_project_data_path() const;
String get_resource_path() const;
String get_main_pack_path() const;
String get_imported_files_path() const;

static ProjectSettings *get_singleton();
Expand Down
43 changes: 43 additions & 0 deletions core/extension/gdextension.compat.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/**************************************************************************/
/* gdextension.compat.inc */
/**************************************************************************/
/* This file is part of: */
/* GODOT ENGINE */
/* https://godotengine.org */
/**************************************************************************/
/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */
/* */
/* Permission is hereby granted, free of charge, to any person obtaining */
/* a copy of this software and associated documentation files (the */
/* "Software"), to deal in the Software without restriction, including */
/* without limitation the rights to use, copy, modify, merge, publish, */
/* distribute, sublicense, and/or sell copies of the Software, and to */
/* permit persons to whom the Software is furnished to do so, subject to */
/* the following conditions: */
/* */
/* The above copyright notice and this permission notice shall be */
/* included in all copies or substantial portions of the Software. */
/* */
/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */
/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */
/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */
/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */
/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */
/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */
/**************************************************************************/

#ifndef DISABLE_DEPRECATED

#include "gdextension.h"
Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary


Error GDExtension::_open_library_bind_compat_88049(const String &p_path, const String &p_entry_symbol) {
return open_library(p_path, p_entry_symbol, Vector<String>());
}

void GDExtension::_bind_compatibility_methods() {
ClassDB::bind_compatibility_method(D_METHOD("open_library", "path", "entry_symbol"), &GDExtension::_open_library_bind_compat_88049);
}
Comment on lines +39 to +41
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no longer necessary. After PR #88418, GDExtension::open_library() isn't exposed anymore.


#endif
24 changes: 21 additions & 3 deletions core/extension/gdextension.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include "core/object/method_bind.h"
#include "core/os/os.h"
#include "core/version.h"
#include "gdextension.compat.inc"
Copy link
Member

Choose a reason for hiding this comment

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

To follow current include conventions, this should be:

#include "gdextension.h"
#include "gdextension.compat.inc"

#include "core/config/project_settings.h"
#include "core/extension/gdextension_manager.h"
#include "core/io/dir_access.h"
#include "core/object/class_db.h"
#include "core/object/method_bind.h"
#include "core/os/os.h"
#include "core/version.h"

#include "gdextension_manager.h"

extern void gdextension_setup_interface();
Expand Down Expand Up @@ -679,7 +680,7 @@ GDExtensionInterfaceFunctionPtr GDExtension::get_interface_function(const String
return *function;
}

Error GDExtension::open_library(const String &p_path, const String &p_entry_symbol) {
Error GDExtension::open_library(const String &p_path, const String &p_entry_symbol, const Vector<String> &p_lookup_paths) {
String abs_path = ProjectSettings::get_singleton()->globalize_path(p_path);
#if defined(WINDOWS_ENABLED) && defined(TOOLS_ENABLED)
// If running on the editor on Windows, we copy the library and open the copy.
Expand Down Expand Up @@ -716,6 +717,22 @@ Error GDExtension::open_library(const String &p_path, const String &p_entry_symb
}
#endif

if (!p_lookup_paths.is_empty() && !FileAccess::exists(abs_path) && !DirAccess::exists(abs_path)) {
// Try using custom lookup paths.
for (const String &dir : p_lookup_paths) {
String lookup_dir = dir;
lookup_dir = lookup_dir.begins_with("pck_dir://") ? lookup_dir.replace_first("pck_dir:/", ProjectSettings::get_singleton()->get_main_pack_path()) : lookup_dir;
lookup_dir = lookup_dir.begins_with("bundle_dir://") ? lookup_dir.replace_first("bundle_dir:/", OS::get_singleton()->get_bundle_resource_dir()) : lookup_dir;
lookup_dir = lookup_dir.begins_with("exe_dir://") ? lookup_dir.replace_first("exe_dir:/", OS::get_singleton()->get_executable_path().get_base_dir()) : lookup_dir;
Comment on lines +724 to +726
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to replace this with the custom path mapping from PR #87586 after that is merged, so that these paths aren't exclusive to GDExtension loading. Perhaps a TODO should be added here?

Also, are we sure we want to commit to these specific prefix names? Not that I want to spend lots of time bike-shedding the names :-) but we will be committing to them as part of our external API, so they probably should go through a little bit wider discussion.

lookup_dir = lookup_dir.path_join(abs_path.get_file());
lookup_dir = ProjectSettings::get_singleton()->globalize_path(lookup_dir);
if (FileAccess::exists(lookup_dir) || DirAccess::exists(lookup_dir)) {
abs_path = lookup_dir;
break;
}
}
}

Error err = OS::get_singleton()->open_dynamic_library(abs_path, library, true, &library_path);
ERR_FAIL_COND_V_MSG(err == ERR_FILE_NOT_FOUND, err, "GDExtension dynamic library not found: " + abs_path);
ERR_FAIL_COND_V_MSG(err != OK, err, "Can't open GDExtension dynamic library: " + abs_path);
Expand Down Expand Up @@ -801,7 +818,7 @@ void GDExtension::deinitialize_library(InitializationLevel p_level) {
}

void GDExtension::_bind_methods() {
ClassDB::bind_method(D_METHOD("open_library", "path", "entry_symbol"), &GDExtension::open_library);
ClassDB::bind_method(D_METHOD("open_library", "path", "entry_symbol", "lookup_paths"), &GDExtension::open_library, DEFVAL(Vector<String>()));
ClassDB::bind_method(D_METHOD("close_library"), &GDExtension::close_library);
ClassDB::bind_method(D_METHOD("is_library_open"), &GDExtension::is_library_open);

Expand Down Expand Up @@ -932,8 +949,9 @@ Error GDExtensionResourceLoader::load_gdextension_resource(const String &p_path,
FileAccess::get_modified_time(p_path),
FileAccess::get_modified_time(library_path));
#endif
Vector<String> lookup_paths = config->get_value("configuration", "lookup_paths", Vector<String>());

err = p_extension->open_library(is_static_library ? String() : library_path, entry_symbol);
err = p_extension->open_library(is_static_library ? String() : library_path, entry_symbol, lookup_paths);
if (err != OK) {
#if defined(WINDOWS_ENABLED) && defined(TOOLS_ENABLED)
// If the DLL fails to load, make sure that temporary DLL copies are cleaned up.
Expand Down
8 changes: 7 additions & 1 deletion core/extension/gdextension.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ class GDExtension : public Resource {
static HashMap<StringName, GDExtensionInterfaceFunctionPtr> gdextension_interface_functions;

protected:
#ifndef DISABLE_DEPRECATED
Error _open_library_bind_compat_88049(const String &p_path, const String &p_entry_symbol);

static void _bind_compatibility_methods();
#endif

static void _bind_methods();

public:
Expand All @@ -120,7 +126,7 @@ class GDExtension : public Resource {
static String get_extension_list_config_file();
static String find_extension_library(const String &p_path, Ref<ConfigFile> p_config, std::function<bool(String)> p_has_feature, PackedStringArray *r_tags = nullptr);

Error open_library(const String &p_path, const String &p_entry_symbol);
Error open_library(const String &p_path, const String &p_entry_symbol, const Vector<String> &p_lookup_paths = Vector<String>());
void close_library();

#if defined(WINDOWS_ENABLED) && defined(TOOLS_ENABLED)
Expand Down
1 change: 1 addition & 0 deletions doc/classes/GDExtension.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
<return type="int" enum="Error" />
<param index="0" name="path" type="String" />
<param index="1" name="entry_symbol" type="String" />
<param index="2" name="lookup_paths" type="PackedStringArray" default="PackedStringArray()" />
Copy link
Member

Choose a reason for hiding this comment

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

Would be good to document it.

<description>
Opens the library at the specified [param path].
[b]Note:[/b] You normally should not call this method directly. This is handled automatically by [method GDExtensionManager.load_extension].
Expand Down
7 changes: 7 additions & 0 deletions misc/extension_api_validation/4.2-stable.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,10 @@ Validate extension JSON: Error: Field 'classes/GPUParticles3D/properties/process
Validate extension JSON: Error: Field 'classes/Sky/properties/sky_material': type changed value in new API, from "ShaderMaterial,PanoramaSkyMaterial,ProceduralSkyMaterial,PhysicalSkyMaterial" to "PanoramaSkyMaterial,ProceduralSkyMaterial,PhysicalSkyMaterial,ShaderMaterial".

Property hints reordered to improve editor usability. The types allowed are still the same as before. No adjustments should be necessary.


GH-88049
--------
Validate extension JSON: Error: Field 'classes/GDExtension/methods/open_library/arguments': size changed value in new API, from 2 to 3.

Added optional "lookup_paths" argument. Compatibility method registered.
Loading