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 drain_filter for SlotMap #77

Open
jakoschiko opened this issue Oct 2, 2021 · 8 comments
Open

Add drain_filter for SlotMap #77

jakoschiko opened this issue Oct 2, 2021 · 8 comments

Comments

@jakoschiko
Copy link

Would it be reasonable to add an API that allows this?

fn update_players(world: &mut World, players: &mut SlotMap<PlayerKey, Player>) {
    let mut entries = players.entries();
    
    while let Some(entry) = entries.next_entry() {
        let player = entry.get_mut();
        player.update(world);
        
        if player.is_dead() {
            let player = entry.remove();
            player.deinit(world);
        }
    }
}

In contrast to the Entry API of the secondary maps, this Entry API would provide access to only the occupied slots. I think it should be possible to implement this. If wished I would provide a PR.

@orlp
Copy link
Owner

orlp commented Oct 4, 2021

Rather than entries, I think a drain_filter method would be more inline with the rest of Rust. Would that work for you?

https://doc.rust-lang.org/std/collections/hash_map/struct.HashMap.html#method.drain_filter

So:

fn update_players(world: &mut World, players: &mut SlotMap<PlayerKey, Player>) {
    players.drain_filter(|(player_key, player)| {
        player.update(world);
        if player.is_dead() {
            player.deinit(world);
            return true;
        }
        false
    });
}

@jakoschiko
Copy link
Author

Yes, drain_filter would work for me. In some cases I need the ownership of the removed elements and that's also possible with drain_filter. Though it's unstable and the name and signature is discussed since 2017, see rust-lang/rust#43244.

@orlp
Copy link
Owner

orlp commented Oct 5, 2021

Just for the record, SlotMap::drain_filter would not use drain_filter internally, so the resulting feature would be available on stable. And I can choose whatever name I want to, I think drain_filter is fine, even if the similar function in the standard library ends up changing name (I can always deprecate and add the new name as well).

@orlp orlp changed the title Entry API for SlotMap Add drain_filter for SlotMap Oct 5, 2021
@starovoid
Copy link

How is this issue going? Do you need help?

@orlp
Copy link
Owner

orlp commented Feb 8, 2023

I have been very busy and not always as productive the last year and a bit. I'd like to move this into slotmap 2, which I'm expecting to start working on Soon:tm:, after winding down my current work on glidesort and another library I'm releasing soon.

@starovoid
Copy link

I've seen glidesort, great job! I will follow updates and news here and there. What do you think about adding "help wanted" notes on this and other repositories? I am sure that many people would like to help you.

@orlp
Copy link
Owner

orlp commented Feb 8, 2023

@starovoid The annoying part is that I've been meaning to rewrite slotmap for a while now, and that any help at this stage would be ultimately counterproductive to both parties. I understand this is frustrating if you want features on a certain timeline, so I'm sorry for that.

@dacut
Copy link

dacut commented Apr 2, 2024

FWIW, the standard library changed the name; it's currently extract_if(), and the feature gate changed to match (!#feature[(extract_if)]).

Would also love to see this (here and in the std collections).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants