Skip to content

Conversation

@Diddykonga
Copy link

@Diddykonga Diddykonga commented Jan 25, 2026

Objective / Solution

Add methods for field removal on DynamicStruct:

DynamicStruct::remove_at( &mut self, index: usize ) -> Option<(Cow<'static, str>, Box<dyn PartialReflect>)>
DynamicStruct::remove_by_name( &mut self, name: &str ) -> Option<(Cow<'static, str>, Box<dyn PartialReflect>)>
DynamicStruct::remove_if( &mut self, f: FnMut((&str, &dyn PartialReflect))->bool ) -> Option<(Cow<'static, str>, Box<dyn PartialReflect>)>

Testing

4 New Tests, located in bevy_reflect/structs.rs:
dynamic_struct_remove_at
dynamic_struct_remove_by_name
dynamic_struct_remove_with
dynamic_struct_remove_combo


Showcase

Click to view showcase
#[derive(Reflect, Default)]
struct MyStruct {
    a: (),
    b: (),
    c: (),
}

 #[test]
fn dynamic_struct_remove_combo() {
    let mut my_struct = MyStruct::default().to_dynamic_struct();

    assert_eq!(my_struct.field_len(), 3);

    let field_2 = my_struct
        .remove_at(
            my_struct
                .index_of(
                    my_struct
                        .field("b")
                        .expect("Invalid name for `my_struct.field(name)`"),
                )
                .expect("Invalid field for `my_struct.index_of(field)`"),
        )
        .expect("Invalid index for `my_struct.remove_at(index)`");

    assert_eq!(my_struct.field_len(), 2);
    assert_eq!(field_2.0, "b");

    let field_3_name = my_struct
        .name_of(
            my_struct
                    .field_at(1)
                    .expect("Invalid index for `my_struct.field_at(index)`"),
            )
            .expect("Invalid field for `my_struct.name_of(field)`")
            .to_owned();
    let field_3 = my_struct
        .remove_by_name(field_3_name.as_ref())
        .expect("Invalid name for `my_struct.remove_by_name(name)`");

    assert_eq!(my_struct.field_len(), 1);
    assert_eq!(field_3.0, "c");

    let field_1_name = my_struct
        .name_at(0)
        .expect("Invalid name for `my_struct.name_at(name)`")
        .to_owned();
    let field_1 = my_struct
        .remove_if(|(name, _field)| name == field_1_name)
        .expect("No valid name/field found for `my_struct.remove_if(|(name, field)|{})`");

    assert_eq!(my_struct.field_len(), 0);
    assert_eq!(field_1.0, "a");
}

@github-actions
Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Reflection Runtime information about types X-Uncontroversial This work is generally agreed upon D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 25, 2026
Copy link
Member

@MrGVSV MrGVSV left a comment

Choose a reason for hiding this comment

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

A couple nits but also I don't think we should include the other new methods on Struct (at least not part of this PR).

@Diddykonga Diddykonga changed the title DynamicStruct field removal methods and Struct QOL DynamicStruct field removal methods Jan 26, 2026
@Diddykonga Diddykonga changed the title DynamicStruct field removal methods Reflect DynamicStruct field removal methods Jan 26, 2026
@Diddykonga
Copy link
Author

I split this PR between the Dynamic Struct removal methods and the Struct QOL #22708, to be merged after this one.

@Diddykonga Diddykonga requested a review from MrGVSV January 26, 2026 00:30
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Please update the MSRV in this PR, and remove the lint warning. Once that's done, this LGTM.

@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 26, 2026
@Diddykonga Diddykonga force-pushed the dynamic_struct_removal branch from 23fb958 to 4315d52 Compare January 26, 2026 23:26
bevy_reflect: Update MSRV from 1.85.0 to 1.87.0
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jan 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Reflection Runtime information about types C-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants