setTimeout in object method continues to run after object is destroyed

Question

I have a class called Bullet which is essentially a div on a space invader webpage. When this bullet gets 'fired' I call a method which gradually moves the 'bullet' up the screen.

When the bullet gets to the edge of the screen I want to remove the whole bullet object from memory. However, the setTimeout loop continues to run even after I've deleted it (I think).

I'm sure there is a better way to do this! Perhaps it's foolish to run the loop like this?

TIA

this.bulletmove = new CustomEvent("bulletmove",{detail:this.name});

...


/**
 * moves the bullet up the screen gradually
 */
fire(){

    var that = this;

    setTimeout(function(){
        that.moveUp();
        window.dispatchEvent(that.bulletmove);
        that.fire();

    },50);

}

The event is picked up in a controller script which checks if the bullet has reached the edge of the screen at which point it is deleted:

window.addEventListener('bulletmove', function(evt) {
    checkCollision(evt);
},false);

...


/**
*Check if the bullet has gone off screen and deletes it
**/

function checkCollision(e){

    var bulletName = e.detail;
    
    var bullet = bullets[bulletName];

    //check if the bullet has gone off screen
    if (bullet.bottom < 0){
        bullet.destroy;
        delete bullets[e.detail];
        bullet=null;
    }
}

Answer 1

In the following code, an outer div forms the boundaries of a playfield and all game elements inside the playfield are represented by divs.

Game state consists of an array of game elements (in this instance: one ship and zero or more bullets). Once an element is no longer visible (here: simply off the right-hand side of the playfield), it is removed from game state.

The game loop uses requestAnimationFrame to repeatedly render the game state to the playfield.

Bullet position is calculated using the time of firing and the time elapsed (I added a little randomness to bullet velocity just for fun).

Game elements such as bullets have an associated generator function called as part of the game loop, to retrieve the next state of the element (a bullet "moves by itself" after the initial appearance).

Firing a bullet in this design is as simple as creating a new bullet object with an initial position and an instance of a generator function to account for its trajectory; and then adding that pair to the game state.

const elem = ({ kind = 'div', classN = '' }) => {
  const el = document.createElement(kind)
  el.classList.add(classN)
  return el
}

const applyStyle = (el, style) => 
  (Object.entries(style)
         .forEach(([k, v]) => el.style[k] = v), el)
         
const cssPixels = (str) => +(str.slice(0, -2))

const isVisible = (left) => 
  cssPixels(left) < cssPixels(playfield.style.width)

const createPlayfield = () =>
  applyStyle(elem({ classN: 'playfield' }), { width: '300px' })

const createShip = (startLeft, width) => 
  [{ classN: 'ship', style: { left: startLeft, width } }, null]

const createBullet = (startLeft) => {
  const b = {
    classN: 'bullet',
    style: { left: startLeft },
    firingTime: +new Date(),
    velocity: 0.5,
    velocitySeed: Number('1.' + ~~(Math.random() * 9)),
    startLeft
  }
  const g = bulletStateGen(b)      
  return [ b, () => g.next() ]
}    

const bulletPos = ({ firingTime, 
                     startLeft,
                     velocity,
                     velocitySeed }, now = +new Date()) => 
    `${~~(velocity * (now - firingTime) * velocitySeed + cssPixels(startLeft))}px`

const bulletStateGen = function*(b) {
  while (1) {
    const left = bulletPos(b)
    
    if (!isVisible(left))
      break

    b.style = { left }
    yield(b)
  }
}

const fire = (startLeft) => 
  state.unshift(createBullet(startLeft))

const tick = () => 
  state = state.reduce((acc, [o, next]) => {
    if (!next)
      return acc.push([o, next]), acc
          
    const { value, done } = next()
      
    if (done)
      return acc
          
    return acc.push([value, next]), acc
  }, [])

const blank = () => playfield.innerHTML = ''
          
const render = () => {
  blank()
  state.forEach(([{ classN, style = {} }]) => 
    playfield.appendChild(applyStyle(elem({ classN }), style)))
}

let ship = createShip('10px', '50px')
let state = [ship]
let playfield = createPlayfield()

const gameLoop = () => 
  (render(), tick(), requestAnimationFrame(gameLoop))

const init = () => {       
  document.body.appendChild(playfield)
  document.body.onkeyup = (e) =>
    e.key === " " 
    && fire(`${cssPixels(ship[0].style.left) + cssPixels(ship[0].style.width)}px`)
}

init()
gameLoop(state, playfield)

.playfield {
  height: 300px;
  background-color: black;
  position: relative;
}

.ship {
  top: 138px;
  height: 50px;
  background-color: gold;
  position: absolute;
  border-radius: 7px 22px 22px 7px;
}

.bullet {
  top: 163px;
  width: 10px;
  height: 2px;
  background-color: silver;
  position: absolute;
}

Click on the game to focus it, and then press spacebar to fire!

Answer 2

I would use RxJS to monitor the progress of your bullets.

In the example below I have three different bullets. Each within its own boundary. Once fired, they will immediately stop when they exit their box.

For each bullet we have an "animation frame" observable that emits when such a frame is made available by the browser (internally RxJS uses requestAnimationFrame for this). At that point we check whether the bullet is still within its parent bounding box. If it is we move it otherwise we don't and the subscription to the animation frame stream automatically ends.

const rightPos = el => el.getBoundingClientRect().right;
  
const moveBullet = (sel, pos) =>
  document.querySelector(sel)
    .style.left = `${pos}px`;

const fire = (bullet) => {
  const el = document.querySelector(bullet);
  const parentPos = rightPos(el.parentNode);
  return animationFrames().pipe(
    map(() => rightPos(el)),
    takeWhile(pos => pos < parentPos)
  );
}

const bullet1$ = fire('#bullet1');
const bullet2$ = fire('#bullet2');
const bullet3$ = fire('#bullet3');

const fire$ = fromEvent(document.querySelector('button'),'click');

fire$.subscribe(() => {
  bullet1$.subscribe(pos => moveBullet('#bullet1', pos+1));
  bullet2$.subscribe(pos => moveBullet('#bullet2', pos+1));
  bullet3$.subscribe(pos => moveBullet('#bullet3', pos+1));
});

div {
  height: 30px;
  border: 1px solid black;
  margin: 5px;
  position: relative;
}

span { position: absolute; }

<script src="https://unpkg.com/rxjs@7.5.7/dist/bundles/rxjs.umd.min.js"></script>

<script>
const {animationFrames, fromEvent} = rxjs;
const {map, takeWhile} = rxjs.operators;
</script>


<div style="width:150px"><span id="bullet1">🏉</span></div>
<div style="width:300px"><span id="bullet2">🥏</span></div>
<div style="width:450px"><span id="bullet3">⚽️</span></div>

<button>Fire!</button>

Answer 3

I think you should use setInterval instead of calling fire() again - by calling that function, a new setTimeout is created (with a new handler); before the removal of the object, you call obj.halt(), and that clears the setInterval correctly.

const obj = {
  name: "objName",
  bulletmove() {
    return new CustomEvent("bulletmove", {
      detail: this.name
    })
  },
  halt() {
    clearInterval(this.intervalHandler)
  },
  intervalHandler: null,
  fire() {
    const handler = setInterval(() => {
      // this.moveUp()
      // console.log("move up")
      window.dispatchEvent(this.bulletmove())
      // this.fire()
    }, 500)
    this.intervalHandler = handler
  },
}

let i = 0

window.addEventListener('bulletmove', function(e) {

  // this if-else if mocks the collision detection
  // expected: log obj.name 5 times, then clear the interval,
  // then event should not be called anymore
  if (i < 5) {
    console.log(i, e.detail)
  } else if (i < 8) {
    obj.halt()
    console.log(i)
  } else if (i < 100) {
    console.log(i, e.detail)
  }
  i++
})

obj.fire()

ANOTHER WAY

A cleaner approach would be if the fire method returned its own "clear function", and you could use that in the event handling:

const obj = {
  name: "objName",
  bulletmove() {
    return new CustomEvent("bulletmove", {
      detail: this.name
    })
  },
  fire() {
    const handler = setInterval(() => {
      // this.moveUp()
      // console.log("move up")
      window.dispatchEvent(this.bulletmove())
      // this.fire()
    }, 500)
    return () => clearInterval(handler)
  },
}

let i = 0

const fireHandler = obj.fire()
const eventHandler = (clearFn) => (e) => {

  // this if-else if mocks the collision detection
  // expected: log obj.name 5 times, then clear the interval,
  // then event should not be called anymore
  if (i < 5) {
    console.log(i, e.detail)
  } else if (i < 8) {
    clearFn()
    console.log(i)
  } else if (i < 100) {
    console.log(i, e.detail)
  }
  i++
}

const eventHandlerWithRemoveFn = eventHandler(fireHandler)

window.addEventListener('bulletmove', eventHandlerWithRemoveFn)

The drawback of this method is that you need to add each object's event handler separately to the window, its benefit is more control, cleaner code (no need to save that handler in the object).

A MODIFIED VERSION FOR MULTIPLE INTERVALS

This is a version of the previous solution, where the clearing functions are stored in the window object:

const eventHandler = (e) => {
  const i = e.detail.eventCounter
  if (i < 3) {
    console.log(i, e.detail.name)
  } else if (i < 4) {
    window.bulletIntervals[e.detail.name]()
    console.log(i, e.detail.name + " is halted")
  } else if (i < 100) {
    console.log(i, e.detail.name)
  }
}

const getBullet = (i) => ({
  eventCounter: i, // only for mocking!
  name: `objName-${i}`,
  bulletmove() {
    return new CustomEvent("bulletmove", {
      detail: {
        name: this.name,
        eventCounter: this.eventCounter,
      }
    })
  },
  fire() {
    const handler = setInterval(() => {
      window.dispatchEvent(this.bulletmove())
      this.eventCounter++
    }, 500)
    if (!window.bulletIntervals) window.bulletIntervals = {}
    window.bulletIntervals[this.name] = () => clearInterval(handler)
  },
})

const bullets = [
  getBullet(0),
  getBullet(1),
  getBullet(2),
]

const fireAll = (bullets) => {
  window.addEventListener("bulletmove", eventHandler)
  bullets.forEach((bullet) => {
    bullet.fire()
  })
}
fireAll(bullets)

Answer 4

Have you tried a clearTimeout method to stop the setTimeout from firing?

https://www.freecodecamp.org/news/javascript-settimeout-how-to-set-a-timer-in-javascript-or-sleep-for-n-seconds/

const fireBullet = setTimeout(function(){
    that.moveUp();
    window.dispatchEvent(that.bulletmove);
    that.fire();

},50);

clearTimeout(fireBullet)