0

I have two functions inside my Class. First fires timeout and looped it (works like interval) and second to clearTimeout. My problem is, that clearTimeout doesn't work. How can I fix it?

this.startMove = function() {
    setTimeout(function handler() {
        self.moveMonster(moveMonsterCallback);
        setTimeout(handler, self.speed);
    }, this.speed);
};

this.stopMove = function() {
    clearTimeout(self.startMove());
}

For example I want to run these functions on click.

  • Possible duplicate of [Stop setInterval call in JavaScript](https://stackoverflow.com/questions/109086/stop-setinterval-call-in-javascript) – PRMoureu Aug 25 '17 at 20:06
  • 1
    better use https://developer.mozilla.org/en-US/docs/Web/API/window/requestAnimationFrame for animating something in your page. Also be aware of the fact, that the delay of setTimeout is not guaranteed! It can differ – Joshua K Aug 25 '17 at 20:06
  • well you are creating a new interval when you call startMove in stopMove.... – epascarello Aug 25 '17 at 20:07
  • PRMoureu, its simple clear interval. I have more complicated problem and your suggest doesn't work for me JoshuaK, thanks I will check it, but now i I have to resolve problem by way i wrote above epascarello you are right. I desperately have trying all possible combinations :) – michalgrzasko Aug 26 '17 at 12:01

3 Answers3

1

You need to assign your timeout to a handler:

this.startMove = function() {
    self.timeout = setTimeout(function handler() {
        self.moveMonster(moveMonsterCallback);
        setTimeout(handler, self.speed);
    }, this.speed);
};

this.stopMove = function() {
    clearTimeout(self.timeout);
}

Edit

As michalgrzasko mentions, the above code doesn't work. The reason being that the handle is assigned to the wrong timeout function. The outer timeout is only set once, while the inner timeout is called in a recursive loop, thus that is the timeout which needs to be cleared.

this.startMove = function() {
    setTimeout(function handler() {
        self.moveMonster(moveMonsterCallback);
        self.handle = setTimeout(handler, self.speed);
    }, this.speed);
};

this.stopMove = function() {
    clearTimeout(self.timeout);
}

However

Again, as michalgrzasko and several others point out, the better solution is to use setInterval, as it is easier to read, and thus safer:

this.startMove = function() {
    self.moveInterval = setInterval(function handler() {
        self.moveMonster(moveMonsterCallback);
    }, this.speed);
};

this.stopMove = function() {
    clearInterval(self.moveInterval);
}
Community
  • 1
  • 1
Michael Horn
  • 3,819
  • 9
  • 22
  • He's creating a new timeout-Handler, not an interval-handler. I'm not sure if it's fail safe, if `this.speed` is very small. – Joshua K Aug 25 '17 at 20:11
  • @JoshuaK It's safe. Either `stopMove()` runs before the timeout occurs, and it clears it. Or it runs after, and clearing it has no effect. JS is single-threaded so there's no concurrency problem. – Barmar Aug 25 '17 at 20:13
  • @Barmar yeah. but self.timeout holds only the first created handle and is never overridden by the setTimeout call **inside** the handler function. He calls handler and not this.startMove after the first call. It wont work if I see it clear – Joshua K Aug 25 '17 at 20:16
  • first why would you need to call setTimeout inside setTimeout ? Just call the this.startMove() again. ...; which in this case would self.startMove(). – trk Aug 25 '17 at 20:17
  • @82Tuskers in this case you're right. But it makes sense if you want to do more stuff when the animation starts, but not in every step (showing a message "Animation started" or something like this). – Joshua K Aug 25 '17 at 20:23
  • in this case the question is: why using setTimeout instead of setInterval? If `moveMonster` needs less time than self.speed it's the easier solution – Joshua K Aug 25 '17 at 20:25
  • This solustion doesn't work for me. On execute stopMove(), interval still is running. Guys, honestly I don't know why I am using timeout instead interval. I read that timeout is safer, isn't it? – michalgrzasko Aug 26 '17 at 12:06
  • Ah, after taking a better look I see I assigned the wrong timeout to the handle. It should have been assigned to the timeout inside of handler(), since that's the one which gets recursively called. Having said that, yes I think it would be both simpler and safer to use an interval – Michael Horn Aug 26 '17 at 18:42
0

Let me know if this works for you :

this.startMove = function() {
    self.timeout = setTimeout(function handler() {
        self.moveMonster(moveMonsterCallback);
        self.startMove();
    }, self.speed);
};

this.stopMove = function() {
    clearTimeout(self.timeout);
}

So basically I have reused the @Michael Horn's solution. And, I am happy to use this answer as an to his edited answer (if the question resolves).

trk
  • 2,106
  • 14
  • 20
  • Yes, this is what I need. I don't know, why I have using handler() and complicated function, if it's so easy. Probably I have mixed different solutions found in the Internet. Thanks! :) – michalgrzasko Aug 26 '17 at 15:44
0

If you use setTimeout or setInterval and want the ability to cancel them, you need to save the returnvalue of this function in a variable. The return-value is a handler, that can be used in clearTimeout or clearInterval.

In this scenario setInterval would be easier if the moveMonster function has a smaller time consumption than this.speed.

this.startMove = function() {
    this.startMove.intervalHandler = setInterval(function handler() {
        self.moveMonster(moveMonsterCallback);
    }, this.speed);
};

this.stopMove = function() { clearInterval(this.startMove.intervalHandler); }

I prefer using the function scope to save such things like Handler. If you don't like it use the parent scope or the context

If it's important to use setTimeout instead of setInterval you have solution in other answers. It's simply the same: Save the returnvalue of setTimeout und use it later with clearTimeout.


If moveMonsterCallback should be called if the animation is over, take a look at promises instead of callbacks. With callbacks you're getting fast in a position, it's hard to track the callings.


A word on animations: They're tricky! Don't use setTimeout. Better call requestAnimationFrame on the window object.

function step(timestamp) {
  if (!step.startTime) step.startTime = timestamp;
  var delta = step.getDelta(timestamp);
  // do the calculations of how far you have to move the monster in this time range
  // move the monster...
  ...
  // animation is stopped? If not: We want to animate another timeframe
  if (!step.stopped) {
    window.requestAnimationFrame(step);
  }
}
step.startTime = null;
step.stopped = false;
step.getDelta = (timestamp)=>timestamp-step.startTime;

function stopMove() {
  step.stopped = true;
}

window.requestAnimationFrame(step);

This solution provides a better result, because it's independent of the precision of setTimeout. It works with time differences

Joshua K
  • 2,407
  • 1
  • 10
  • 13
  • Hi, thanks for such extensive response! My moveMonster function changes the property of monster instance (eg. monster.x += 1 / monster.y += -1, etc.) depends to monster.direction (right, left, up, down). I want every 500ms change the values of x and y and execute callback function which changes css in DOM element. So... moveMonster isn't anything what i could animate. I don't know how to combine requestAnimationFrame with my solution. I am pasting here my code only to look at my functions to better understanding my Class: https://jsfiddle.net/pesxgacu/ (obviously here it dosen't work) – michalgrzasko Aug 26 '17 at 16:00