Tukukkk

I have two issues/questions with figuring out a proper way to write this code for a player leveling system.

My First question/issue is that when this code is run it works as intended with a slight problem. public void levelUp() is being used to, "set", the level if you will (I am sure this is a sloppy way of doing this but we will get to that in the next question) while levelUpXp() is being used as the only way I currently know how to store the elements of int requiredXP to let the game know when the player has reached X amount of xp to level him up to X level. ding() is simply a notification for the player to be aware that they have increased in level.

As stated before this all works the way I am wanting it to work however upon implementing ding() after a countless amount of time trying to write multiple if statements to do what ding is doing now, I have run into an issue where curXP is NOT equal to either reqXP or requiredXP due to the amount of XP the player is gaining not always being an exact number within requiredXP. While I already assumed this would happen, it is now resulting in either a level up notification not being sent because curXP == requiredXP[x] is not always the case if the amount of xp needed to level is 17 but after killing an enemy the player goes from 15xp to 18xp or curXP <= requiredXP[x] causing the level up notification to be constantly sent until that player reaches their next level in which the notification is still sent however with a new level attached.

Second question will be below code.

Here is my player class and methods:

public class Player extends Creature {



int health = 100;

int maxHealth = 100;

int attackDamage = 25;

int numHealthPotions = 3;

int healthPotHeal = 30;

int curXP = 0;

int level = 0;

int reqXP = 0;



int currentLevel = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

int requiredXP = { 0, 6, 17, 36, 65, 105, 158, 224, 305, 402 };



public void levelUp() {



    if (curXP == requiredXP[0]) {

        level = currentLevel[0];

    } else if (curXP == requiredXP[1]) {

        level = currentLevel[1];

    } else if (curXP == requiredXP[2]) {

        level = currentLevel[2];

    } else if (curXP == requiredXP[3]) {

        level = currentLevel[3];

    } else if (curXP == requiredXP[4]) {

        level = currentLevel[4];

    } else if (curXP == requiredXP[5]) {

        level = currentLevel[5];

    } else if (curXP == requiredXP[6]) {

        level = currentLevel[6];

    } else if (curXP == requiredXP[7]) {

        level = currentLevel[7];

    } else if (curXP == requiredXP[8]) {

        level = currentLevel[8];

    } else if (curXP == requiredXP[9]) {

        level = currentLevel[9];

    }

}



public void levelUpXp() {



    if (level == currentLevel[0]) {

        reqXP = requiredXP[0];

    } else if (level == currentLevel[1]) {

        reqXP = requiredXP[1];

    } else if (level == currentLevel[2]) {

        reqXP = requiredXP[2];

    }else if (level == currentLevel[3]) {

        reqXP = requiredXP[3];

    } else if (level == currentLevel[4]) {

        reqXP = requiredXP[4];

    }else if (level == currentLevel[5]) {

        reqXP = requiredXP[5];

    } else if (level == currentLevel[6]) {

        reqXP = requiredXP[6];

    }else if (level == currentLevel[7]) {

        reqXP = requiredXP[7];

    } else if (level == currentLevel[8]) {

        reqXP = requiredXP[8];

    }else if(level == currentLevel[9]) {

        reqXP = requiredXP[9];

    }

}



public void ding() {

    if(level == 2 && curXP == requiredXP[1]) {

        System.out.println(" #############################");

        System.out.println(" # You have reached level " + level + "! # ");

        System.out.println(" #############################");

    } else if(level == 3 && curXP == requiredXP[2]) {

        System.out.println(" #############################");

        System.out.println(" # You have reached level " + level + "! # ");

        System.out.println(" #############################");

    } else if(level == 4 && curXP == requiredXP[3]) {

        System.out.println(" #############################");

        System.out.println(" # You have reached level " + level + "! # ");

        System.out.println(" #############################");

    } else if(level == 5 && curXP == requiredXP[4]) {

        System.out.println(" #############################");

        System.out.println(" # You have reached level " + level + "! # ");

        System.out.println(" #############################");

    } else if(level == 6 && curXP == requiredXP[5]) {

        System.out.println(" #############################");

        System.out.println(" # You have reached level " + level + "! # ");

        System.out.println(" #############################");

    } else if(level == 7 && curXP == requiredXP[6]) {

        System.out.println(" #############################");

        System.out.println(" # You have reached level " + level + "! # ");

        System.out.println(" #############################");

    } else if(level == 8 && curXP == requiredXP[7]) {

        System.out.println(" #############################");

        System.out.println(" # You have reached level " + level + "! # ");

        System.out.println(" #############################");

    } else if(level == 9 && curXP == requiredXP[8]) {

        System.out.println(" #############################");

        System.out.println(" # You have reached level " + level + "! # ");

        System.out.println(" #############################");

    } else if(level == 10 && curXP == requiredXP[9]) {

        System.out.println(" #############################");

        System.out.println(" # You have reached level " + level + "! # ");

        System.out.println(" #############################");

    }

}



public int getLevel() {

    return level;

}



public void setLevel(int level) {

    this.level = level;

}



public void Level() {

    int maxLevel = 200;

    int maxLevelXP = 1000000;

    for (int currentLevel = 1; currentLevel < maxLevel; currentLevel += 1) {

        float x = currentLevel / (float) maxLevel;

        double y = Math.pow(x, 2.61);

        int requiredXP = (int) (y * maxLevelXP);

        System.out.println("Level " + currentLevel + " XP: " + requiredXP);

    }



}

Here is game class where ding and other previously mentioned Methods are used:

GAME: while (running) {



        p.levelUp();

        p.levelUpXp();

        System.out.println("-----------------------------------------------");



        String enemy = e.enemies[rand.nextInt(e.enemies.length)];

        int enemyHealth = e.enemyHealth;

        int preLevel = 1;

        int curLevel = preLevel + 1;



        System.out.println("t# " + enemy + " has appeared! #n");



        while (enemyHealth > 0) {



            System.out.println(

                    "t" + userName + "nt HP: " + p.health + " Level: " + p.level + " Exp: " + p.curXP + "nt" + "nt" + enemy + "nt HP: " + enemyHealth);

            System.out.println("nt What would you like to do?");

            System.out.println("t 1. Attack");

            System.out.println("t 2. Drink health potion");

            System.out.println("t 3. Run!");



            String input = in.nextLine();

            if (input.equals("1")) {

                int damageDealt = rand.nextInt(p.attackDamage);

                int damageTaken = rand.nextInt(e.maxEnemyAD);



                enemyHealth -= damageDealt;

                p.health -= damageTaken;

                if (damageDealt > 0) {

                    System.out.println("t> You strike the " + enemy + " for " + damageDealt + " damage!");

                } else if (damageDealt < 1) {

                    System.out.println("t> You attempt to hit the " + enemy + " but miss!");

                }

                if (damageTaken > 0) {

                    System.out.println("t> The " + enemy + " retaliates! You take " + damageTaken + " damage!");

                } else if (damageTaken < 1) {

                    System.out.println("t> The " + enemy + " retaliates but misses!");

                }

                if (p.health < 1) {

                    System.out.println(

                            "t ##################" + "nt # You Have Died! #" + "nt ##################");

                    break;

                }



            } else if (input.equals("2")) {

                if (p.numHealthPotions > 0 && p.health != p.maxHealth) {

                    p.health += p.healthPotHeal;



                    p.numHealthPotions--;

                    if (p.health > p.maxHealth) {

                        p.health = p.maxHealth;

                    }



                    System.out.println("t> You drink the Health Potion!" + "nt> You now have " + p.health

                            + " Health!" + "nt> You now have " + p.numHealthPotions + " Health Potion(s) left!");



                } else if (p.health == p.maxHealth) {

                    System.out.println("t> Your health is already full!");



                } else if (p.numHealthPotions < 1) {

                    System.out.println("t You have no Health Potions left!");

                }

            } else if (input.equals("3")) {

                System.out.println("t> You run away from the " + enemy + "!");

                continue GAME;

            } else {

                System.out.println("t Invalid Command!");

            }

        }

        if (p.health < 1) {

            System.out.println("You fought bravely but in the end, you're just another corpse...");

            break;

        }



        System.out.println("-----------------------------------------------");



        if (enemyHealth < 1) {

            p.curXP += 3;

            p.levelUp();

            p.levelUpXp();



        }



        System.out.println(" # The " + enemy + " was defeated! # ");

        System.out.println(" # You have " + p.health + " HP left! # ");

        System.out.println(" # You have gained " + e.xpGive + " xp! # ");

        p.ding();

        if (rand.nextInt(100) < e.dropChance) {

            p.numHealthPotions++;

            System.out.println(" # The " + enemy + " droppped a Health Potion! # ");

            System.out.println(" # You now have " + p.numHealthPotions + " Health Potion(s)! # ");

        }



        System.out.println("-----------------------------------------------");

        System.out.println("tWhat would you like to do?");

        System.out.println("t1. Press on!");

        System.out.println("t2. Get me out of here!");



        String input = in.nextLine();



        while (!input.equals("1") && !input.equals("2")) {

            System.out.println("tInvalid Command!");

            input = in.nextLine();



        }



        if (input.equals("1")) {

            System.out.println("t> You march on, deeper into the Dungeon!");



        } else if (input.equals("2")) {

            System.out.println(

                    "tYou exit the dungeon. Battered and bruised as you may be, you live to fight another day!");

            break;

        }

    }



}

Now my second and last question is with my Player class and more specifically the way my levelUp(), levelUpXp() and ding() Methods are written. There has to be a better way right? I have read Javadocs, watched tutorials, read physical books and I have even done courses on sites like Codecademy and just can't seem to wrap my head around this.

My initial goal was to have the Array automatically switch from currentLevel[0](Level 1) to currentLevel[1](Level 2) once the amount of reqXP of requiredXP[1] was met. I know a .next() exists for ArrayLists similar to rand.nextInt() but I don't know if that is exclusive to ArrayLists or if they can be used with Arrays as well. The current code you see now is a result of this lack of knowledge so any help or advice is much appreciated.

I am still new to programming and this is also only my second post in 2 years.

I'm not sure if it is ideal to keep the logic on how much XP is needed for a level up inside the player class itself. Usually these are properties kept outside and feed into the logic. While there are certain designs that consider the player to be a data class with some external manager performing modifications like the levleUp and other routines on the player object, this somehow violates OOP principles where state should be encapsulated in the class and method invocations used to alter the state internally.

By that, the first thing which comes to my mind on analyzing your code is, that you are actually performing a mapping between XP and the level. Your current solution creates such a mapping per player. If you have a system with hundreds of players this mapping would probably create a lot of duplicate data unless you want a per-player solution, which also would not work with the current approach. This therefore is a strong candidate for refactoring.

You might use an implementation of Map that you can leverage to see how many XP are required for a player to gain a level-up. As mention before, ideally you sepcify such a mapping outside of the code and then only read in the configuration on application start. This allows you to tweak your application later on without having to recompile the whole code. As the levels are furthermore progressing you only need to store the XP required per level.

You could i.e. have an xpPerLevel.txt file with the following content:

6

17

36

...

which you could read in with a method like below:

public Map<Integer, Integer> loadXpPerLevel() {

    Map<Integer, Integer> xpPerLevel = new LinkedHashMap<>();

    int level = 1;

    try (Stream<String> lines = Files.lines(Paths.get("xpPerLevel.txt"))) {

      lines.forEach(line -> xpPerLevel.put(level++, Integer.valueOf(line));

    }

    return xpPerLevel;

}

Ideally this code sits outside of the Player class and only the xpPerLevel map is injected to the player through its constructor. This allows certain different XP-level settings for different kinds of players, i.e. premium players get a different map injected than normal players. Via a strategy pattern this can be customized quite easily.

Next in your levelUp method you perform a check for the currentXP against the requiredXP. In order for a level up to kick in the player has tho have exactly the same amount of XP as required. If s/he has more though, no level up would occur. With the changes proposed from above the levelUp() method can be refactored as follows, which was renamed to checkCurrentXP to give it a bit more clarity:

private void checkCurrentXP() {

    Integer xpRequired = null;

    do {

        xpRequired = xpPerLevel.get(curLevel);

        if (null != xpRequired) {

            if (curXP >= xpRequired) {

                performLevelUp();

            }

        }

    } while (xpRequired == null || curXP < xpRequired);

}

This method simply retrieves the XP required for a level up based on the user's current level and checks whether the user already acquired more XP than needed for a level up and in that case invokes the performLevelUp() method, which is just a renamed version of ding(). As the level up is XP driven it may occur that a user acquired more XP than actually needed for a single level up in which cace the proposed soultion automatically would perform a second, third, ... level up as well.

Note that I started the xpPerLevel.txt by the XP requirements needed to reach level 2 as the current logic would initially bump a player to level 2 automatically as 0 XP are required to reach that level. On applying these changes you basically only need to store the xpPerLevel map, the current level of the user as well as the gained XP on the user.

As before with the xpPerLevel map, ding() or performLevelUp(), as I renamed it to, are also good candidates for a strategy pattern that allows you to define different level up behaviors for different players which can be changed during runtime i.e. to see during test time which level-up logic is more suitable if multiple candidates may exist (i.e. one strategy might reset the XP of a user after a level up was performed while an other just builds up on the old amount. Yet another strategy might refill the player's HP back to maximum and so forth). The method itself has also a lot of duplicate code, which you don't really need as you don't do anything differently from level to leve. It is therefore a strong candidate for refactoring as well:

private void performLevelUp() {

    System.out.println(" #############################");

    System.out.println(" # You have reached level " + (++curLevel) + "! # ");

    System.out.println(" #############################");

}

As the level of a player is dependent on the XP earned I'd remove the setLevel(int level) method completly. Instead I'd provide a method that allows a player to gain XP:

public void awardXP(int xpAmount) {

    curXP += xpAmount;

    checkCurrentXP();

}

Right after you awarded the player with a number of XP it will automatically check the whether it lead to a level up or not and thus update the user's level accordingly.

As hopefully can be seen, certain state, such as the player's level and curXP, is encapsulated in the player's object and through invoking methods on that player's object that state gets manipulated. This is in essence what object-oriented programming should be.

Whithin your game loop you only need to award the player a certain amount of XP which will trigger internal state changes automatically. So there is no need to invoke p.levelUp() and p.levelUpXp() within your game loop.

As your main-loop is basically a typical console application reading in some user input and performing some task on the input applying a command pattern here does make the code a bit more readable. You basically refactor out your if/if-else segments into own little classes that are only focusing on that particular task. Your loop is basically responsible for too many things, which violates a bit the single responsibility principle which is further a part of SOLID.

Basically what you can do to improve your code here is, i.e. introduce a new EventLoop class where you can register a couple of commands with it. The commands are just implementations of either an interface or abstract base class. The EventLoop class will store the commands within a map for a certain input command. Upon user-input the input will be parsed and the respective command invoked, if available. In Java this may look something like this:

public class EventLoop {



    private final Map<String, ConsoleCommand> registeredCommands;



    EventLoop() {

        reisteredCommands = new ConcurrentSkipListMap<>();

    }



    public void registerCommand(String command, ConsoleCommand action) {

        if (action == null) {

            throws new IllegalArgumentException("No defined action for command " + command);

        }

        registeredCommands.put(command, action);

    }



    public vod unregisterCommand(String command) {

        registeredCommands.remove(command);

    }



    public void update(GameState state) {

        Scanner scanner = new Scanner(System.in);



        while(GameState.FINISHED != state.currentState()) {

          printHelp(state);



          String input = scanner.nextLine();

          // String input = validate(input);

          ConsoleCommand action = registeredCommands.get(input);

          if (null != action) {

              action.perform(state);

          } else {

              System.err.println("t Invalid Command!");

          }

        }

        scanner.close();

    }



    private void printHelp(GameState state) {

        System.out.println(

                getPlayerInfo(state.getPlayer()) 

                + "ntnt" 

                + getEnemyInfo(state.getEnemy()));

        System.out.println("nt What would you like to do?");

        for(String cmd : registeredCommands.keySet()) {

            ConsoleCommand action = registeredCommands.get(cmd);

            System.out.println("t " + cmd + ". " + action.printHelp());

        } 

    }



    private String getPlayerInfo(Player player) {

        return "t" + player.getName() + "nt HP: " player.getHealth() + " Level: " + player.getCurrentLevel() + " Exp: " + player.getCurrentXP();

    }



    private String getEnemyInfo(Creature enemy) {

        return "t" + enemy.getName() + "nt HP: " +enemy.getHealth() + " Level: " + enemy.getCurrentLevel();

    }

}

A ConsoleCommand defines now the concrete action to perform once invoked. As can be seen from the EventLoop two methods are at least necessary:

public interface ConsoleCommand {

    void perform(GameState state);

    String printHelp();

}

If you later on decide that you want to pass certain properties to a command, you should change the interface to an abstract class and implement a method like parseCommand(String input) in it so that inheriting classes automatically have access to the already parsed values.

A concrete implementation of the AttackCommand may now look like this

public class AttackCommand implements ConsoleCommand {



    private Random rand = new SecureRandom();



    @Override

    public String printHelp() {

        return "Attack"

    }



    @Override

    public void perform(GameState state) {



        Player player = state.getPlayer();

        Creature enemy = state.getEnemy();

        int damageDealt = rand.nextInt(player.getAttackDamage());

        int damageTaken = rand.nextInt(enemy.getAttackDamage());



        player.takeDamage(damageTaken);

        enemy.takeDamage(damageDealt);



        if (damageDealt > 0) {

            System.out.println("t> You strike the " 

                + enemy.getName() + " for " + damageDealt + " damage!");

         } else if (damageDealt < 1) {

            System.out.println("t> You attempt to hit the " 

                + enemy.getName() + " but miss!");

         }

         if (damageTaken > 0) {

             System.out.println("t> The " + enemy.getName() 

                + " retaliates! You take " + damageTaken + " damage!");

         } else if (damageTaken < 1) {

             System.out.println("t> The " + enemy.getName() 

                + " retaliates but misses!");

         }



         if (player.getHealth() < 1) {

             System.out.println("t ##################" 

                 + "nt # You Have Died! #" 

                 + "nt ##################");

             state.updateGameState(GameState.FINISHED);

         }



         if (enemy.getHealth() < 1) {

             System.out.println("t Enemy " + enemy.getName() 

                 + " was crushed by your mighty strikes. You have been awarded with "

                 + enemy.getAmountOfXPWorthForKilling() + " XP";

             player.awardXP(enemy.getAmountOfXPWorthForKilling());

         }

    }

}

I hope that by the presented examples it does make sense to you, that by separating the actual actions into commands the general code becomes less cluttered and therefore easily readable and understandable. You further can test an action more easily as well.

As you might have noticed that I also changed a couple of things throughout this example. I like to have names of methods that actually do the same things on the object to be the same. So player.takeDamage(int) does basically the same as enemy.takeDamage(int). Refactoring this behavior to a parent class would make sense here as everything has to die once it has no HP left. The difference here is though that if a player dies the game is over compared to when a creature dies the player is awarded with XP.

Further, it is IMO a good design to refactor the overall GameState out of the actual main-loop so that it can get passed around more easily. This allows the action commands to update the game state if needed without having to perform a callback into the main-loop.

Welcome on Code Review.

Your game's loop is way too long. You have to separate your code into functions that constitute logical units.

E.g., taking and dealing of damages have to be moved into two separate function dealDamage and takeDamage. The whole loop of the battle into a function doBattle. Spawn of enemy have to be wrap into a spawnEnemy, inside which you place a call to doBattle. Etc.

I'm afraid to tell you that all your logic seem to be weird.

Instead of increasing the level and Xp in a clumsy way, just remove ding, levelUp and levelUpXp, replace:

p.curXP += 3;

p.levelUp();

p.levelUpXp();

by

p.addXp(3);

and add this function :

public void addXp(int reward) {

    curXP += reward;

    while (level < requiredXP.length && requiredXP[level] < curXP) {

        ++level;

        System.out.println(" #############################");

        System.out.println(" # You have reached level " + level + "! # ");

        System.out.println(" #############################");

    }

}

By doing that, you can get rig or reqXP and currentLevel too.

Your max level become the size of requiredXP (which you could rename experienceGap).

You can populate this array using the logic from Level() (adding the values to the array instead of printing them)

If you have some trouble to understand how game mechanics works, dive into the GameDev site.

This could be shortened by looping through requiredXP until you reach a value that curXP is equal to or using a switch statement. The same could be done in levelUpXp().

You are right that using and ArrayList<Integer> could simplify things as well. Possibly look into a list or queue data type.

You could also call ding() from inside levelUp() or levelUpXp() instead of the while loop or set a flag variable (eg. newLevel) that it tests for to skip the test conditions inside.