Refactor lastAttackedBy to hurtBy #4894
Conversation
sim/pokemon.js
Outdated
| this.hurtBy.push(lastHurtBy); | ||
| } | ||
|
|
||
| get lastHurtBy() { |
There was a problem hiding this comment.
I don't like getters for various reasons, the main one being readability – it's not obvious from a glance that this property is automatically updated rather than something you need to write to.
data/moves.js
Outdated
| this.debug('Boosted for getting hit by ' + pokemon.lastAttackedBy.move); | ||
| let hurtByTarget = pokemon.hurtBy.find(function (x) { | ||
| return x.source === target && x.damage > 0 && x.thisTurn; | ||
| }); |
There was a problem hiding this comment.
You don't seem to use the found value. Did you mean some rather than find?
Also, your code style prefers new-style lambdas here:
let hurtByTarget = pokemon.hurtBy.some(h =>
p.source === target && p.damage > 0 && p.thisTurn;
);
There was a problem hiding this comment.
I use the found value in the debug output to get the move that triggered the boost. If that's not too important I can use some and a more generic debug statement.
|
@Marty-D thoughts? |
|
Looks good to me @Zarel. We can deal with splitting "thisTurn"-specific stuff from past turn stuff at a later date. |
|
Cool, thank you! |
Messed up pretty bad with my last pull request and had to open a new one. I apologize, still getting used to git. That said, I added the lastHurtBy getter as requested by Marty-D.
Original description:
Replaced the 'lastAttackedBy' attribute of a Pokemon with an array of attacks that the Pokemon has recently received. This corrects a bug in double battles arising from lastAttackedBy always getting replaced by the most recent attack, which caused Avalanche and Revenge to not double in power when they otherwise should have (e.g. see https://replay.pokemonshowdown.com/gen7doublescustomgame-796785834). This also addresses the corresponding task in #2444.