oyachai / HearthSim

Generic Hearthstone game simulator
MIT License
314 stars 59 forks source link

Implemented Lightbomb #123

Closed Hofls closed 9 years ago

oyachai commented 9 years ago

Thanks! One thing, I think Lightbomb is affected by spell damage, so the second to last argument to takeDamageAndNotify should be true.

Hofls commented 9 years ago

Sorry for bad english. Thanks for feedback. But if second to last argument to takeDamageAndNotify is true, then Lightbomb is affected by spellpower twice.
I guess i should not use EffectCharacterDamageSpell?

oyachai commented 9 years ago

I see... I think it's dangerous to use EffectCharacterDamageSpell in this situation. I think what happens now is that first, it damages all minions by 0 + spell damage (default damage_ is 0), and then it damages all minions by their attack value. So, each minion will be damaged twice, triggering on-damage effects twice.

The proper thing to do is to define a new Effect, something like

public class EffectCharacterDamageByAttack<T extends Card> implements EffectCharacter<T> {

    private final boolean effectedBySpellpower;

    public EffectCharacterDamageByAttack(int damage) {
        this(damage, false);
    }

    protected EffectCharacterDamageByAttack(boolean effectedBySpellpower) {
        this.effectedBySpellpower = effectedBySpellpower;
    }

    @Override
    public HearthTreeNode applyEffect(PlayerSide targetSide, CharacterIndex targetCharacterIndex, HearthTreeNode boardState) {
        Minion targetMinion = boardState.data_.modelForSide(targetSide).getCharacter(targetCharacterIndex);
        return targetMinion.takeDamageAndNotify(targetMinion.getTotalAttack(), PlayerSide.CURRENT_PLAYER, targetSide, boardState, this.effectedBySpellpower, true);
    }
}

You can then return this in getAoeEffect() and not have a use_core() override.

Hofls commented 9 years ago

Thank you!