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

Grouping Exported Properties in the Editor #226

Open
gg-yb opened this issue Apr 12, 2023 · 10 comments
Open

Grouping Exported Properties in the Editor #226

gg-yb opened this issue Apr 12, 2023 · 10 comments
Labels
c: register Register classes, functions and other symbols to GDScript feature Adds functionality to the library

Comments

@gg-yb
Copy link
Contributor

gg-yb commented Apr 12, 2023

Motivation

For quality of life, it is sometimes advisable to group related properties in the editor. This is already possible with GDScript, and used extensively by the built-in node types in Godot.

Examples, and a first flawed attempt at introducing this to gdext can be seen in #214.

On Syntax

There are different proposals for syntax, each of which have their unique pros and cons. Some of these originate from the same issue in gdnative, godot-rust/gdnative#855.

Goals

  1. Rust code should not be limited with regards to item organization, field ordering, field naming etc. as to not violate expectations of developers coming from a Rust background.
  2. Type-safety must be upheld, illegal states must be unrepresentable.
  3. Verbosity should be kept to the minimum extent required to make the feature intuitive, understandable, and surprise-less.

#[export_group]

This was originally proposed in #214, but fails to achieve the first goal, as field naming and field ordering are mandated by group membership.

#[derive(GodotClass)]
struct Foo {
    #[export_group(name = "My Things", prefix = "my_")]
    #[export(...)]
    my_x: i32,
    #[export(...)]
    my_y: i32,
}

Marker Fields

Originally proposed in godot-rust/gdnative#855 (comment), this has the same downside as the above, but with a different syntax.

Separate Structs Deriving ExportGroup

This would introduce a new derivable trait ExportGroup which would allow use as a property type. The exported fields of the struct deriving this trait would be available as grouped properties.

#[derive(ExportGroup)]
#[group(name="Offset", prefix="offset_")]
struct OffsetGroup {
    #[export]
    x: f32,
    #[export]
    y: f32,
}

#[derive(GodotClass)]
#[class(init, base=Node)]
struct MyNode {
    #[export(group)]
    offsets: OffsetGroup,
}

A question arising here is whether to allow overriding the name and prefix at the site of usage, which would enable uses such as

#[derive(ExportGroup)]
struct MyImportantType {
    #[export]
    a: i32,
    #[export]
    b: GodotString,
}

#[derive(GodotClass)]
struct Foo {
    #[export(group_name="Element 1", prefix="element_1_")]
    el1: MyImportantType,
    #[export(group_name="Element 2", prefix="element_2_")]
    el2: MyImportantType,
}

which enables structs to be re-used as custom (domain specific) property types which are reusable over many structs.

Use Marker Structs

Similar to the previous approach, but require fields to be kept on the struct deriving GodotClass.

#[derive(ExportGroup)]
#[group(name="Offset", prefix="offset_")]
struct OffsetGroup; // just a tag

#[derive(GodotClass)]
#[class(init, base=Node)]
struct MyNode {
    #[export(group = OffsetGroup)]
    offset_x: f32,
    #[export(group = OffsetGroup)]
    offset_y: f32,
}

Loses the reusability option of the previous approach, but more close to how the data would be laid out in GDScript.

String Based Groups

Like the previous approach, but instead of marker structs it uses string-based group identifiers which do not actually exist as Rust items.

#[derive(GodotClass)]
#[class(init, base=Node)]
#[group(name="Offset", prefix="offset_")]
struct MyNode {
    #[export(group = "Offset")]
    offset_x: f32,
    #[export(group = "Offset")]
    offset_y: f32,
}

Explicit Property Paths

I do not know much about gdnative, but this seems to be a way to achieve property groups there:

#[property(path = "foo/bar")]

creates a property available as bar in a group foo. This has the advantage of decoupling the notion of groups from Rust entirely, and making it purely aesthetic, but when reusability of domain-specific editor-editable types is a concern, this may actually be a downside.

@Bromeon Bromeon added feature Adds functionality to the library c: register Register classes, functions and other symbols to GDScript labels Apr 12, 2023
@lilizoey
Copy link
Member

Personally I'd prefer the structs deriving ExportGroup approach, if they could additionally be used for making non-grouped it would also give us a way to reuse exports between different classes that have the same exported variables but dont necessarily want to group them.

i think allowing you to override the name at the usage site would also be good.

@lilizoey
Copy link
Member

lilizoey commented May 4, 2023

Another thing to consider is subgroups. A group can have subgroups, but subgroups can't have subgroups.

@FlooferLand
Copy link

Not exactly sure if this would work syntactically, but I did subconciously end up trying this while i was trying to blindly figure out whenever export_group existed:

#[export_group("Settings")] {
    #[export] volume: f32,
    #[export] pitch: f32
}

I'm not sure whenever this could work inside a struct declaration (I seem to get syntax errors trying it out) but I have used this type of syntax before inside function bodies (i've used it with #[cfg()] to return different values based on what feature is enabled).

@fpdotmonkey
Copy link
Contributor

fpdotmonkey commented Oct 29, 2023

What about doing it as a trait? The macro-based APIs could also be provided, but a trait-based API could be a little nicer for more complex applications. Here's a back-of-the-napkin design for what it could look like.

impl EditorRepr for MyClass {
    fn repr(&self, editor_stuff: EditorStuff) -> EditorStuff {
        editor_stuff
            .default(self.float) // do nothing fancy for .default()
            .group( // provide a label and a chain of parameters
                "Several things"
                |ed_stuff| {
                    ed_stuff
                        .different_repr(self.happiness)
                        .default(self.position)
                        .done()
                })
            .done()
    }
}

@lilizoey
Copy link
Member

What about doing it as a trait?

I was thinking that it'd be nice if the Property trait could declare property groups as well as just properties somehow.

@Bromeon
Copy link
Member

Bromeon commented Oct 29, 2023

What about doing it as a trait? The macro-based APIs could also be provided, but a trait-based API could be a little nicer for more complex applications.

This is anyway planned as part of #4, but it would need the whole builder API infrastructure, which is not yet in place. Eventually the idea is that the proc-macro is just a frontend that uses the builders in the backend. Currently the development is quite usage-driven (building the proc-macro API first).

@Bromeon
Copy link
Member

Bromeon commented Jul 8, 2024

For a workaround, see also Discord.

@gg-yb
Copy link
Contributor Author

gg-yb commented Nov 7, 2024

For a workaround, see also Discord.

Hey, thanks for the information that there is a workaround!

Is it possible to copy-paste the relevant content into this GitHub thread if the content is in a form where it could easily be reproduced here? If so, that would be greatly appreciated as we cannot access Discord from within company networks and on company devices due to a network blocklist.

As a positive side-effect, that would also help searchability of the relevant topics here on the site :)

@Bromeon
Copy link
Member

Bromeon commented Nov 7, 2024

I don't want to copy the original picture without the author's consent, but to sum up:

You can emulate the workaround using dummy properties (whose value, here u32, is unused) and the usage_flags key of the #[var] attribute. The indentation is for illustration purposes.

// Properties outside a group.
#[export]
level_name: GString,


// First group.
#[var(usage_flags = [GROUP, EDITOR, READ_ONLY])
configurations: u32,

    #[export(file = "*.toml")]
    map_config_file: GString,

    #[export(file = "*.toml")]
    enemy_config_file: GString,


// Second group.
#[var(usage_flags = [GROUP, EDITOR, READ_ONLY])
world_parameters: u32,

    #[export]
    world_size: Vector2i,

    #[export]
    gravity: f32,

    #[export]
    particles: Option<Gd<GpuParticles2D>>,

It's worth noting that this relies on the order of fields being registered in the order of declaration, which isn't something we guarantee (while I don't plan on changing it, there may be reasons in the future). And probably some other implementation details... So use at your own risk 🙂

@Bromeon
Copy link
Member

Bromeon commented Nov 7, 2024

Out of curiosity, are you using godot-rust at work? Are you allowed to say more about it (can also be non-publicly)? ☺️

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

No branches or pull requests

5 participants