Skip to content

Instance creation now goes through proper placeholder process #1597

Merged
Bromeon merged 4 commits into
masterfrom
bugfix/object-placeholders
May 14, 2026
Merged

Instance creation now goes through proper placeholder process #1597
Bromeon merged 4 commits into
masterfrom
bugfix/object-placeholders

Conversation

@Bromeon
Copy link
Copy Markdown
Member

@Bromeon Bromeon commented May 11, 2026

Fixes #1404.
Fixes #912.

This PR changes Gd::default_instance() so it goes through the editor/placeholder process when needed.

I decided against blanket call to ClassDb::instantiate(), because this downgrades an indirect C function call (create callback) to a Godot reflection call with all the bells and whistles + extra Variant marshalling. I didn't want to have this overhead on hot paths in games, as object default-creation is still a pretty fundamental operation.

Instead, the code first checks whether editor is running, and only then goes through ClassDb::instantiate(). For non-editor runs, the old way works fine, so we keep using that.

Checking whether editor is running turns out to be harder than I thought, and takes up half of this PR (but contained in last commit). The problem is that Engine class isn't available in init-stage < Scene for Godot < 4.4, and I've found no other easy way than Engine::is_editor_hint() to check this. So we need to have a tristate (editor, runtime, unknown).


Apart from this, the PR adds quite a few itests around placeholder behavior. Thanks to #1591, we can now test those and will be notified immediately if we break something or Godot changes semantics.

I would use this PR as a base for future improvements, e.g. no longer implying tool in #[class(singleton)].

@Bromeon Bromeon added bug c: core Core components labels May 11, 2026
Bromeon added 2 commits May 11, 2026 20:26
`init_stage_test.rs`: classes `SomeObject` and `MainLoopCallbackSingleton`. If substituted
as placeholders, `bind()` would panic.
Cover `Gd`'s placeholder behavior for runtime classes in the editor, regarding:
* default/cached property values (`get`, `set`, property list, class identity)
* panic in `bind()`
* virtual callbacks (skipped for placeholders)
@Bromeon Bromeon force-pushed the bugfix/object-placeholders branch from afd41af to 1410d09 Compare May 11, 2026 18:32
@GodotRust
Copy link
Copy Markdown

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

@Bromeon Bromeon force-pushed the bugfix/object-placeholders branch from 1410d09 to 47d0abe Compare May 11, 2026 18:48
@TitanNano
Copy link
Copy Markdown
Contributor

TitanNano commented May 13, 2026

Looks good. I also tested it with my project, and it didn't break any of my tool classes. 👍 Maybe we provide a deprecated alias for is_editor as is_editor_hint? That is currently a breaking change.

@Bromeon
Copy link
Copy Markdown
Member Author

Bromeon commented May 13, 2026

Oh, very good catch, I missed that we publicly re-export this. I'll add it back, we can always change in the future.

Comment thread godot-core/src/classes/class_runtime.rs Outdated
panic!(
"Gd::{method}() called on a placeholder instance of `{name}`.\n\
A runtime (= non-tool) class does not have a real instance in the editor.\n\
Use `#[class(tool)]`, or guard with `Engine::singleton().is_editor_hint()`.",
Copy link
Copy Markdown
Contributor

@Yarwin Yarwin May 13, 2026

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

#1597 (comment) 🙂 but yes.

Comment thread godot-core/src/classes/class_runtime.rs Outdated
pub(crate) fn panic_placeholder_bind<T>(method: &str) -> ! {
panic!(
"Gd::{method}() called on a placeholder instance of `{name}`.\n\
A runtime (= non-tool) class does not have a real instance in the editor.\n\
Copy link
Copy Markdown
Contributor

@Yarwin Yarwin May 13, 2026

Choose a reason for hiding this comment

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

We don't use "runtime class" anywhere in public-facing docs. Can we use tool class instead, like earlier? In the past you raised argument that it is implementation detail which might not be obvious/intuitive to user (since Godot doesn't really use it anywhere)

Copy link
Copy Markdown
Member Author

@Bromeon Bromeon May 14, 2026

Choose a reason for hiding this comment

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

Fair, I changed it to "non-tool class".

Maybe we can at some point introduce "runtime class" as a concept, but it should probably be in a more prominent place, not some isolated error messages.

@Yarwin
Copy link
Copy Markdown
Contributor

Yarwin commented May 13, 2026

I checked with simple setup in editor - init creates real instances (as it should), editor operates on placeholders:

Details
#[derive(GodotClass)]
#[class(init, base = Resource)]
struct MyClass {
    #[export]
    #[init(val = GString::from("my default"))]
    #[var(get=my_something)]
    something: GString,
}

#[godot_api]
impl MyClass {
    #[func]
    fn my_something(&self) -> GString {
        godot_print!("henlo");
        self.something.capitalize()
    }
}

#[derive(GodotClass)]
#[class(init,  base = Node)]
struct MyClass2 {
    #[export]
    #[var(get = my_get2)]
    value: i32,
    #[export]
    other_value: Option<Gd<MyClass>>,
    base: Base<Node>,
}

#[godot_api]
impl MyClass2 {
    #[func]
    fn my_get2(&self) -> i32 {
        godot_print!("hello {:}", self.to_gd());
        42
    }
}
image

Bromeon added 2 commits May 14, 2026 14:30
Replace `AtomicBool` with `AtomicEnum`.
The "Unknown" state applies mostly for Godot < 4.4 && level < Scene.
@Bromeon Bromeon force-pushed the bugfix/object-placeholders branch from 47d0abe to 41cb5ae Compare May 14, 2026 12:30
@Bromeon Bromeon added this pull request to the merge queue May 14, 2026
Merged via the queue into master with commit c7a45f8 May 14, 2026
23 checks passed
@Bromeon Bromeon deleted the bugfix/object-placeholders branch May 14, 2026 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug c: core Core components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gd::default_instance bypasses placeholder instances Crash when loading resource on init with API 4.3

4 participants