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

Add Option<Res>::cloned #8749

Open
evaogbe opened this issue Jun 4, 2023 · 3 comments
Open

Add Option<Res>::cloned #8749

evaogbe opened this issue Jun 4, 2023 · 3 comments
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@evaogbe
Copy link

evaogbe commented Jun 4, 2023

What problem does this solve or what need does it fill?

Res does not implement Clone, but instead has a custom clone method. This does not play well when using an Option<Res> that you need to clone since Option::cloned requires the inner type to implement Clone.

What solution would you like?

It would be nice to have an extension to Option<Res> that creates a custom cloned method that calls the custom Res::clone method. Then I could do something like this:

let slot_transfer_data = mouse_data
        .cloned()
        .filter(|m| m.location == MouseLocation::Slot)
        .and_then(|m| m.inventory_index);
let map_transfer_data = mouse_data
        .cloned()
        .filter(|m| m.location == MouseLocation::Map)
        .and_then(|m| camera.viewport_to_world_2d(camera_transform, m.position));

What alternative(s) have you considered?

Right now my code looks like this:

let (slot_transfer_data, map_transfer_data) = if let Some(mouse_data) = mouse_data {
        let slot_transfer_data = if mouse_data.location == MouseLocation::Slot {
            mouse_data
                .inventory_index
                .zip(dragged_item.data)
                .filter(|(dest_index, (dragged_index, _))| dest_index != dragged_index)
        } else {
            None
        };
let map_transfer_data = if mouse_data.location == MouseLocation::Map {
            camera
                .viewport_to_world_2d(camera_transform, mouse_data.position)
                .zip(dragged_item.data)
        } else {
            None
        };
        (slot_transfer_data, map_transfer_data)
    } else {
        (None, None)
    };

IMO it's pretty messy. It's possible there's a simpler workaround already existing. I am new to Rust, so this is the best I've got.

Additional context

I think I know just enough Rust to be able to contribute this if y'all think it's worth it. Also this change doesn't appear to be breaking, so that's nice.

@evaogbe evaogbe added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Jun 4, 2023
@mockersf
Copy link
Member

mockersf commented Jun 4, 2023

#4109 for context on why the trait is not implemented

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Jun 4, 2023
@TheBlek
Copy link
Contributor

TheBlek commented Jun 5, 2023

For this particular use case you could probably write something like this:

let (slot_transfor_data, map_transfer_data) = match mouse_data {
    Some(mouse_data) if mouse_data.location == MouseLocation::Map => {
        (Some(mouse_data.data), None)
    },
    Some(mouse_data) if mouse_data.location == MouseLocation::Slot => {
        (None, Some(mouse_data.data))
    },
    _ => (None, None)
};

Also, as far as I know this would get rid of the clone in question entirely.

@evaogbe
Copy link
Author

evaogbe commented Jun 6, 2023

Thanks for the tip! I guess the question remains whether there are other usecases that can't be solved in the same way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

No branches or pull requests

4 participants