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

No longer run Rust classes inside Godot editor #365

Merged
merged 2 commits into from
Jul 30, 2023
Merged

Conversation

Bromeon
Copy link
Member

@Bromeon Bromeon commented Jul 30, 2023

Closes #70.
Closes #132.

Raises MSRV from 1.66 to 1.70.

So far, the workaround to prevent Rust classes from running in the editor was:

if Engine::singleton().is_editor_hint() {
    return;
}

With this PR, this is no longer necessary. By default, all virtual callbacks (ready, process etc.) are no longer invoked.
Note that classes will still be registered (this is necessary as scene trees would be broken otherwise).

This is still quite rudimentary and can be refined over time, but it should already improve the user experience.

The old behavior -- which may be desired by plugins/extensions rather than games -- can be restored with a configuration in the init trait:

#[gdextension]
unsafe impl ExtensionLibrary for DodgeTheCreeps {
    fn editor_run_behavior() -> EditorRunBehavior {
        EditorRunBehavior::Full
    }
}

See API Docs for more information.

@Bromeon Bromeon added feature Adds functionality to the library c: engine Godot classes (nodes, resources, ...) labels Jul 30, 2023
@GodotRust
Copy link

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

@Bromeon
Copy link
Member Author

Bromeon commented Jul 30, 2023

Merging since I need the changes for follow-up work, but feel free to leave feedback!

Add enum EditorRunBehavior to configure semantics.
@Bromeon Bromeon enabled auto-merge July 30, 2023 12:25
@Bromeon Bromeon added this pull request to the merge queue Jul 30, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 30, 2023
Required for OnceCell/OnceLock, which are needed for EditorRunBehavior and will likely
play an important role in safe access to global handles.
@Bromeon Bromeon enabled auto-merge July 30, 2023 12:50
@Bromeon Bromeon added this pull request to the merge queue Jul 30, 2023
Merged via the queue into master with commit 8e56c5f Jul 30, 2023
13 checks passed
@Bromeon Bromeon deleted the feature/no-editor branch July 30, 2023 13:03
@kulkalkul
Copy link
Contributor

I figured game classes also benefit from running inside editor, occasionally. I think them similar to how @tool scripts work in gdscript. Might not worth the hussle, but having a way to register classes as "tool classes" and allowing them to run in editor might be preferable to Full vs. NoVirtuals in some places.

@Bromeon
Copy link
Member Author

Bromeon commented Jul 31, 2023

Yep, I was considering per-class as well (also why I made the behavior an enum and not bool).

One possibility would be something like this:

#[derive(GodotClass)]
#[class(init, base=Node, tool)]
struct MyClass {
    ...
}

And then the global config:

#[gdextension]
unsafe impl ExtensionLibrary for DodgeTheCreeps {
    fn editor_run_behavior() -> EditorRunBehavior {
        EditorRunBehavior::ToolsOnly
    }
}

Probably this should be the default.

Under that design, would it even be useful to have EditorRunBehavior::NoVirtuals (i.e. everything off)?

@kulkalkul
Copy link
Contributor

kulkalkul commented Jul 31, 2023

Yeah, that would be perfect!

Under that design, would it even be useful to have

I don't think so. No real usecase came into my mind

@Bromeon
Copy link
Member Author

Bromeon commented Aug 6, 2023

@kulkalkul "tool classes" are implemented in #374.

@kulkalkul
Copy link
Contributor

Yeah, that's way better, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: engine Godot classes (nodes, resources, ...) feature Adds functionality to the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Faulty InputMap action doesn't exist error message. Extension classes running in editor may be undesired
3 participants