-3

I am writing a Rogue-like in Rust. I have a Player which has a Vec of boxed TimedEffects. A timed effect has an activate(&mut Player) and deactivate(&mut Player) method, and a tick() method that decrements the time remaining. When it goes to zero, the deactivate() method should be called.

pub struct Player {
    timed_effects: Vec<Box<TimedEffect>>,
}

Each time the player moves, I need to decrement the time remaining. This is done inside a Player method:

impl Player {
    fn regenerate(&mut self) {
        self.timed_effects.iter_mut().for_each(|te| te.tick());

        for i in 0..self.timed_effects.len() {
            if self.timed_effects[i].ticks_remaining() == 0 {
                let bte = self.timed_effects.swap_remove(i);
                bte.deactivate(self);
            }
        }
    }
}

I originally tried passing the self parameter (&mut Player) to the tick() method of the TimedEffect objects, with logic to invoke deactivate(player) automatically when the time went to zero.

Iterating over the vector constitutes a borrow of (Player) self, which is incompatible with passing the &mut Player parameter to tick(), or even to a separate iteration with .filter.

Instead, as shown, I find myself iterating over a range of integers, extracting the box from the vector into a local variable, and then invoking .deactivate(self) on it.

This is painful.

Given the effects need to have some connection with their victim so the deactivate() method can undo the effect, how should I be architecting these objects so that I don't wind up tangled in this web of dependent borrows?

Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
aghast
  • 14,785
  • 3
  • 24
  • 56
  • 1
    Please provide a [MCVE]. My guess is that you got some compiler error somewhere that would point you to a suitable existing Q&A, but we can't see what the error would be. The lack of MCVE also means that we would have to retype all of your dependent code, potentially introducing exciting new bugs. – Shepmaster Mar 23 '18 at 01:35
  • That being said, using my magical powers of deduction, I'm going to guess it's a duplicate of one or more of [How can I call a mutating method while holding a reference to self?](https://stackoverflow.com/q/27335252/155423), [Passing mutable self reference to method of owned object](https://stackoverflow.com/q/30681468/155423), or [cannot borrow `self.x` as immutable because `*self` is also borrowed as mutable](https://stackoverflow.com/q/31093841/155423). Please go ahead and either [edit] your question to explain why it isn't answered by those questions or mark the question as a duplicate. – Shepmaster Mar 23 '18 at 01:36
  • I'm learning Rust - I've got compiler errors all over the place! But I followed those errors, and produced the code shown, which avoids the borrow by not using rust's iterators. My question was really about the design, and what design changes could produce a nicer result. (The fact that I didn't have an error probably hindered my search, since I didn't have the right keywords...) – aghast Mar 23 '18 at 02:16
  • Rust is protecting your future self from shooting itself in the foot. Imagine iterating over `.tick(&mut player)` worked now. What if in the future you decided to add a timed effect that upon `.deactivate()` removes all other timed effects from the player? You would end up with an invalidated iterator. The design rust forced you to adopt does not have that problem. – MB-F Mar 23 '18 at 09:00

1 Answers1

2

To avoid the borrowing issue, you can do it in two steps:

  1. Transfer the effects which are done from timed_effects to a buffer,
  2. Iterate over the buffer and invoke deactivate.

I'm going to use nightly's Vec::drain_filter for slicker code, although it's certainly not mandatory.

fn regenerate(&mut self) {
    self.timed_effects.iter_mut().for_each(|te| te.tick());

    let timed_out: Vec<_> =
        self.timed_effects
            .drain_filter(|t| t.ticks_remaining() == 0)
            .collect();

    //  Split iteration in a second step to stop borrowing self.
    for timer in timed_out {
        timer.deactivate(self);
    }
}

For further efficiency, you can pass a reusable buffer to regenerate (to avoid allocations).

Matthieu M.
  • 287,565
  • 48
  • 449
  • 722