Skip to content

Support @export_storage attribute #1183

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

Merged
merged 1 commit into from
Jun 2, 2025

Conversation

Swivelgames
Copy link
Contributor

@Swivelgames Swivelgames commented May 31, 2025

Partially completes #1128

See GDScript: Add @export_storage annotation godotengine/godot#82122 added in Godot 4.3

Adds the ability to annotate a field with @export_storage:

#[derive(GodotClass)]
#[class(base=RigidBody3D)]
struct Vehicle {
    /// Equivalent to:
    /// ```gdscript
    /// @export_storage var steering_angle: float
    /// ```
    #[export(storage)]
    steering_angle: f64,

    base: Base<RigidBody3D>
}

@Swivelgames Swivelgames marked this pull request as draft May 31, 2025 04:33
@Bromeon Bromeon added feature Adds functionality to the library c: register Register classes, functions and other symbols to GDScript labels May 31, 2025
@Bromeon Bromeon added this to the 0.3.x milestone May 31, 2025
@Bromeon
Copy link
Member

Bromeon commented May 31, 2025

Thanks a lot for your contribution! 👍

I would probably merge the other changes to exports #1166 first, which might cause small merge conflicts, but I expect it wouldn't be difficult to resolve. Maybe you can already rebase onto that branch 🙂

Regarding failing CI, you can run tests locally, see Dev tools and testing.

@GodotRust
Copy link

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

@Swivelgames
Copy link
Contributor Author

Awesome! @Bromeon It turns out that this is more involved than I had initially expected. @export_storage modifies the PropertyUsageFlags of the property, so I may have to do a bit more refactoring. I'll have something pushed up soon! 🙌

@Swivelgames Swivelgames force-pushed the feature/export-storage branch 2 times, most recently from b3b5201 to 284935c Compare May 31, 2025 21:25
@Swivelgames Swivelgames marked this pull request as ready for review May 31, 2025 21:27
@Swivelgames
Copy link
Contributor Author

Swivelgames commented May 31, 2025

I was having issues running itest, but it's because I forgot to set CARGO_TARGET_DIR=target.

To save space, I have my CARGO_TARGET_DIR set to ~/.cache/cargo/target by default. Oops 😅

I'm able to run them now. I'll address the issues I found 👍

Original Post

@Bromeon ./check.sh test seems to be passing, but itest fails for me with:

./check.sh itest                                                                                                                                  130Checks to run: itest

Found 'godot' executable with version 4.4.1.stable.arch_linux
> cargo build -p itest --no-default-features
   Compiling godot-codegen v0.2.4 (./gdext/godot-codegen)
   Compiling godot-macros v0.2.4 (./gdext/godot-macros)
   Compiling godot-ffi v0.2.4 (./gdext/godot-ffi)
   Compiling godot-core v0.2.4 (./gdext/godot-core)
   Compiling itest v0.0.0 (./gdext/itest/rust)
   Compiling godot v0.2.4 (./gdext/godot)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 13.31s
> godot --path itest/godot --headless -- \[\]
ERROR: Condition "!FileAccess::exists(path)" is true. Returning: ERR_FILE_NOT_FOUND
   at: open_dynamic_library (drivers/unix/os_unix.cpp:893)
ERROR: GDExtension dynamic library not found: 'res://itest.gdextension'.
   at: open_library (core/extension/gdextension.cpp:701)
ERROR: Error loading extension: 'res://itest.gdextension'.
   at: load_extensions (core/extension/gdextension_manager.cpp:291)
Godot Engine v4.4.1.stable.arch_linux - https://godotengine.org

SCRIPT ERROR: Parse Error: Identifier "IntegrationTests" not declared in the current scope.
          at: GDScript::reload (res://TestRunner.gd:41)
ERROR: Failed to load script "res://TestRunner.gd" with error "Parse error".
   at: load (modules/gdscript/gdscript.cpp:3022)

@Swivelgames Swivelgames marked this pull request as draft May 31, 2025 21:38
@Swivelgames Swivelgames force-pushed the feature/export-storage branch 3 times, most recently from 19985b7 to 5cf7cb6 Compare May 31, 2025 22:40
@Swivelgames Swivelgames marked this pull request as ready for review May 31, 2025 22:41
@Swivelgames
Copy link
Contributor Author

Tests are working with Godot 4.4. I'll try and see if I can downgrade my Godot version to test with older versions and make sure everything is good to go 👍

@Swivelgames Swivelgames force-pushed the feature/export-storage branch from 5cf7cb6 to 8e051d9 Compare May 31, 2025 23:09
@Swivelgames
Copy link
Contributor Author

Final fixes in place. Tests are passing for Godot 4.1, 4.2, 4.3, and 4.4 🙌

Also tested in my current project, and @export_storage is behaving correctly 👍

@Swivelgames Swivelgames force-pushed the feature/export-storage branch from 8e051d9 to 4dd6e37 Compare May 31, 2025 23:33
@Swivelgames
Copy link
Contributor Author

Swivelgames commented May 31, 2025

Rebased from master now that #1166 has been merged, resolved conflicts, and still passing on 4.1-4.4 👍

@Swivelgames Swivelgames force-pushed the feature/export-storage branch from 4dd6e37 to 819207d Compare May 31, 2025 23:47
@Swivelgames
Copy link
Contributor Author

Swivelgames commented May 31, 2025

I was doing an unnecessary #[cfg()]. Wasn't critical, but it was unnecessary. Just removed it. Still passing on 4.1-4.4.

No more changes incoming 👍

Copy link
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@Swivelgames Swivelgames force-pushed the feature/export-storage branch from 819207d to 3177514 Compare June 1, 2025 18:16
@Bromeon Bromeon added this pull request to the merge queue Jun 2, 2025
@Bromeon Bromeon changed the title Added @export_storage Export Type Support @export_storage attribute Jun 2, 2025
Merged via the queue into godot-rust:master with commit 88cbc42 Jun 2, 2025
17 checks passed
@Bromeon
Copy link
Member

Bromeon commented Jun 2, 2025

Thanks a lot for your contribution, and congrats to the first merged PR! 😊

@Swivelgames Swivelgames deleted the feature/export-storage branch June 2, 2025 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: register Register classes, functions and other symbols to GDScript feature Adds functionality to the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants