Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check if unit is alive after effects #63

Open
olistic opened this issue May 8, 2018 · 8 comments
Open

Check if unit is alive after effects #63

olistic opened this issue May 8, 2018 · 8 comments
Labels
good first issue Looking to contribute? Start here! pkg: core Pertaining to @warriorjs/core status: claimed Issues that are currently being worked on by someone type: enhancement A potential improvement to something that already exists

Comments

@olistic
Copy link
Owner

olistic commented May 8, 2018

A negative effect could potentially kill a unit. We need to check if the unit is still alive after executing the effects and return early if the unit died. Example: a "poisoned" effect (it doesn't exist, at least yet) that damages the unit turn by turn.

@olistic olistic added good first issue Looking to contribute? Start here! pkg: core Pertaining to @warriorjs/core type: enhancement A potential improvement to something that already exists labels May 8, 2018
@glneto
Copy link
Contributor

glneto commented May 21, 2018

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.

@olistic
Copy link
Owner Author

olistic commented May 21, 2018

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?

@olistic olistic changed the title Check if unit is alive after effect Check if unit is alive after effects May 21, 2018
@abenezeradane
Copy link

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.`);
	}
    }
}

@olistic
Copy link
Owner Author

olistic commented May 23, 2018

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.

@pigalot
Copy link
Contributor

pigalot commented May 23, 2018

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

@msaikaushik
Copy link

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

@olistic olistic added the status: claimed Issues that are currently being worked on by someone label Aug 10, 2018
@olistic
Copy link
Owner Author

olistic commented Aug 10, 2018

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

@glneto
Copy link
Contributor

glneto commented Jan 21, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Looking to contribute? Start here! pkg: core Pertaining to @warriorjs/core status: claimed Issues that are currently being worked on by someone type: enhancement A potential improvement to something that already exists
Projects
None yet
Development

No branches or pull requests

5 participants