Add `ClassCodegenLevel::Core` #1289

ogapo · 2025-08-27T13:29:24Z

mostly just mimic'd the setup for server classes
added special_case::is_class_level_core
moved "Object" and "OpenXRExtensionWrapperExtension" based on a previous "TODO" comment
added "ProjectSettings" to the list as it seems to also quallify (also editing project settings in Core is currently the only way to affect the initial window creation params)

Unsure if there are any gotchas here, need to do some testing locally (still trying to get my project to use a local gdext build) but wanted to open a draft PR to enable discussion on a concrete change.

tle-oops · 2025-08-27T15:20:38Z

godot-codegen/src/special_cases/special_cases.rs

 #[rustfmt::skip]
 pub fn is_class_level_server(class_name: &str) -> bool {
    // Unclear on if some of these classes should be registered earlier than `Scene`:
    // - `RenderData` + `RenderDataExtension`
    // - `RenderSceneData` + `RenderSceneDataExtension`

    match class_name {
-        // TODO: These should actually be at level `Core`
-        | "Object" | "OpenXRExtensionWrapperExtension" 
-
        // Declared final (un-inheritable) in Rust, but those are still servers.
        | "AudioServer" | "CameraServer" | "NavigationServer2D" | "NavigationServer3D" | "RenderingServer" | "TranslationServer" | "XRServer" 


Currently DisplayServer doesn't seem to be available in InitLevel:Server either - so I'm guessing it should go here?

based on the list within register_server_singletons() on the engine's register_server_types.cpp

@tle-oops great catch, thank you!

@ogapo Could you also add in a comment the link https://github.com/godotengine/godot/blob/master/servers/register_server_types.cpp, and mention both register_server_types + register_server_singletons?

There are a few classes missing:

NativeMenu

DisplayServer

TextServer (not a singleton)

TextServerExtension

probably many more...

Are all the ones listed in register_server_types actually registered at that level?

Many of them aren't exposed as part of Godot's public API, so we can ignore them -- but also many are...

Are all the ones listed in register_server_types actually registered at that level?

Yes it seems to be consistent in terms of the types being registered. Though looking through the initialization order of the types, their singletons and the Init steps for Extensions (on_init_level calls) in godot's main() - the register_xxxxx_singletons() calls are not where they're supposed to be - notably register_core_singletons() and register_server_singletons(). So simply changing those here will likely result in a similar situation where the singletons aren't available in the correct InitLevel.

good news is that ProjectSettings is in "early"_core group and that seems to be correctly registered before the CORE init step.

though this is on the engine's side and not godot-rust

GodotRust · 2025-08-27T17:22:44Z

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

Bromeon · 2025-08-27T17:23:06Z

Could you elaborate the use case a bit? What is now possible that previously wasn't?

ogapo · 2025-08-27T20:42:30Z

@Bromeon So to give you full context here, we are ultimately just trying to parametrize the size and mode of the initial application window Godot creates. This is just to restore a saved position from last run, but I was trying to do it properly (as the window is created) rather than modifying the window after it's begun to render.

It's not ideal, and not strictly related to this PR, but the only way to set those params seems to be to write to the in-memory ProjectSettings before the window is created but after the ProjectSettings are loaded. On the C++ side, this happens very early and ProjectSettings singleton is available by the time gdextension_manager->initialize_extensions(GDExtension::INITIALIZATION_LEVEL_CORE); gets called. However, when we tried to call ProjectSettings::singleton() on the rust side, it hits an assert because ProjectSettings::INIT_LEVEL = crate::init::InitLevel::Scene.

That lead me to dig in and find where this INIT_LEVEL is coming from which seems to be partly based on the extension_api.json but also based on some hardcoded lists in get_api_level() and special_cases.rs. I've updated special_cases.rs to list "ProjectSettings" | "Engine" | "OS" | "Time" as classes available at InitLevel::Core, but doing this required adding some boilerplate since, although InitLevel::Core exists, ClassCodegenLevel only previously had variants for Scene, Servers, and Editor. These classes are the ones explicitly loaded by register_early_core_singletons() on the C++ side.

With all this in place, we are able to call ProjectSettings::singleton() successfully from within our ExtensionLibrary::on_level_init when level = InitLevel::Core.

Bromeon

Thanks a lot, now I understand. This is a very nice addition! 👍

We have a test for init-levels, could you extend that with a small API usage of one of the Core levels?

gdext/itest/rust/src/object_tests/init_level_test.rs

Lines 30 to 40 in 6f894ce

    
           // Run during on the `on_level_init` of the entry point. 
        
           pub fn initialize_init_level_test(level: InitLevel) { 
        
               if level == InitLevel::Servers { 
        
                   assert!(!HAS_RUN.load(Ordering::Acquire)); 
        
                   let mut some_object = SomeObject::new_alloc(); 
        
                   // Need to go through Godot here as otherwise we bypass the failure. 
        
                   some_object.call("set_has_run_true", &[]); 
        
                   some_object.free(); 
        
               } 
        
           }

Perhaps there's an assertion we can find that always holds true. Like two Time.get_ticks_usec() instants being monotonously increasing (>=) and non-zero, or maybe you have a better idea...

And maybe adjust this comment to something more general, like "Test level-specific behavior."

gdext/itest/rust/src/lib.rs

Lines 24 to 25 in 6f894ce

    
           // Testing that we can initialize and use `Object`-derived classes during `Servers` init level. See `object_tests::init_level_test`. 
        
           object_tests::initialize_init_level_test(level);

godot-codegen/src/special_cases/special_cases.rs

ogapo · 2025-08-28T14:43:42Z

@Bromeon I would appreciate a re-review here when you have time. Went down a startup-order rabbit hole heh.

I added a unit test as you mentioned, but I'm concerned the previous test may not have been running (and possibly mine now too). I can't tell how the previous test would have been passing since it was relying on something happening in InitLevel::Servers but fn min_level() -> InitLevel wasn't overridden and the default in the ExtensionLibrary trait is InitLevel::Scene. Unfortunately I don't know enough about the itest framework to tell and the cargo test output doesn't seem to list the test names :-\

Bromeon

First, the current CI failure of not finding ProjectSettings is due to the CI running with a reduced ("minimal") set of available classes, to speed up codegen and compilation times. Classes that are used need to be in this list. ProjectSettings isn't, but OS/Engine/Time are.

Second, for running itests, you don't need cargo test -- see book for instructions 🙂 they will be listed when run. But the code running during startup isn't a real test case -- we would need to see what happens if it panics; ideally it would make CI fail.

What happens if you change the minimum init level?

Bromeon · 2025-08-28T14:56:18Z

godot-codegen/src/special_cases/special_cases.rs

+        | "RenderSceneData" | "RenderSceneDataExtension"
+        => ClassCodegenLevel::Servers,
+        // Declared final (un-inheritable) in Rust, but those are still servers.
+        // NOTE: while these _types_ are available at Server level, the singletons themselves are actually not available until _even after_ Editor level.


This seems like a ticking time bomb -- without singletons, these classes aren't really usable at all, so what does classifying them as Servers achieve in practice?

Also, are you sure that's true? Editor level is never run for exported games, that would mean games couldn't use the AudioServer for example. I think we're missing something here.

I agree, it did not instill confidence when I was looking through Main::setup/setup2. I kept these at ClassCodegenLevel::Servers mostly because that preserved the existing classification before my change and I wasn't sure if maybe something else was dependent on it. It seems weird to me that register_server_singletons() isn't called until well after Scene and Editor mode are initialized but it's right there in setup2 :-.

I don't mean to say it's dependent on Editor level though, just that it runs after editor level init (if it's going to run) and also after Scene init. I've adjusted the documented startup order to reflect that a bit clearer.

/// Initialization order for Godot (see https://github.com/godotengine/godot/blob/master/main/main.cpp) /// - Main::setup() /// - register_core_types() /// - register_early_core_singletons() /// - initialize_extensions(GDExtension::INITIALIZATION_LEVEL_CORE) /// - Main::setup2() /// - register_server_types() /// - initialize_extensions(GDExtension::INITIALIZATION_LEVEL_SERVERS) /// - register_core_singletons() ...possibly a bug. Should this be before LEVEL_SERVERS? /// - register_scene_types() /// - register_scene_singletons() /// - initialize_extensions(GDExtension::INITIALIZATION_LEVEL_SCENE) /// - IF EDITOR /// - register_editor_types() /// - initialize_extensions(GDExtension::INITIALIZATION_LEVEL_EDITOR) /// - register_server_singletons() ...another weird one. /// - Autoloads, etc.

All of this runs before anything like an autoload enters the tree so I'm guessing maybe nobody has been trying to access those servers in the extension init stuff thus far? Either that or I'm completely misreading how register_server_singletons works internally which I suppose is possible.

My takeaway from all this is, there's no way to know unless we test it with some example classes (and make sure it runs correctly in CI, see below).

Even if we analyze Godot's behavior, it's just a temporary snapshot, and the behavior might already be different tomorrow. Plus there's a very good chance we interpret something wrong when just reading code.

Yeah, I agree. This is why I wish the api_level they had in the json was actually which level it's available at. Would be much better to be getting these from godot itself (so it can morph over time).

Re: CI The problem is I'm pretty sure right now there's no level of on_level_init that I could actually test any of the server singletons here based on the C++ startup order. They're not actually available until the first frame after initialization it seems like (at least from the perspective of extensions). Do we want to change their classification to Scene for now or leave it as it was?

Hm, if the singletons only become available after 1 frame, what practical use does the init level have for them? There's no way to call any of their APIs on startup?

Maybe we should check this with Godot devs, seems not very intuitive 🤔

I think it's available on frame 1 but not during any of the extension init calls. I agree though I thought it might be a bug when I saw how late the servers are bound but I could see there may be being a reason I don't grok. The startup sequence seems to be full of magic.

Hm, if we're not sure, we should probably not write something in the comment that may potentially be wrong. Rather say that the order is unclear and would need to be investigated in detail.

I've added a CI test to see if I can get the Rendering Server during Scene init (at the extension level) which runs prior to frame 1. If it fails, I think that implies the conclusion is correct (and maybe this is worth opening an issue on the Godot side?). If it passes, I'll adjust the comment and at least we'll have a test case :-)

Well, looks like it passed. I'm definitely flummoxed. On the C++ side, the call to register_server_singletons(); which includes Engine::get_singleton()->add_singleton(Engine::Singleton("RenderingServer", RenderingServer::get_singleton(), "RenderingServer")); occurs at main.cpp:3450 which seems to be very clearly after the call to GDExtensionManager::get_singleton()->initialize_extensions(GDExtension::INITIALIZATION_LEVEL_SCENE); which occurs at main.cpp:3389

I'll remove the comment about server availability, but it's weird that this seems to be required to access ProjectSettings but not to get RenderingServer. I'm also not sure how to confirm on the Godot side what level these servers are intended to be available at (I thought this was how to tell).

godot-codegen/src/special_cases/special_cases.rs

ogapo · 2025-08-28T21:21:10Z

Second, for running itests, you don't need cargo test -- see book for instructions 🙂 they will be listed when run.

I'm getting an error when I run check.sh

$ ./check.sh itest
./check.sh: line 97: conditional binary operator expected
./check.sh: line 97: syntax error near `godotBin'
./check.sh: line 97: `    if [[ -v godotBin ]]; then'

Looks like bash doesn't like that syntax but I'm not sure why.

But the code running during startup isn't a real test case -- we would need to see what happens if it panics; ideally it would make CI fail.

Not sure I follow. Wouldn't the asserts I added make CI fail?

What happens if you change the minimum init level?

My understanding is it controls whether on_level_init gets called for particular levels. From the docs

/// This will only be invoked for levels >= [`Self::min_level()`], in ascending order. Use `if` or `match` to hook to specific levels.
fn on_level_init(level: InitLevel) {

The trait defaults min_level() to

fn min_level() -> InitLevel {
    InitLevel::Scene
}

so the test that was previously there should have been failing I think since initialize_init_level_test was only running when InitLevel::Servers was specified, and so class_run_during_servers_init ought to have been failing already. This is why I am suspicious if it's actually being run.

Bromeon · 2025-08-28T21:33:28Z

Wouldn't the asserts I added make CI fail?

I don't know, since it's not a real test case, I don't know by heart who catches and handles the panic.
You could verify with assert_eq!(1, 2).

so the test that was previously there should have been failing I think since initialize_init_level_test was only running when InitLevel::Servers was specified, and so class_run_during_servers_init ought to have been failing already. This is why I am suspicious if it's actually being run.

It's not really related to the original task, but do you have time to debug this?

If you get trouble with check.sh, you can also just run:

cargo build -p itest --no-default-features

ogapo · 2025-08-28T23:07:41Z

It's not really related to the original task, but do you have time to debug this?

Not sure what was going on before but it's definitely running all the levels now (I saw itest observed_all_init_levels fail once) and also confirmed that panics in on_level_init will fail CI (saw that happen once too).

Bromeon

Thanks a lot for the updates! The new init-level tests start to look good 👍

godot-codegen/src/special_cases/codegen_special_cases.rs

Bromeon · 2025-08-29T22:00:15Z

godot-codegen/src/special_cases/special_cases.rs

+        // Anything that comes from another extension could be available in Core but since there would be load order dependencies,
+        // instead don't allow calls until Server level.
+        | "OpenXRExtensionWrapperExtension" 


What are "load order dependencies"? Could you formulate this in simpler terms, so people like me understand it? 😁

I just mean there doesn't seem to be a way (at least I haven't seen one, maybe there is) to ensure that your extension depends on another one such that we could assume any registered classes are available at the core level (during the init loop). Like your extension could init before or after the one that makes this class available is what I meant. Presumably it would be safe to access by the time we are at Servers level then because the Core init call would be guaranteed to have run by then for all extensions.

Then I'd mention

Suggested change

// Anything that comes from another extension could be available in Core but since there would be load order dependencies,

// instead don't allow calls until Server level.

| "OpenXRExtensionWrapperExtension"

// Symbols from another extension could be available in Core, but since GDExtension can currently not guarantee

// the order of different extensions being loaded, we prevent implicit dependencies and require Server.

| "OpenXRExtensionWrapperExtension"

But is this logic sound? Two things:

This assumes that for extensions a, b, Godot initializes a_core, b_core, a_servers, b_servers, ... It would make sense given Godot's own initialization, but is this what happens? Either way, this assumption should be documented.

The problem you describe can also happen on other levels, no? Why does Servers not have the exact same problem -- I'm in Servers level and want to access another extension's Servers class, but how can I guarantee that extension has loaded before mine?

Re: 1 - that is indeed what happens. I've documented the call to initialize_extensions(GDExtension::INITIALIZATION_LEVEL_CORE) (which loops over all extensions internally) on the C++ side in special_cases.rs. Open to suggestions on where else to document this. I personally feel like given the way it's called out it should be a fairly straightforward assumption of the pattern but definitely happy to adjust with more broad language if you have recommendations.

Re: 2 - I agree it's not clear. I actually don't know anything about "OpenXRExtensionWrapperExtension" and when it initializes its classes. The existing codebase classified this one as available at Server level (with a comment to the effect that it "should be available in Core", which did not have support at the time). I just felt it was best to preserve the existing classification before and after my change. If it's wrong, perhaps it'd be better to address it in a separate PR from someone who knows how to test that one?

Re 2: I think keeping existing classification is OK, although it's probably also fine to use the "official" level, now that there is Core.

I just think the logic stated in the comment is misleading -- because order of extension initialization is a general problem, nothing specific to Core, and as such changing levels doesn't really "fix" it. It can create an artificial ordering though (since higher levels are initialized after lower ones), but I wouldn't encourage using this to circumvent such issues.

Bromeon · 2025-08-29T22:02:04Z

godot-codegen/src/special_cases/special_cases.rs

+        | "RenderSceneData" | "RenderSceneDataExtension"
+        => ClassCodegenLevel::Servers,
+        // Declared final (un-inheritable) in Rust, but those are still servers.
+        // NOTE: while these _types_ are available at Server level, the singletons themselves are actually not available until _even after_ Editor level.


Hm, if we're not sure, we should probably not write something in the comment that may potentially be wrong. Rather say that the order is unclear and would need to be investigated in detail.

godot-codegen/src/util.rs

itest/rust/src/lib.rs

itest/rust/src/object_tests/init_level_test.rs

- mostly just mimic'd the setup for server classes added special_case::is_class_level_core - trying to dry up all the special case stuff into special_cases module - added unit testing for init levels. Make sure utilizing the early_core_singletons works. - documented Godot startup sequence in special_cases.rs and tried to reference where each class list was (roughly) coming from - promoted some base types to Core but hestiant to promote the whole list since it's very brittle and later levels are always safe.

- also adjusted singleton test to use Servers level since that's what we are classifying them as

tle-oops reviewed Aug 27, 2025

View reviewed changes

Bromeon added quality-of-life No new functionality, but improves ergonomics/internals c: engine Godot classes (nodes, resources, ...) labels Aug 27, 2025

ogapo marked this pull request as ready for review August 27, 2025 20:42

Bromeon reviewed Aug 27, 2025

View reviewed changes

godot-codegen/src/special_cases/special_cases.rs Outdated Show resolved Hide resolved

Bromeon reviewed Aug 28, 2025

View reviewed changes

godot-codegen/src/special_cases/special_cases.rs Outdated Show resolved Hide resolved

ogapo force-pushed the pr/dnikdel/core-level-classes branch 2 times, most recently from cd25372 to 64555e4 Compare August 28, 2025 22:19

Bromeon reviewed Aug 29, 2025

View reviewed changes

ogapo added 2 commits September 2, 2025 10:51

applying more PR feedback

a488fe1

ogapo force-pushed the pr/dnikdel/core-level-classes branch from 42df8f5 to a488fe1 Compare September 2, 2025 14:51

updating comments based on CI results

843d263

- also adjusted singleton test to use Servers level since that's what we are classifying them as

Bromeon changed the title ~~Adding ClassCodegenLevel::Core~~ Add ClassCodegenLevel::Core Sep 8, 2025

	// Run during on the `on_level_init` of the entry point.
	pub fn initialize_init_level_test(level: InitLevel) {
	if level == InitLevel::Servers {
	assert!(!HAS_RUN.load(Ordering::Acquire));

	let mut some_object = SomeObject::new_alloc();
	// Need to go through Godot here as otherwise we bypass the failure.
	some_object.call("set_has_run_true", &[]);
	some_object.free();
	}
	}

	// Testing that we can initialize and use `Object`-derived classes during `Servers` init level. See `object_tests::init_level_test`.
	object_tests::initialize_init_level_test(level);

Uh oh!

Add ClassCodegenLevel::Core #1289

Are you sure you want to change the base?

Add ClassCodegenLevel::Core #1289

Uh oh!

Conversation

ogapo commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tle-oops Aug 27, 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!

tle-oops Aug 28, 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!

GodotRust commented Aug 27, 2025

Uh oh!

Bromeon commented Aug 27, 2025

Uh oh!

ogapo commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Bromeon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ogapo commented Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

ogapo Aug 29, 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!

Uh oh!

ogapo commented Aug 28, 2025

Uh oh!

Bromeon commented Aug 28, 2025

Uh oh!

ogapo commented Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

Add `ClassCodegenLevel::Core` #1289

Add `ClassCodegenLevel::Core` #1289

ogapo commented Aug 27, 2025 •

edited

Loading

tle-oops Aug 27, 2025 •

edited

Loading

tle-oops Aug 28, 2025 •

edited

Loading

ogapo commented Aug 27, 2025 •

edited

Loading

ogapo commented Aug 28, 2025 •

edited

Loading

Bromeon left a comment •

edited

Loading

ogapo Aug 29, 2025 •

edited

Loading

ogapo commented Aug 28, 2025 •

edited

Loading

Bromeon left a comment •

edited

Loading