Allow to generate API from user-provided JSON and headers via `api-custom-json` feature. #1124

Yarwin · 2025-04-15T08:28:49Z

What problem does this PR solve?

Allows to perform the codegen without godot binary, i.e. only with extension_api.json and extension_interface.h (with paths to both provided by GODOT4_GDEXTENSION_JSON and GODOT4_GDEXTENSION_HEADERS env variables respectively).

Godot version is being parsed from header key in provided JSON file; we can force users to provide env variable instead (or do both - env variable & parsing of JSON file as a failsafe 🤷 ).

What has to be done

Not sure how to test it, should I create action in pipeline?

GodotRust · 2025-04-15T08:40:08Z

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1124

godot-bindings/src/godot_exe.rs

godot-bindings/src/godot_json.rs

Bromeon

Thanks a lot, this is a great feature! 👍

Would it be possible to keep the whole JSON parsing (and nanoserde dependency) in godot-codegen crate? Everything you need should already be there, and godot-bindings is mostly a "glue code" crate that links the different parts together.

Not sure how to test it, should I create action in pipeline?

In the godot-itest composite action, we could add an input godot-indirect-json (false by default).

When set to true, it would manually call --dump-extension-api via bash script, and compile godot-rust with the api-custom-json feature against the generated JSON file, setting the correct environment.

We could then add a new job (maybe just basic Linux one) to use that. Possibly over time we could combine it with another, but initially it probably makes sense to isolate it, so we see any issues regarding api-custom-json immediately.

Bromeon · 2025-04-15T09:07:26Z

godot-bindings/src/godot_json.rs

+// GDExtension JSON can get excessively big, so we extract only the very first key – the header.
+fn extract_gdextension_json_header_section(
+    mut lines: impl Iterator<Item = Result<String, std::io::Error>>,
+) -> String {
+    let mut header = String::new();
+    'outer: loop {
+        let Some(Ok(line)) = lines.next() else {
+            panic!("Failed to extract \"header\" section from provided GDExtension JSON.")
+        };
+
+        // Keep parsing until we stumble on "header" key.
+        if !line.contains("\"header\"") {
+            continue;
+        }
+        header.push('{');
+
+        // Parse header section, assuming its structure is the same as one in `GDExtensionJSONHeader`.
+        '_parse_header: loop {
+            let Some(Ok(line)) = lines.next() else {
+                panic!("Failed to extract \"header\" section from provided GDExtension JSON.");
+            };
+            if line.contains("}") {
+                header.push('}');
+                break 'outer;
+            }
+            header.push_str(&line);
+        }
+    }
+
+    header
+}


This has quite a few assumptions about the JSON structure, which aren't guaranteed by Godot and may change in the future:

headers is near the beginning (otherwise this becomes inefficient)

there is no occurrence of "header" anywhere else

{ and } are only used as JSON delimiters (while they might occur in strings)

possibly more regarding the line-based approach, since JSON isn't necessarily structured in lines

Keep also in mind that in the future, #615 might be implemented on Godot side, in a way that extensions would write their own JSON API spec, that is then consumed by others. As long as the JSON structure is correct (i.e. keys and values), then that's a valid API spec.

Given this is only an optimization (unless I'm missing something?), it might be more robust to use general nanoserde, which will also keep the code simpler. In the happy path, the whole spec needs to be parsed anyway, and I'm OK to accept small performance waste in error cases (e.g. version mismatch). That aside, JSON parsing in Rust Release mode is blazing fast already.

In this case providing Godot version should be an user responsibility – since proper deserialization might be impossible before knowing the version beforehand (in other words – paragraph 22, we won't be able to parse the gdextension api json if we won't know the version it came from, and we won't know the version before we extract it from said JSON).

We can optimistically parse the JSON -- our models should be backwards-compatible to any extension_api.json starting with Godot 4.1. You're right that we may not detect older versions, but other than that, should be OK?

we won't be able to parse the gdextension api json if we won't know the version it came from

Why not? There is currently one #[cfg] in the models, and we can just remove that:

gdext/godot-codegen/src/models/json.rs

Lines 222 to 223 in 0289637

#[cfg(since_api = "4.4")]

pub is_required: Option<bool>, // Only virtual functions have this field.

I settled on minimal deserialization (only the header) mostly because of a cyclic dependency (godot-codegen relies on godot-bindings); I included comment with reasoning as well.

I tried moving json.rs to godot-bindings from godot-codegen and IMO the result was a bit ugly and confusing + I would rather avoid huge (non necessary) refactors without proper consultation/plan while implementing some other feature (especially since I haven't messed too much with codegen)

I'll implement the test action for pipeline before Friday (18.04.2025).

OK; could you add 1 line in the Cargo.toml over the nanoserde dep that the reasoning is elaborated in godot_json.rs? 🙂

Bromeon

Thanks for the update!

I'm not sure about GODOT4_GDEXTENSION_HEADER(S). It might be useful for someone having a newer version than the release build, but for someone just using a modified Godot build (e.g. certain modules disabled), there isn't really any freedom in how the C API can change:

extra declarations are ignored by godot-rust
changed declarations lead to compile errors or runtime UB

So the only case where providing that file is useful is when the latest .h shipped by godot-rust is older than the Godot in-dev version (or someone drastically modifies the engine). While we might want to support this case, I'm not sure if we should require the .h file to be provided.

When the .h is absent, I was first thinking about looking at the version specified in the JSON, or the api-4-* feature, but it's probably much simpler: just use the latest known gdextension_interface.h file. The C API must be backwards-compatible because the whole compatibility promise of GDExtension falls apart otherwise. So newer features can remain unused, but should work alongside older JSONs.

Bromeon · 2025-04-15T19:52:39Z

godot-bindings/src/godot_json.rs

+
+pub fn load_custom_gdextension_json(watch: &mut StopWatch) -> String {
+    let path = std::env::var("GODOT4_GDEXTENSION_JSON")
+        .expect("gdext with `api-custom-json` feature requires GODOT4_GDEXTENSION_JSON environment variable (with the path to the said json).");


Suggested change

.expect("gdext with `api-custom-json` feature requires GODOT4_GDEXTENSION_JSON environment variable (with the path to the said json).");

.expect("godot-rust with `api-custom-json` feature requires GODOT4_GDEXTENSION_JSON \

environment variable (with the path to the extension_api.json file).");

Bromeon · 2025-04-15T19:53:52Z

godot-bindings/src/godot_json.rs

+    watch: &mut StopWatch,
+) {
+    let path = std::env::var("GODOT4_GDEXTENSION_HEADERS")
+        .expect("gdext with `api-custom-json` feature requires GODOT4_GDEXTENSION_HEADERS environment variable (with the path to the said headers).");


Suggested change

.expect("gdext with `api-custom-json` feature requires GODOT4_GDEXTENSION_HEADERS environment variable (with the path to the said headers).");

.expect("gdext with `api-custom-json` feature requires GODOT4_GDEXTENSION_HEADER \

environment variable (with the path to gdextension_interface.h).");

(should also be ...HEADER, not ...HEADERS)

Bromeon · 2025-04-15T19:55:54Z

godot/src/lib.rs

-compile_error!("The feature `double-precision` currently requires `api-custom` due to incompatibilities in the GDExtension API JSON.");
+#[cfg(all(
+    feature = "double-precision",
+    all(not(feature = "api-custom"), not(feature = "api-custom-json"))


Suggested change

all(not(feature = "api-custom"), not(feature = "api-custom-json"))

any(not(feature = "api-custom"), not(feature = "api-custom-json"))

nope, all – running double precision without api-custom OR api-custom-json features enabled should result in compile error (i.e. both/all features are off)

Then there's no point in nesting all though; just have 3 conditions in the outer all 😉

Bromeon · 2025-04-15T19:56:50Z

godot/src/lib.rs

+    feature = "double-precision",
+    all(not(feature = "api-custom"), not(feature = "api-custom-json"))
+))]
+compile_error!("The feature `double-precision` currently requires `api-custom` or `api-custom-json` due to incompatibilities in the GDExtension API JSON.");


"due to incompatibilities in the GDExtension API JSON" is a bit vague -- if we mention it, we should probably elaborate slightly. It's also fine to omit it entirely.

I added link to aforementioned issue: godotengine/godot#86346, the question is – is it still relevant 🤔?

Bromeon · 2025-04-15T20:02:27Z

godot-bindings/src/godot_json.rs

+// GDExtension JSON can get excessively big, so we extract only the very first key – the header.
+fn extract_gdextension_json_header_section(
+    mut lines: impl Iterator<Item = Result<String, std::io::Error>>,
+) -> String {
+    let mut header = String::new();
+    'outer: loop {
+        let Some(Ok(line)) = lines.next() else {
+            panic!("Failed to extract \"header\" section from provided GDExtension JSON.")
+        };
+
+        // Keep parsing until we stumble on "header" key.
+        if !line.contains("\"header\"") {
+            continue;
+        }
+        header.push('{');
+
+        // Parse header section, assuming its structure is the same as one in `GDExtensionJSONHeader`.
+        '_parse_header: loop {
+            let Some(Ok(line)) = lines.next() else {
+                panic!("Failed to extract \"header\" section from provided GDExtension JSON.");
+            };
+            if line.contains("}") {
+                header.push('}');
+                break 'outer;
+            }
+            header.push_str(&line);
+        }
+    }
+
+    header
+}


OK; could you add 1 line in the Cargo.toml over the nanoserde dep that the reasoning is elaborated in godot_json.rs? 🙂

Bromeon · 2025-04-15T20:03:11Z

godot-bindings/src/godot_json.rs

+use crate::depend_on_custom_json::header_gen::{generate_rust_binding, patch_c_header};
+use crate::{GodotVersion, StopWatch};
+use std::fs;
+use std::path::Path;
+
+use crate::godot_version::validate_godot_version;
+use nanoserde::DeJson;


If you remove the empty line, they can all be sorted.

Bromeon · 2025-04-15T20:05:14Z

godot-bindings/src/godot_json.rs

+// At first re-using mapping from godot-codegen json.rs might seem more desirable but there are few issues to consider:
+// * Overall JSON file structure might change slightly from version to version, while header should stay consistent (otherwise it defeats the purpose of having any header at all).
+// Having two parsers – minimal one inherent to api-custom-json feature and codegen one – makes handling all the edge cases easier.
+// * `godot-codegen` depends on `godot-bindings` thus simple re-using types from former in side the latter is not possible (cyclic dependency).
+// Moving said types to `godot-bindings` would increase the cognitive overhead (since domain mapping is responsibility of `godot-codegen`, while godot-bindings is responsible for providing required resources & emitting the version).
+// In the future we might experiment with splitting said types into separate crates.


This should come at the beginning of the file, after //! (can still stay in separate // paragraph, just before imports).

Yarwin · 2025-04-17T07:50:22Z

All aforementioned suggestions has been applied; api-custom-json uses the latest headers by default while GODOT4_GDEXTENSION_HEADERS can be optionally set if someone uses modified version of the engine.

The only thing left is creating proper github action for testing.

- Implement codegen from user-provided json (`godot_json.rs`, similar to `godot_exe.rs`) - Add `linux-custom-api-json` job to pipeline - Use the latest GDExtension headers by default, but allow users to specify their own in case if they are using their own, modified version of the engine. - Allow to use `double-precision` feature with an `api-custom-json`.

Yarwin · 2025-04-20T05:31:49Z

New job added to both pipelines and commits has been squashed

Yarwin added feature Adds functionality to the library c: tooling CI, automation, tools labels Apr 15, 2025

Yarwin force-pushed the generating-api-from-user-provided-json branch 2 times, most recently from 0ff22cc to 44ae350 Compare April 15, 2025 08:36

Yarwin force-pushed the generating-api-from-user-provided-json branch 2 times, most recently from 00740ad to 83c08b8 Compare April 15, 2025 08:53

Yarwin commented Apr 15, 2025

View reviewed changes

godot-bindings/src/godot_exe.rs Outdated Show resolved Hide resolved

Yarwin commented Apr 15, 2025

View reviewed changes

godot-bindings/src/godot_json.rs Outdated Show resolved Hide resolved

Bromeon reviewed Apr 15, 2025

View reviewed changes

Yarwin force-pushed the generating-api-from-user-provided-json branch from 0f24542 to 9b20a3d Compare April 19, 2025 11:49

Bromeon added this to the 0.3.x milestone Apr 19, 2025

Yarwin force-pushed the generating-api-from-user-provided-json branch from 9b20a3d to ad11cfe Compare April 19, 2025 19:39

	#[cfg(since_api = "4.4")]
	pub is_required: Option<bool>, // Only virtual functions have this field.

	.expect("gdext with `api-custom-json` feature requires GODOT4_GDEXTENSION_JSON environment variable (with the path to the said json).");
	.expect("godot-rust with `api-custom-json` feature requires GODOT4_GDEXTENSION_JSON \
	environment variable (with the path to the extension_api.json file).");

	.expect("gdext with `api-custom-json` feature requires GODOT4_GDEXTENSION_HEADERS environment variable (with the path to the said headers).");
	.expect("gdext with `api-custom-json` feature requires GODOT4_GDEXTENSION_HEADER \
	environment variable (with the path to gdextension_interface.h).");

	all(not(feature = "api-custom"), not(feature = "api-custom-json"))
	any(not(feature = "api-custom"), not(feature = "api-custom-json"))

Uh oh!

Allow to generate API from user-provided JSON and headers via api-custom-json feature. #1124

Are you sure you want to change the base?

Allow to generate API from user-provided JSON and headers via api-custom-json feature. #1124

Uh oh!

Conversation

Yarwin commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What has to be done

Uh oh!

GodotRust commented Apr 15, 2025

Uh oh!

Uh oh!

Uh oh!

Bromeon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Yarwin Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bromeon left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bromeon Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Yarwin commented Apr 17, 2025

Uh oh!

Yarwin commented Apr 20, 2025

Uh oh!

Uh oh!

Allow to generate API from user-provided JSON and headers via `api-custom-json` feature. #1124

Allow to generate API from user-provided JSON and headers via `api-custom-json` feature. #1124

Yarwin commented Apr 15, 2025 •

edited

Loading

Yarwin Apr 15, 2025 •

edited

Loading

Bromeon left a comment •

edited

Loading

Bromeon Apr 15, 2025 •

edited

Loading