Check if unit is alive after effects about warriorjs HOT 8 OPEN

I took a look at this issue, but I imagine that if an effect that damages the unit is created, it would call the unit.takeDamage which already verifies if the unit died. Am I missing something here? Or maybe I misunderstood the issue.

from warriorjs.

I think this will be clearer if I explain how the turns are played by the units in the floor:

1. In the order the units were added to the floor, their prepareTurn method is executed. This is when the playTurn method of each unit is called, executing the senses immediately but deferring the action for later. Nothing changes in the game at this point; units just analyze their environment to decide what the best course of action is.
2. In that same order, the performTurn method of each unit is executed. This is when the actions that were deferred for later are executed, having impact in the game.

Example:

╔════════╗
║   @s  >║
╚════════╝

We have the Warrior there, next to a Sludge. The warrior is full health, but the sludge is about to die (one hit from the Warrior and it's dead). During (1), both the Warrior and the Sludge decide to attack. Later in (2), the Warrior attacks first, killing the Sludge. The Sludge had already decided that it was going to attack, but it wouldn't be ok if he was able to do so because he's now dead. That's why this check exists.

Now, what this issue is about is putting a similar check in place for the effects. The effects are executed here, during performTurn, and can have an impact on the game (e.g. kill the affected unit).

Example:

Our Warrior is about to die (1 HP) and he's under the "poisoned" effect, that subtracts 2 HP each turn. The poison takes effect here, killing the Warrior. It wouldn't be ok if the Warrior was able to the action after, because he's now dead. So another isAlive() check is needed.

Open questions:

• Where do we put the isAlive() check?
  a. Once after executing the passTurn of each of the effects
  b. Once after executing the passTurn of all the effects

• Do we want to execute the effects before the action (as it's currently being done) or after?

from warriorjs.

Thanks for that response, it really cleared things up. The isAlive() check should occur after the execution of each effect, checking if the current health of the player is at or greater than 0. Unless I am wrong, the performTurn method should be written like this for proper implementation.

performTurn() {
    if (this.isAlive()) {
        this.effects.forEach(effect => effect.passTurn());
	if(this.isAlive()){
            if (this.turn.action && !this.isBound()) {
                const [name, args] = this.turn.action;
                this.abilities.get(name).perform(...args);
            }
	}
	else {
	    this.log(`You have $(this.health) out of $(this.maxHealth). You are dead.`);
	}
    }
}

from warriorjs.

Hey @PB020, I encourage you to open a PR if you want to try a fix for this! We can discuss the code there with better instruments.

Here are some comments regarding what you already posted though:

this.effects.forEach(effect => effect.passTurn());
if (this.isAlive()) {

There you're only checking once, after all effects have been executed. In order to check after each effect, the check needs to be done inside the forEach. But to be able to return early, the forEach needs to be replaced with a for..of loop.

else {
  this.log(`You have $(this.health) out of $(this.maxHealth). You are dead.`);
}

This code is redundant and should be removed. If an effect kills the unit, this will be logged by the effect already.

from warriorjs.

You can use some if you need to exit early rather than a for..of loop.

from warriorjs.

@olistic , @PB020 , can i work on this?

from warriorjs.

@icedune Sure! Let me know if you have any questions.

from warriorjs.

It's been a while since the last comment on this, so I opened #250 - hope it works as expected. If there is any case I missed let me know.

from warriorjs.