Issue with new actors and initialization

[dgpark...@gmail.com]

Hello guys!

Just started using excaliburjs for a new project and I'm really excited. The fact that it's written in typescript and using typescript for implementation is so so so nice! API documentation is superb btw. 

In regards to my question, I have an actor that essentially is creating additional actors based on a keyboard event. 

This is a new class which extends actor obviously and on the key event I create a new instance of my other actor class which also extends actor.

I'm doing this through the update function which is listening for keyboard events and then using the passed in engine parameter to add it to the current scene.

if (engine.input.keyboard.wasPressed(Input.Keys.Space)) {

const bomb = new Bomb({xPos: this.x, yPos: this.y})

engine.currentScene.add(bomb);

}

This behaves as normal I believe which actually adds my actor to the scene(confirmed by logging out the currentScene) and it is actually visible in game.

however I tried to perform some additional configuration inside the onInitialize function on my Bomb class but it never gets called.

The update function performs as expected and logging out isInitialized always returns false. This is also confirmed by logging the currentScene and inspecting the actor property which is also false.

It seems as if actors that are added in this manner just never get initialized. My first assumption is that I'm just not implementing it right. 

Is this expected behavior, a bug, or something else? Any help is greatly appreciated.

Bomb class:

import { Actor, Color, Input, Timer } from 'excalibur';

import { KeyEvent } from 'Input/Index';

export class Bomb extends Actor {

   constructor(bomberPos: any){

       super()

       this.setWidth(10);

       this.setHeight(10);

       this.x = bomberPos.xPos;

       this.y = bomberPos.yPos;

       this.color = new Color(0, 0, 0);

   }

   public update(engine, delta) {

       //This behaves as expected

       console.log(engine);

   }

    //This never gets called

   public onInitialize(engine) {

       console.log('init')

       const fuse = new Timer( () => {

           this.kill;

           console.log('fuse set');

       }, 5);

   }

}

Bomber class:

import { Actor, Color, Input } from 'excalibur';

import { KeyEvent } from 'Input/Index';

import { Bomb } from '../bombs/bombs.actor';

export class Bomber extends Actor {

constructor(){

super()

this.setWidth(25);

this.setHeight(25);

this.x = 150;

this.y = 150;

this.color = new Color(255, 255, 255);

}

public update(engine, delta) {

//manage movement

if (engine.input.keyboard.isHeld(Input.Keys.W) || engine.input.keyboard.isHeld(Input.Keys.Up)) {

this.y -= 1;

} else if (engine.input.keyboard.isHeld(Input.Keys.S) || engine.input.keyboard.isHeld(Input.Keys.Down)) {

this.y += 1;

} else if (engine.input.keyboard.isHeld(Input.Keys.A) || engine.input.keyboard.isHeld(Input.Keys.Left)) {

this.x -= 1;

} else if (engine.input.keyboard.isHeld(Input.Keys.D) || engine.input.keyboard.isHeld(Input.Keys.Right)) {

this.x += 1;

}

//manage skills

if (engine.input.keyboard.wasPressed(Input.Keys.Space)) {

const bomb = new Bomb({xPos: this.x, yPos: this.y})

engine.currentScene.add(bomb);

console.log(engine.currentScene);

}

}

}

from package.json for versions:

"dependencies": {

"excalibur": "^0.14.0"

},

"devDependencies": {

"ts-loader": "^3.2.0",

"typescript": "^2.6.2",

"webpack": "^3.10.0",

"webpack-dev-server": "^2.9.7"

}

npm version: 5.4.2

node version: 8.9.1

Thanks again

[Kamran Ayub]

It's time for bed here so I can't delve too deeply yet, but one thing that jumps out at me is the super easy thing to overlook where you don't add "super.<method>(...)" to overridden methods like "update". onInitialize doesn't need it (except for UI Actors, which is a defect and we have an open issue for I believe).

We do have an issue to help make it easier on users by not requiring using the super call but for now for any methods on Actor that you override, you have to call super.methodName. Because you aren't calling the base implementation, no logic is run for initialization (or any other base update logic). It's easy to forget which is why we want to remove the need to do it for 99% of cases. Add a thumbs up here if you like that! https://github.com/excaliburjs/Excalibur/issues/582

Another protip: you can use engine.add which is shorter, or if you have a single engine, you can make it global and not have to rely on parameters within Actor.

Hope that helps!

[dgpark...@gmail.com]

My current work around until I can figure out how to handle it better.

On my bomb class:

public startFuse() {

setTimeout( () => {

this.kill();

console.log('killed');

}, 3000)

console.log('started fuse');

}

On my bomber class:

const bomb = new Bomb({xPos: this.x, yPos: this.y})

engine.currentScene.add(bomb);

bomb.startFuse();

[Kamran Ayub]

Take a look at my earlier reply, I think it'll fix it but I'll also recommend never using the setTimeout/setInterval JS methods with Excalibur games within the mainloop; use the Excalibur Timer class instead which hooks into the engine main loop and ensures a constant update rate in time with your game (the JS ones are outside the game event loop).

Note to self: this is maybe good advice to add to the user guide.

--
You received this message because you are subscribed to the Google Groups "excaliburjs" group.
To unsubscribe from this group and stop receiving emails from it, send an email to excaliburjs...@googlegroups.com.
To post to this group, send email to excal...@googlegroups.com.
Visit this group at https://groups.google.com/group/excaliburjs.
To view this discussion on the web visit https://groups.google.com/d/msgid/excaliburjs/eaf38faa-6878-4a95-943a-1c15abccffb9%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[dgpark...@gmail.com]

Wow, nice catch! Not having to call it on the update function really threw me off!

Thanks for the fast reply! Going to make this change asap.

Also thanks for the setTimeout tip. 

[Kamran Ayub]

No problem, since you're brand new, please keep sending us pain points and feedback! It'll help us make Ex easier to use.

To view this discussion on the web visit https://groups.google.com/d/msgid/excaliburjs/0fac6ffb-6f5c-4504-9dae-f4c8ba644177%40googlegroups.com.

[dgpark...@gmail.com]

So I've done some additional testing after implementing what you suggested and it still seems to be behaving rather oddly. Even after adding the super call, onInitialize never gets called.

I've even commented out my override and checked for events that are part of the Actor lifecycle and the same is true. It seems that it's just never going through the initialize step which is odd because that seems to be the first event that should be fired after being added to the scene.

Even logging of the current scene and inspecting the actor the property isInitialized is set to false.

I know in the doc's it says _initialize isn't meant to be called by the users but it seems like the only way to force it through the initialize event.

[dgpark...@gmail.com]

What's interesting as well, is that if you force the initialize through _initialize call, the super call is no longer needed. This is also true with the update function (no super call needed)  even without forced _intialize. Which if I read your earlier post correctly, you were saying that you 'need' to make the call to the base method.

forcing initialize function

        if (engine.input.keyboard.wasPressed(Input.Keys.Space)) {

           const bomb = new Bomb({xPos: this.x, yPos: this.y})

           engine.currentScene.add(bomb);

            //force actor initialization

           bomb._initialize(engine);

       }

and in the actor class:

    public onInitialize(engine) {

       console.log('onInitialized ran');

       console.log(this.isInitialized);

       setTimeout(() => {

           console.log(this.isInitialized);

       }, 3000);

   }

as you can see there is no super call there, and everything runs as expected. The first log of isInitialized returns false and the second one inside the timeout returns true. 

[Kamran Ayub]

Hmm, well if you want to try making a repro public we could take a look? You added super.update(engine, delta) in all the update looks you overrode then?

[dgpark...@gmail.com]

Ok I just made my repo public so you can take a look. I also just committed what I've done tonight so you have exactly whats on my local right now.

here's the link:

https://github.com/dgparker/project-countdown

[Erik Onarheim]

Hi Dylan,

I was able to correct this issue with the initialize by removing the extra update in the bomb.actor.ts, also I added a super.update(...) to bomber.actor.ts.

I've opened a PR on your repo with a correction https://github.com/dgparker/project-countdown/pull/1

Overriding update without calling super.update will cause unintended side-effects currently in excalibur. We do have an issue in this milestone to make this a bit more bullet proof https://github.com/excaliburjs/Excalibur/issues/582

Let me know if this helps,

Erik