demilich1 / metastone

Hearthstone simulator written in Java with full GUI support
GNU General Public License v2.0
132 stars 80 forks source link

(Baron Rivendare + Skeleton Knight / Anub'Arak issues) ReturnMinionToHandSpell should check if the minion is still on the battlefield before trying to remove it #346

Open doctorpangloss opened 7 years ago

doctorpangloss commented 7 years ago

ReturnMinionToHandSpell from deathrattles shouldn't try to getLogic().removeMinion if the minion isn't on the battlefield anymore. Otherwise, if Baron Rivendare is in play, two copies of the minion will appear in the set aside zone and glitch out effects further down the line.

I think this issue can be resolved if the second battlecry action checks that its target is still valid. This might look something like...

final EntityReference target = battlecryAction.getSpell().hasPredefinedTarget() ? battlecryAction.getSpell().getTarget() : battlecryAction.getTargetKey();
final boolean targetable = target == null || target.isTargetGroup() ||
    /* The target is still among the valid targets for the action */
    getValidTargets(playerId, battlecryAction).stream().map(EntityReference::pointTo).anyMatch(er -> er.equals(target));
if (!battlecryAction.canBeExecuted(context, player) || !targetable) {
    return;
}
webadict commented 7 years ago

That's a good call and a good issue to bring up. This is actually pretty important.