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

Dependencies between GDExtensions #615

Open
kang-sw opened this issue Feb 20, 2024 · 11 comments
Open

Dependencies between GDExtensions #615

kang-sw opened this issue Feb 20, 2024 · 11 comments
Labels
feature Adds functionality to the library status: upstream Depending on upstream fix (typically Godot)

Comments

@kang-sw
Copy link
Contributor

kang-sw commented Feb 20, 2024

Out of mere curiosity, I wonder if there's any possibility in the future to implement dependencies between GdExtension binaries written in different languages? Being relatively new to exploring this repository, I'm not fully acquainted with the technical specifics. However, it appears that all GdExtensions register classes and methods with the engine. If the methods registered by GdExtensions are treated no differently from those of internal engine classes, wouldn't it be possible for the engine to accept classes registered by other GdExtensions at the time a Rust Extension is registered?

Of course, it's not possible to know which classes a GdExtension will register at compile time, but could we not retrieve metadata for classes already registered with the engine when a Rust GdExtension loads at runtime (based on some user-configurable flag setting), recreate bindings, and, based on this, support Rust bindings for other GdExtension Classes in the next iteration?

I'm not entirely sure, but I presume there must be a way within the engine to specify the load order between different GdExtensions. Consequently, unless a user updates their plugins, the bindings themselves would likely be deterministically generated.

@kang-sw kang-sw changed the title Dependency among gdExtensions Dependency among GdExtensions Feb 20, 2024
@GsLogiMaker
Copy link

I've been thinking about this exact issue. I've not attempted implementing this, but theoretically if you provide a header for your GDExtension API then another compiled language could use that to interface with it.

Alternatively, you could use Godot objects and use the call function to use any exposed methods. This would be the simplest solution and works for any language, but you would be sacrificing type safety and maybe performance.

@Bromeon Bromeon added feature Adds functionality to the library status: upstream Depending on upstream fix (typically Godot) labels Feb 20, 2024
@Bromeon
Copy link
Member

Bromeon commented Feb 20, 2024

This might be more something for a Godot proposal than for godot-rust 🙂

But yes, we had this discussion in the GDExtension team, and I agree it would be quite valuable.
The good news are, this is already partially supported, even though quite manual:

  1. Navigate to a Godot project directory, with a project that uses extensions (i.e. has one or more .gdextension files).
  2. Invoke Godot 4 on the command line, as: godot4 --headless --dump-extension-api
  3. The resulting extension_api.json contains also symbols from the extensions.

Now, I haven't tried using that extension_api.json as input for gdext, and manually overwriting the file would likely be brittle. What you could try however, is setting GODOT4_BIN only inside the project and making sure the Godot executable is run in the working directory of the project.godot file. That way, it should pick up additions from extension.

Maybe we can think about ways to support this workflow better from gdext.
Don't hesitate to open a Godot proposal to get the ball rolling for a proper feature in that regard.

@Bromeon
Copy link
Member

Bromeon commented Feb 20, 2024

Maybe worth noting, GDExtension has not really been designed for a full ecosystem, meaning there will be several challenges:

  1. I'm not sure if the load order of extensions is deterministic.
    • There is the file .godot/extension_list.cfg which could possibly be used to override the ordering.
  2. There is no package or dependency manager for extensions, no central repository and no versioning scheme.
    • People would need to ensure compatibility by hand or through custom scripts.
    • Cargo could help for Rust extensions, but likely, the majority of extensions will be written in C++. And then there's Go, Swift, Python, Kotlin... consolidating all the build systems is near-impossible unless you go for heavyweight solutions like Bazel/Buck2/...
    • It will be hard to establish any conventions (on source repos or versions) unless they are part of official GDExtension itself.
  3. Godot has no namespacing. If two extensions define the same class, they can't be loaded together.
    • Something like prefixes might help, but it may also make usage more complex.

That said, we don't need to go the full route to be useful, even manually depending on extensions that were hand-selected could be a nice addition.

@kang-sw
Copy link
Contributor Author

kang-sw commented Feb 20, 2024

GDExtension has not really been designed for a full ecosystem

Aha, that was why I need to look for Godot proposals for this ... Thanks, now that makes a lot of sense! My curiosity is fully resolved; may I close this issue?

@Bromeon
Copy link
Member

Bromeon commented Feb 20, 2024

You can gladly start a discussion in a proposal, but maybe we can keep this open for now, for "bridge-the-gap" solutions on godot-rust side 🙂

@Bromeon Bromeon changed the title Dependency among GdExtensions Dependencies between GDExtensions Oct 28, 2024
@PikaDude
Copy link

After some guidance from @Bromeon, I tried using the api-custom feature of the crate to use https://github.com/MizunagiKB/gd_cubism, but immediately encountered an issue. Due to this line, it generates the extension_api.json using the Godot binary, but the working directory is set to the directory of the json output, which is the environment variable OUT_DIR.

cmd.current_dir(cwd)
This isn't ideal, because then the working directory won't be of the Godot project containing the GDExtension I wish to utilise.

After manually hardcoding the correct working directory, the next issue was fixing the error "class GDCubismEffect has unknown API type extension" being returned by this function

pub fn get_api_level(class: &JsonClass) -> ClassCodegenLevel {
// Work around wrong classification in https://github.com/godotengine/godot/issues/86206.
fn override_editor(class_name: &str) -> bool {
cfg!(before_api = "4.3")
&& matches!(
class_name,
"ResourceImporterOggVorbis" | "ResourceImporterMP3"
)
}
if special_cases::is_class_level_server(&class.name) {
ClassCodegenLevel::Servers
} else if class.api_type == "editor" || override_editor(&class.name) {
ClassCodegenLevel::Editor
} else if class.api_type == "core" {
ClassCodegenLevel::Scene
} else {
panic!(
"class {} has unknown API type {}",
class.name, class.api_type
)
}
}
I mapped "extension" to Scene thanks to @Bromeon's advice.

After that, I was able to build my godot-rust extension, but upon trying to use it with the following code, I got the following error in Godot.

use godot::classes::{GdCubismUserModel, IGdCubismUserModel};
use godot::prelude::*;

#[derive(GodotClass)]
#[class(base=GdCubismUserModel)]
struct MyModel {
    base: Base<GdCubismUserModel>,
}

#[godot_api]
impl IGdCubismUserModel for MyModel {
    fn init(base: Base<GdCubismUserModel>) -> Self {
        godot_print!("Hello, world!");

        Self { base }
    }
}
  core/extension/gdextension.cpp:471 - Unimplemented yet
  Extension runtime class MyModel cannot descend from GDCubismUserModel which isn't also a runtime class
  godot-core\src\registry\class.rs:414 - Failed to register class `MyModel`; check preceding Godot stderr messages.

From here this is completely out of the range of my knowledge, hopefully someone can figure this out and get this working 🙏

@0x53A
Copy link
Contributor

0x53A commented Oct 28, 2024

Extension runtime class MyModel cannot descend from GDCubismUserModel which isn't also a runtime class

I understand it as, extension classes are not allowed to inherit from extension classes from a different extension. That is, a rust_gdext class can inherit from either a built-in (aka runtime) class (Node2D) or a rust_gdext class; but NOT from a gd_cubism class.

	if (self->extension_classes.has(parent_class_name)) {
		parent_extension = &self->extension_classes[parent_class_name];
	} else if (ClassDB::class_exists(parent_class_name)) {
		if (ClassDB::get_api_type(parent_class_name) == ClassDB::API_EXTENSION || ClassDB::get_api_type(parent_class_name) == ClassDB::API_EDITOR_EXTENSION) {
			ERR_PRINT("Unimplemented yet");
			//inheriting from another extension
		} else {
			//inheriting from engine class
		}

https://github.com/godotengine/godot/blob/a3080477ac0421aef24ca0916c40559abbf4846b/core/extension/gdextension.cpp#L363-L374

So what you're looking to do just seems plain impossible without changes to godot itself.

Note that you could look into composition instead of inheritance, that is, instead of inheriting from GdCubismUserModel, have a child node of type GdCubismUserModel (I don't know if that would work)

@Yarwin
Copy link
Contributor

Yarwin commented Oct 29, 2024

One more thing – for some reason codegen for resources (and as far as I'm aware only the resources?) derived by way explained by PikaDude returns "c_void" for its set_s/setters, which causes build to fail.

error[E0277]: the trait bound `std::ffi::c_void: godot_convert::FromGodot` is not satisfied
   --> /home/irwin/apps/godot/bullet-hell-shit/bullet-hell-rust/target/debug/build/godot-core-f3565041a3c07d75/out/classes/projectile_config.rs:139:17
    |
139 |                 < CallSig as PtrcallSignatureTuple > ::out_class_ptrcall(method_bind, "ProjectileConfig", "set_display_type", self.object_ptr, self.__checked_id(), args,)
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `godot_convert::FromGodot` is not implemented for `std::ffi::c_void`, which is required by `(std::ffi::c_void, i32): signature::PtrcallSignatureTuple`
    |
    = help: the following other types implement trait `godot_convert::FromGodot`:
              *const std::ffi::c_void
              *mut std::ffi::c_void
note: required for `(std::ffi::c_void, i32)` to implement `signature::PtrcallSignatureTuple`
   --> /home/irwin/apps/godot/opensource-contr/missing_docs/gdext/godot-core/src/meta/signature.rs:350:28
    |
350 |         impl<$R, $($Pn,)*> PtrcallSignatureTuple for ($R, $($Pn,)*)
    |                            ^^^^^^^^^^^^^^^^^^^^^     ^^^^^^^^^^^^^^
351 |             where $R: ToGodot + FromGodot + Debug,
    |                                 --------- unsatisfied trait bound introduced here
...
597 | impl_ptrcall_signature_for_tuple!(R, (p0, 0): P0);
    | ------------------------------------------------- in this macro invocation
    = note: this error originates in the macro `impl_ptrcall_signature_for_tuple` (in Nightly builds, run with -Z macro-backtrace for more info)

image

@Bromeon
Copy link
Member

Bromeon commented Oct 29, 2024

@PikaDude could you maybe upload the custom extension_api.json?

@PikaDude
Copy link

@PikaDude could you maybe upload the custom extension_api.json?

The one I had made for gd_cubism? Sure.
extension_api.json

@Bromeon
Copy link
Member

Bromeon commented Jan 12, 2025

Important to consider the Wasm impacts. #973 is a PR with a detailed explanation of why the current approach doesn't support multiple Rust extensions in WASM. I don't think the proposed solution is mergeable as-is, but it's good at showing some problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Adds functionality to the library status: upstream Depending on upstream fix (typically Godot)
Projects
None yet
Development

No branches or pull requests

6 participants