slotmap icon indicating copy to clipboard operation
slotmap copied to clipboard

Add drain_filter for SlotMap

Open jakoschiko opened this issue 4 years ago • 8 comments

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.

jakoschiko avatar Oct 02 '21 10:10 jakoschiko

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
    });
}

orlp avatar Oct 04 '21 20:10 orlp

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.

jakoschiko avatar Oct 05 '21 11:10 jakoschiko

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 avatar Oct 05 '21 12:10 orlp

How is this issue going? Do you need help?

starovoid avatar Feb 08 '23 15:02 starovoid

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.

orlp avatar Feb 08 '23 15:02 orlp

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.

starovoid avatar Feb 08 '23 17:02 starovoid

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

orlp avatar Feb 08 '23 17:02 orlp

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

dacut avatar Apr 02 '24 22:04 dacut