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

Let gdextension_dir function as standalone argument #1258

Merged
merged 1 commit into from
Oct 19, 2023

Conversation

Repiteo
Copy link
Contributor

@Repiteo Repiteo commented Oct 2, 2023

Right now, if you have a custom build of Godot to build against, you have to redundantly provide arguments for both a gdextension_dir and custom_api_file. This changes it so that, if only gdextension_dir is provided, it will implicitly setup a path for custom_api_file using the default name

@Repiteo Repiteo requested a review from a team as a code owner October 2, 2023 18:35
@Repiteo Repiteo force-pushed the standalone-gdextension_dir branch 2 times, most recently from 3a1fae9 to 30e6c43 Compare October 3, 2023 16:47
@Faless
Copy link
Contributor

Faless commented Oct 15, 2023

Thank you for looking into this, I agree this is a needed change.

I would prefer doing the following though:

diff --git a/tools/godotcpp.py b/tools/godotcpp.py
index 69a4652..6b44272 100644
--- a/tools/godotcpp.py
+++ b/tools/godotcpp.py
@@ -284,8 +284,8 @@ def generate(env):
 
 
 def _godot_cpp(env):
-    api_file = normalize_path(env.get("custom_api_file", env.File("gdextension/extension_api.json").abspath), env)
     extension_dir = normalize_path(env.get("gdextension_dir", env.Dir("gdextension").abspath), env)
+    api_file = normalize_path(env.get("custom_api_file", env.File(extension_dir + "/extension_api.json").abspath), env)
     bindings = env.GodotCPPBindings(
         env.Dir("."),
         [

@Calinou Calinou added enhancement This is an enhancement on the current functionality topic:gdextension This relates to the new Godot 4 extension implementation labels Oct 15, 2023
@capnm

This comment was marked as off-topic.

@dsnopek
Copy link
Collaborator

dsnopek commented Oct 18, 2023

I think this also needs to be rebased on the latest master in order to fix the CI issue

@dsnopek dsnopek added the topic:buildsystem Related to the buildsystem or CI setup label Oct 18, 2023
@Repiteo
Copy link
Contributor Author

Repiteo commented Oct 18, 2023

@Faless That'd certainly be simpler! I'll have double-check and see if that will work as expected, as I recall this variable being particularly finicky for some reason

EDIT: Everything seems to work!

Copy link
Collaborator

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@dsnopek dsnopek merged commit 5c4a7dc into godotengine:master Oct 19, 2023
12 checks passed
@Repiteo Repiteo deleted the standalone-gdextension_dir branch October 19, 2023 14:09
@dsnopek
Copy link
Collaborator

dsnopek commented Oct 23, 2023

Cherry-picked for 4.1 in PR #1281

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is an enhancement on the current functionality topic:buildsystem Related to the buildsystem or CI setup topic:gdextension This relates to the new Godot 4 extension implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants