setTimeout not working properly inside a constructor function method

Question

I am trying to create an amateur mmorpg JavaScript browser game, multiplayer actually. This is a mmorpg so I am going to generate enemies randomly (doesn't matter now), the point is I have made a constructor function which creates a separate object, not linked to the prototype (which is nice). This separate object will be some individual npc, the amount will depend on how many we generate.

function myEnemy(posX,posY) {
    this.uuid = 'aa1b05',
    this.health = 2000,
    this.shield = 2000,
    this.posX = posX,
    this.posY = posY,
    this.speed = 320,
    this.damage = 80,
    this.ratio = 1000,
    //this.target = 'uuid',
    this.aggro = true,
    this.strike = function() {
        this.health -= this.damage
        console.log(this.health)
        //if(this.aggro === true) { setTimeout(this.strike(), this.ratio); }
    }
    this.attack = function() {
        if(this.aggro === true) { setTimeout(this.strike(), this.ratio); }
    }
};

Don't bother about the uuid, it's work in progress. The enemy does have some basic stats, health, position, speed, etc. I have removed some methods from this example for simplicity.

This code here spawns an enemy: let enemy = new myEnemy(0,0)

In this example I am just reducing the enemy's health each time it strikes, because I haven't implemented the targeting system yet. It's working fine, the problem comes once this.attack() is executed it's it only strikes once.

The other problem is the reason why I commented that line in this.strike(), instead of repeating the action each second it repeats the action so fast that it crashes my computer.

As far as I have gone, no matter what I try, setTimeout doesn't work.

My goal is to have the enemy do the strike function while aggro is = true, I have also tried do while loops without any success, the problem is setTimeout.

Update

This is not a duplicate, I have also tried to remove the () from the strike function without success, it attacks once and then returns NaN, stopping the loop.

You absolutely need to remove the () after this.strike. You want to pass the function to setTimeout, not the result of its execution. — Aioros, Jul 13 '19 at 03:23
Removing the calling parenthesis is still necessary to avoid calling the method immediately ([the previous suggested duplicate](https://stackoverflow.com/questions/7137401/)). You also have to setup retaining the value of `this` that you expect when it is called – [How to access the correct `this` inside a callback?](https://stackoverflow.com/questions/20279484/) — Jonathan Lonowski, Jul 13 '19 at 03:24
`The other problem is the reason why I commented that line in this.strike(), instead of repeating the action each second it repeats the action so fast that it crashes my computer.` Do you have an error message? Have you tried to catch the error? — wahwahwah, Jul 13 '19 at 03:30
Thank you guys it's already fixed by @backtick :) I also managed to get rid of the uuid and store all the enemies in the same array, using the [ ] position as their id — , Jul 13 '19 at 16:15

score 0 · Answer 1 · answered Jul 13 '19 at 03:26

0

setTimeout(this.strike(), this.ratio)

should be

setTimeout(() => this.strike(), this.ratio)

The setTimeout function's first argument is itself a function. You want this.strike to be called later, and this is how you defer that execution.

answered Jul 13 '19 at 03:26

backtick

2,685
10
18

This works because (if I recall correctly) the scope of a lamba is FORCED to be the same as the calling scope, just to note. It is a valid solution certainly, but I generally try not to make a lambda when I only have a single function to call - just pass the ref (perhaps scope-bound if needed) – josh.trow Jul 13 '19 at 03:27
1

Granted for `setTimeout` it's a bit much, just a habit of mine to avoid passing references. I've observed it to be a source of subtle bugs. I like controlling the arguments with which the callback is invoked, something you hand off to the implementation when you pass a reference. It's a cautious habit honestly come by. – backtick Jul 13 '19 at 03:37
This works perfectly =D I don't understand why but it does! – Jul 13 '19 at 16:09
@Chicler I strongly suggest you to really learn why - if you are making a game in Javascript, then I suspect scope will come back to bite you many, many more times – josh.trow Jul 13 '19 at 21:13
thanks @josh.trow but my problem I think is data management, I'm actually sorting everything with arrays and just realised the automatic arrangement breaks my system, if there is any way to disable it I'm saved – Jul 14 '19 at 08:16

score 0 · Answer 2 · edited Jul 13 '19 at 16:14

setTimeout takes a reference to a function - you are actually executing it. You either need to wrap the call in a new function (not needed here!) or remove the parenthesis - potentially you will need to add a bind call as well since you are using this inside your method, but you could also do a let that = this at the top of your constructor and use that everywhere instead of this

// BAD
this.strike = function() {
    this.health -= this.damage
    console.log(this.health)
    if(this.aggro === true) { setTimeout(this.strike(), this.ratio); } //<== This line crashes your comp because it's just recursing in to 'strike' calls
}

// OK
this.strike = function() {
    this.health -= this.damage
    console.log(this.health)
    if(this.aggro === true) { setTimeout(this.strike.bind(this), this.ratio); } //<= we make sure 'this' will ref the right object here, and pass the REFERENCE of the method to setTimeout, not the RESULT of a call
}

// Also OK maybe?
this.strike = function() {
    this.health -= this.damage
    console.log(this.health)
    if(this.aggro === true) { setTimeout(this.strike, this.ratio); }
}.bind(this)

setTimeout not working properly inside a constructor function method

2 Answers2