Two ways of implementing a combat system

Question

Below is a segment of a game I am writing to stay in practice with my Java.

EDIT The class this method is in is the Encounter class. This class just manages the encounter between the player and an enemy, and the other methods are victory() and defeat(), which add gold/experience when player wins, and removes gold when player loses.

There are two ways I am thinking of implementing a combat system, but I am not sure which path is more efficient. The code works perfectly fine; I am only inquiring about the efficiency:

EDIT 2 Included the entire class

Option 1

import java.util.Scanner;

public class Encounter {

    private static Scanner sc;
    private static Player player;
    private static Enemy enemy;

    public static void manage(Player p, Enemy e) {

        player = p;
        enemy = e;

        sc = new Scanner(System.in);

        System.out.println("You are fighting a " + enemy.getName() + "!");
        while(player.getHealth() > 0 && enemy.getHealth() > 0) {

            System.out.println(player.getName() + "'s Health/Mana: " + player.getHealth() + "/" + player.getMana());
            System.out.println(enemy.getName() + "'s' Health/Mana: " + enemy.getHealth() + "/" + enemy.getMana());
            Ability playerAbility = selectAbility();

            //Player attacks enemy => Enemy loses health => player loses mana
            enemy.subtractHealth(playerAbility.getDamage());
            player.subtractMana(playerAbility.getManaCost());

            //Enemy attacks player => Player loses health
            player.subtractHealth(enemy.attackPlayer());
        }

        if(player.getHealth() <= 0) {
            defeat();
        } else if(enemy.getHealth() <= 0) {
            victory();
        }

    }

    private static void defeat() {
        System.out.println("=======================");
        System.out.println("You died to " + enemy.getName() + "!");
        System.out.println("=======================");
        //Handle death here
        player.deathSubtraction();
        player.resetHealthMana();
    }

    private static void victory() {
        System.out.println("You defeated " + enemy.getName() + "!");
        //Handle experience/gold/item(s) here
        player.addExperience(enemy.getExperience());
        player.addGold(enemy.getGold());
        player.checkLevelUp(player);
        displayStats();
        player.resetHealthMana();
        System.out.println("Press enter to continue");
        sc.nextLine();
    }

    private static Ability selectAbility() {
        player.displayMoves();
        while(true) {
            String input = sc.nextLine();
            for(Ability ability : player.getAbilities()) {
                if(input.equalsIgnoreCase(ability.getName()) && ability.getManaCost() < player.getMana()) {
                    return ability;
                }
            }
            System.out.println("Not a valid move/Not enough mana! Select again!");
        }
    }

    private static void displayStats() {
        System.out.println("============================");
        System.out.println("Current Level: " + player.getLevel());
        System.out.println("Current Experience: " + player.getExperience() + "/100");
        System.out.println("Current Gold: " + player.getGold());
        System.out.println("============================");
    }

}

Option 2

import java.util.Scanner;

public class Encounter {

    private static Scanner sc;
    private static Player player;
    private static Enemy enemy;

    public static void manage(Player p, Enemy e) {

        player = p;
        enemy = e;

        sc = new Scanner(System.in);

        System.out.println("You are fighting a " + enemy.getName() + "!");
        while(true) {

            System.out.println(player.getName() + "'s Health/Mana: " + player.getHealth() + "/" + player.getMana());
            System.out.println(enemy.getName() + "'s' Health/Mana: " + enemy.getHealth() + "/" + enemy.getMana());
            Ability playerAbility = selectAbility();

            //Player attacks enemy => Enemy loses health => player loses mana
            enemy.subtractHealth(playerAbility.getDamage());
            player.subtractMana(playerAbility.getManaCost());

            //Enemy attacks player => Player loses health
            player.subtractHealth(enemy.attackPlayer());

            if(player.getHealth() <= 0) {
                defeat();
                break;
            } else if(enemy.getHealth() <= 0) {
                victory();
                break;
            }
        }

    }

    private static void defeat() {
        System.out.println("=======================");
        System.out.println("You died to " + enemy.getName() + "!");
        System.out.println("=======================");
        //Handle death here
        player.deathSubtraction();
        player.resetHealthMana();
    }

    private static void victory() {
        System.out.println("You defeated " + enemy.getName() + "!");
        //Handle experience/gold/item(s) here
        player.addExperience(enemy.getExperience());
        player.addGold(enemy.getGold());
        player.checkLevelUp(player);
        displayStats();
        player.resetHealthMana();
        System.out.println("Press enter to continue");
        sc.nextLine();
    }

    private static Ability selectAbility() {
        player.displayMoves();
        while(true) {
            String input = sc.nextLine();
            for(Ability ability : player.getAbilities()) {
                if(input.equalsIgnoreCase(ability.getName()) && ability.getManaCost() < player.getMana()) {
                    return ability;
                }
            }
            System.out.println("Not a valid move/Not enough mana! Select again!");
        }
    }

    private static void displayStats() {
        System.out.println("============================");
        System.out.println("Current Level: " + player.getLevel());
        System.out.println("Current Experience: " + player.getExperience() + "/100");
        System.out.println("Current Gold: " + player.getGold());
        System.out.println("============================");
    }

}

I am unsure if I should just break when the conditions are met, or have the condition be evaluated in the while loop. I feel that having while(true) can avoid any bugs, and a simple break will ensure that it exits the loop. Any and all suggestions are appreciated and considered.

Answer 1

• You've used a readable idiomatic Java naming convention.
• There is no point reassign p to player and e to enemy, use the full name on entry, make the parameters final.
• Static methods are a code smell in OO programming, they will result is tightly coupled code. They should be studiously avoided until you know the specific circumstances that make them necessary.
• Follow the tell don't ask idiom, tell the player to attack the enemy and tell the enemy to attack the player which will minimise coupling if you use an interface.
• Never use while(true). In this case I would use a do { .. } while(...) loop since you always want this loop to be performed once. Use while(...) when it could be 0..N and do { .. } while(...) when the loop is 1..N.
  • Use a single condition, probably the player's input or battling continues for your loop control for clarity.
• Put the health display inside the Player and Enemy classes (or a common base class) and show it during players and enemy turn.
• Pass the gold & experience as a parameter to the victory method on the player.

Something like this:

public void manage(Player player, Enemy enemy) {
    boolean battling = true;    
    do {
        player.turn(enemy);
        enemy.turn(player);
        battling = player.isAlive() && enemy.isAlive();
        } while(battling);
    }
}

• In the Player and Enemy classes have a Opponent interface (follow the Interface Segregation Principle) that includes a turn method to makes attack and takes damage. The instances of Player and Enemy sends damage to the opponent and reduces their karma. Getting and Setting is a code smell that increases coupling.

e.g.

public turn(Opponent character) {  
    displayStatus();
    attack = chooseAttack();
    reduceMana(attack);
    character.damage(attack);
}

public damage(Attack attack) {    
    hp - hp - attack.damage;
}