orlp / slotmap

Slotmap data structure for Rust
zlib License
1.12k stars 70 forks source link

Add drain_filter for SlotMap #77

Open jakoschiko opened 3 years ago

jakoschiko commented 3 years ago

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 commented 2 years ago

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 commented 2 years ago

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 https://github.com/rust-lang/rust/issues/43244.

orlp commented 2 years ago

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).

starovoid commented 1 year ago

How is this issue going? Do you need help?

orlp commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 6 months ago

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).