[GDExtension] Add config option to specify custom library lookup paths. #88049
Conversation
bruvzg
force-pushed
the
gde_custom_load_path
branch
2 times, most recently
from
e6711f8 to
c0c81e1
Compare
bruvzg
force-pushed
the
gde_custom_load_path
branch
from
c0c81e1 to
ba6959f
Compare
| @@ -35,6 +35,7 @@ | |||
| #include "core/object/method_bind.h" | |||
| #include "core/os/os.h" | |||
| #include "core/version.h" | |||
| #include "gdextension.compat.inc" | |||
There was a problem hiding this comment.
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"
| @@ -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()" /> | |||
|
CC @godotengine/gdextension |
|
|
||
| #ifndef DISABLE_DEPRECATED | ||
|
|
||
| #include "gdextension.h" |
There was a problem hiding this comment.
Thanks!
This makes sense to me at a high-level. My main concern is just being sure that we want to expose these particular paths and their names.
For example: Will developers understand what bundle_dir:// is (I'm not totally sure I understand what it is :-))? Will they understand how pck_dir:// is different than exe_dir://?
| void GDExtension::_bind_compatibility_methods() { | ||
| ClassDB::bind_compatibility_method(D_METHOD("open_library", "path", "entry_symbol"), &GDExtension::_open_library_bind_compat_88049); | ||
| } |
There was a problem hiding this comment.
This is no longer necessary. After PR #88418, GDExtension::open_library() isn't exposed anymore.
| 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; |
There was a problem hiding this comment.
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.
Fixes #87761
Add an optional
lookup_pathsGDExtension config file parameter to specify a list of custom library lookup paths. Paths can be relativeres://path, absolute path, or relative to the other special project locations (bundle_dir://,pck_dir://,exe_dir://).In case of #87761, the issue can be resolved by adding this line to the config:
compatibility_minimum = "4.1" +lookup_paths = [ ""pck_dir://" ] [libraries]