Beginner object-oriented text-based RPG

Question

I'm in the process of writing a text-based RPG in Java using object-oriented programming. I'm fairly new to programming and currently learning through CodeHS and coding this within their sandbox, any help would be greatly appreciated.

My biggest struggle is with the relationship between the Player and Enemy class and how those relate to the Creature class. Methods such as roleToNumber() also seem to not be the best way to do things, but I'm not sure as to how I could implement a similar system in an easier way.

This is the class that handles inputs and outputs as well as creating the encounters and battle phases:

import java.util.Scanner; public class MyProgram extends ConsoleProgram { public void run() { Scanner readName = new Scanner(System.in); System.out.print("Enter your name: "); String userName = readName.nextLine(); Scanner readRole = new Scanner(System.in); System.out.print("Choose your role (Fighter, Ranger, Arcanist): "); String userRole = readRole.nextLine(); while(true){ if(userRole.equalsIgnoreCase("Fighter") || userRole.equalsIgnoreCase("Ranger") || userRole.equalsIgnoreCase("Arcanist")){ break; }else{ System.out.println("Choose a valid role"); readRole = new Scanner(System.in); System.out.print("Choose your role (Fighter, Ranger, Arcanist): "); userRole = readRole.nextLine(); } } //a demo of all of the systems System.out.println(""); Player player = new Player(userName, userRole); scene(player, "a mansion"); if(!player.isDead()){ scene(player, "a rock"); } } public String attack(Creature one, Creature two){ int a = one.attack(two); return one.getName() + " hit " + two.getName() + " for " + a + " damage."; } public void battle(Player one, Creature two){ System.out.println(one); System.out.println(two); while(true){ Scanner readChoice = new Scanner(System.in); System.out.print("\nWhat do you want to do (Attack, Run, Status, Use Potion): "); String userChoice = readChoice.nextLine(); while(true){ if(!userChoice.equalsIgnoreCase("Status") && !userChoice.equalsIgnoreCase("Run") && !userChoice.equalsIgnoreCase("Attack") && !userChoice.equalsIgnoreCase("Use Potion")){ System.out.println("Choose a valid choice"); readChoice = new Scanner(System.in); System.out.print("\nWhat do you want to do (Attack, Run, Status, Use Potion): "); userChoice = readChoice.nextLine(); }else{ break; } } if(userChoice.equalsIgnoreCase("Status")){ System.out.println(one.status()); continue; } if(userChoice.equalsIgnoreCase("Use Potion")){ System.out.println(one.useHealthPotion()); System.out.println(one.status()); continue; } if(userChoice.equalsIgnoreCase("Run")){ int run = (int)(Math.random() * 100 + 1); if(run >= 50){ System.out.println("You successfully run."); break; }else{ System.out.println("You fail at running."); } }else if(userChoice.equalsIgnoreCase("Attack")){ System.out.println(attack(one, two)); System.out.println(two.status()); } if(!two.isDead()){ System.out.println(attack(two, one)); System.out.println(one.status()); if(one.isDead()){ System.out.println("You died!"); break; } }else{ System.out.println("You killed " + two.getName() + "\n"); System.out.println("You gained " + one.gainXp(two) + " exp"); if(one.checkXp()){ System.out.println("You leveled up, your health is restored!"); System.out.println("You have " + one.getXp() + " exp"); }else{ System.out.println("You have " + one.getXp() + " exp"); } System.out.println(one + "\n"); break; } } } public void scene(Player one, String description){ System.out.println(one.getName() + " arrives at " + description); int x = (int)(Math.random() * 3 + 1); for(int i = 0; i < x; i++){ if(one.isDead()){ break; } Enemy randEnemy = new Enemy(one.getLevel()); System.out.println("\nYou encounter " + randEnemy.getName() + " the " + randEnemy.getRole()); battle(one, randEnemy); } } } 

This the Creature class which has basic functions shared between Players and Enemies:

public class Creature{ public String name; public String role; public int maxHp; public int maxAtt; public int minAtt; public int level; public int curHp; public Creature(String name, String role){ this.name = name; this.role = role; } public int attack(Creature other){ int att = (int)(Math.random() * (this.maxAtt - this.minAtt + 1) + this.minAtt); other.curHp -= att; return att; } public boolean isDead(){ if(this.curHp <= 0){ return true; }else{ return false; } } public int getCurHp(){ return curHp; } public void setCurHp(int hp){ if(hp >= maxHp - curHp){ curHp = maxHp; }else{ curHp = hp; } } public String getName(){ return name; } public void setName(String name){ this.name = name; } public String getRole(){ return role; } public void setRole(String role){ this.role = role; } public int getMaxHp(){ return maxHp; } public int getLevel(){ return level; } public String status(){ return name + " has " + curHp + "/" + maxHp + " health."; } public String toString(){ return name + " the " + role + " is level " + level + " with " + curHp + "/" + maxHp + " HP and an attack of " + maxAtt + "-" + minAtt; } } 

This is the Player class:

public class Player extends Creature{ public int xp; private int hpPotions = 3; public Player(String name, String role){ super(name, role); this.level = 1; rollStats(); this.curHp = maxHp; } public String useHealthPotion(){ if(hpPotions >= 1 ){ this.setCurHp(this.getCurHp() + 25); hpPotions--; return hpPotions + " potions left."; }else{ return "No potions to use."; } } public int getHealthPotion(){ return hpPotions; } public void setHealthPotions(int newHpPotions){ hpPotions = newHpPotions; } public int gainXp(Creature other){ int x = other.getLevel(); int gainedXp = x * (int)(Math.random() * (60 - 21) + 20); xp += gainedXp; return gainedXp; } public boolean checkXp(){ if(xp >= level * 40){ xp = xp - (level * 40); levelUp(); return true; }else{ return false; } } public String status(){ return name + " has " + curHp + "/" + maxHp + " health."; } public String getXp(){ return xp + "/" + (level * 40); } //rolling for intitial stats public void rollStats(){ int hp = 0; int att = 0; switch(roleToNumber()){ case 1: hp = 16; att = 10; break; case 2: hp = 13; att = 13; break; case 3: hp = 12; att = 14; break; } maxHp = (roll(6) + hp); maxAtt = (roll(6) + att); minAtt = (maxAtt - 3); } private int roll(int sides){ int aRoll = (int)(Math.random() * sides + 1); return aRoll; } //Changes the inputed role to a number private int roleToNumber(){ if(role.equalsIgnoreCase("Fighter")){ return 1; }else if(role.equalsIgnoreCase("Ranger")){ return 2; }else if(role.equalsIgnoreCase("Arcanist")){ return 3; }else{ return 0; } } //coding for level up with modifiers based on role public void levelUp(){ level++; int hp = 0; int att = 0; switch(roleToNumber()){ case 1: hp = 24; att = 14; break; case 2: hp = 19; att = 19; break; case 3: hp = 16; att = 22; break; } maxHp += (hp * Math.random()/2 + .7); maxAtt += (att * Math.random()/2 + .7); minAtt = maxAtt - 3; this.curHp = maxHp; } } 

And this is the Enemy class:

public class Enemy extends Creature{ public Enemy(int leveled){ super("Filler", "Filler"); this.level = 1; this.setName(randomName()); this.setRole(randomRole()); rollStats(); if(leveled > 1){ for(int i = 1; i < leveled; i++){ levelUp(); } } this.curHp = maxHp; } //pulls a random name from an array public String randomName(){ String[] names = {"Spooky", "Scary", "Yup"}; int index = (int)(Math.random() * names.length); return names[index]; } //pulls a random role from an array, these are pased to roleToNumber public String randomRole(){ String[] roles = {"Orc", "Goblin", "Dragon"}; int index = (int)(Math.random() * roles.length); return roles[index]; } public void rollStats(){ int hp = 0; int att = 0; switch(roleToNumber()){ case 1: hp = 16; att = 10; break; case 2: hp = 13; att = 13; break; case 3: hp = 12; att = 14; break; } maxHp = (roll(6) + hp); maxAtt = (roll(6) + att); minAtt = (maxAtt - 3); } private int roll(int sides){ int aRoll = (int)(Math.random() * sides + 1); return aRoll; } private int roleToNumber(){ if(role.equalsIgnoreCase("Orc")){ return 1; }else if(role.equalsIgnoreCase("Goblin")){ return 2; }else if(role.equalsIgnoreCase("Dragon")){ return 3; }else{ return 0; } } public void levelUp(){ level++; int hp = 0; int att = 0; switch(roleToNumber()){ case 1: hp = 24; att = 14; break; case 2: hp = 19; att = 19; break; case 3: hp = 16; att = 22; break; } maxHp += (hp * Math.random()/2 + .5); maxAtt += (att * Math.random()/2 + .5); minAtt = maxAtt - 3; this.curHp = maxHp; } } 

Answer 1

First, let me say, "Good job." This code is okay for someone in the middle of learning.

With that out of the way, now I'll rip it to shreds. ;->

Let's ignore your main program, which has its own set of problems, in favor of focusing on the class hierarchy as you requested.

Use the basic features of OOP

The first thing I notice, is that you're not really "doing" the classes right. For example, consider the Creature:

public class Creature{ public String name; public String role; public int maxHp; public int maxAtt; public int minAtt; public int level; public int curHp; public Creature(String name, String role){ this.name = name; this.role = role; } 

One thing you should have learned about classes/objects: when you construct an instance of a class, it should be ready to go! There are few exceptions to this, and all of them are awkward.

In your Creature you are breaking this rule. Your constructor sets name and role and leaves all the other instance data unset.

There are several ways around this. The most basic of which is to pass in all the instance data as parameters, or compute it based on parameters. For instance, currHp = maxHp would be a computation, while passing maxHp as a constructor argument would be passing it in.

Alternatively, you could depend on some (sub)class specific methods to return values you need: maxHp = getMaxHp()

Don't use case, use subclasses

If you're writing OO code, and you find yourself using a case statement, there's a good chance you need a subclass of some kind. Not always - your method might switch on some kind of external data - but if you're switching on internal data to change your behavior, you could probably use a subclass to get that result.

In your case, you're switching on roleToNumber(), which is doubly expensive since you are re-running a pure function each time with no caching of the result.

Instead of doing that, create subclasses. Push the majority of the "real" code into the Enemy and Player classes, and use class Orc extends Enemy to provide the string description, numerical stats, and any special attack text that is needed for the game:

class Orc extends Enemy { public Orc() { super(randomName(), "Orc"); } public getMaxHp() { return roll(6) + 16; } public getMaxAttack() { return roll(6) + 10; } } 

You can likewise do the same thing for the player classes, except the name isn't random:

class Fighter extends Player { public Fighter(String name) { super(name, "Fighter"); } ... stats ... } 

That should enable you to eliminate roleToNumber and everything that uses it - just encode the switch as a method that returns the right answer directly.

Write methods that actually do what you're doing

I notice that you've fallen into the trap of writing methods that don't do what your code is trying to do:

public String useHealthPotion(){ if(hpPotions >= 1 ){ this.setCurHp(this.getCurHp() + 25); hpPotions--; return hpPotions + " potions left."; }else{ return "No potions to use."; } } 

Consider setCurHp(). What does it do? It sets the current Hp. That's nice, but what is your code actually doing? Your code is raising the current HP, subject to a maximum.

Why don't you write a method that raises the current HP, subject to a maximum of maxHp? It might be called public raiseCurrHp(int howmuch);

And then you could write:

public String useHealthPotion(){ if(hpPotions >= 1 ){ this.raiseCurrHp(25); hpPotions--; return hpPotions + " potions left."; }else{ return "No potions to use."; } } 

It doesn't seem like much, but over time the useless reduction of every operation to basic getters and setters can drag down your code, making it hard to read and/or modify. Don't be afraid to write the methods that you actually need, rather than some basic set you learned in class.

(On the other hand, gainXp is a good method that does this. So, keep doing that!)

Answer 2

You wrote a good program, and its code reads like a good story. It's a bit lengthy in some places, but that can be worked around. You introduced the game concepts (players, enemies) in an easy to understand way.

---

I agree with Austin's answer in most cases. But I have a different opinion on the number of classes actually needed.

Currently, all players behave the same. They just differ in the amount of health points and some other small details. One way to model this is indeed to create subclasses. I prefer to only create subclasses if the behavior differs not if the data or parameters differ.

A simpler way is to define an enum for the roles. It looks like this:

public enum PlayerRole { Fighter, Ranger, Arcanist } 

One nice thing about enums is that you can use them directly in switch statements. This makes the roleToNumber method unnecessary.

switch (player.getRole()) { case Fighter: return 17; case Ranger: return 23; case Arcanist: return 20; } throw new IllegalStateException(); 

(Most Java programmers write enum constants in all-uppercase, like all other constants. I prefer the C# way of using mixed case.)

---

Your use of Math.random is more complicated than necessary. You should consider creating your own class for this, to focus on the randomness you really need. I'm thinking of something like this:

public class GameRandom { private final Random rnd; public GameRandom(Random rnd) { this.rnd = rnd; } public int d6() { return between(1, 6); } public int between(int minInclusive, int maxInclusive) { return rnd.nextInt(1 + maxInclusive - minInclusive) + minInclusive; } } 

One nice thing about this class is that you can use a fixed "random" number generator, which you create with new GameRandom(new Random(0)). This is useful for testing your code. Because during testing, randomness is hard to test. It's much easier when your test cases always behave the same. The Random class has been producing the exact same output for years, and probably this is even guaranteed.

---

You have some duplicate code for the user input. The typical loop of invalid input, try again should be extracted to a separate method. It might look like this:

private final Scanner input = new Scanner(System.in); private final PrintStream output = System.out; public <T> T choose(String prompt, T... choices) { String choiceNames = Stream.of(choices).map(Object::toString).collect(Collectors.joining(", ")); String actualPrompt = String.format("%s (%s): ", prompt, choiceNames); while (true) { output.print(actualPrompt); String answer = input.nextLine(); for (T choice : choices) { if (answer.equalsIgnoreCase(choice.toString())) { return choice; } } output.println("Invalid choice."); } } 

You can use this method like this:

String role = choose("Choose your role", "Warrior", "Archer", "Alchimist"); switch (role) { case "Warrior": hp = 5; break; case "Archer": hp = 8; break; case "Alchimist": hp = 27; break; } 

Or even like this:

PlayerRole role = choose("Choose your role", PlayerRole.values()); 

No more chance for typos is this code, and you can just add a new role to PlayerRole and it works immediately. This convenience is one of the reasons to write the enum constants in Mixed Case instead of ALL_UPPERCASE.

---

Instead of System.out.println, you could use System.out.printf. The benefit is that you immediately see the general pattern of the output, and the actual data is formatted into this pattern. Example:

System.out.printf("%s hit %s with a %s, causing %d damage.%n", attacker, attackee, weapon, damage); 

The % placeholders may look difficult at first, but millions of people already learned them, and they are quite useful.

Answer 3

I'm in the process of writing a text-based RPG in Java using object-oriented programming. I'm fairly new to programming and currently learning through CodeHS and coding this within their sandbox, any help would be greatly appreciated.

This is a great early project for learning programming. If you are interested in the current state of the art in text based adventure programming, consider reading up on Inform7. It is an astonishing programming language and environment.

As I noted in a comment, I caution you to use OO extremely carefully in this domain. It is very easy to try to encode the rules of your game into the rules of a type system that has nothing to do with your game domain.

My biggest struggle is with the relationship between the Player and Enemy class and how those relate to the Creature class

Your instincts are spot on. Getting this right is crucial, and getting it wrong will create a mess.

A couple things to look for when critiquing class hierarchies are: (1) where am I duplicating code? and (2) what design avenues am I cutting off?

For example: both player and enemy extend creature; that's good. Both player and enemy have nearly identical level-up methods that have nothing to do with each other; why is that? At the same time, apparently only players can drink potions; why is that?

Consider taking a step back and thinking more abstractly about the domain of your game. Your game must:

• Keep track of the locations of game objects like swords and lanterns and bags... and players and monsters. This insight should tell you that the class hierarchy is not yet deep enough; players have something in common with swords; they can be inside something.
• Keep track of the contents of containers, like boxes and bags... and rooms! What's the compelling difference between a room and a box? Nothing, really. They contain things. So boxes have something in common with rooms.
• Resolve attempted actions by players: if the player attempts to put the sword in the bag, either it succeeds or it fails; either way, something happens. If the player attempts to attack the vampire, something happens. Actions are a concern of the program, so maybe there should be a class for actions.
• Resolve attempted actions by enemies -- and not all NPCs are "enemies", so maybe "enemy" is the wrong name. Players and enemies have something in common: they both take actions that are resolved in the context of the game state and the game rules.

My point is that right now you are concentrating on encoding too much specificity into your system. Think hard about what the real "is a kind of" and "can do" relationships are in your game domain, and design your inheritance and interface hierarchies appropriately.

Answer 4

Flexibility

Class Enemy: I would call it NPC (non-player character). You are not committing all non-player creatures to be enemies, do you? Yes, some will attack you, or you will need to attack them. Some you will merely need to exploit, trick... But maybe some will actually offer help, like give items or information. Or they may accompany you on a quest. Then yet others will give you a task / ask you for help. With some you will trade...

How flexible to you want your RGB to be? Single player or multiplayer (MUD)? You have the creature definitions hard-coded into your program which will be the RPG engine. So in order to change/expand, you'll have to edit the actual engine. Instead I would suggest your RPG engine implements a virtual machine. That way editing the game world normally won't require you to edit the engine; you may even be able to do edits to the running game.

If you are making a MUD, such engines exist and are freely downloadable. But you may not do things in Java then. LP-MUD engine, for example, is written in C and allows you to create very elaborate worlds in form of a virtual machine, defined in an object oriented language called LPC; admin players who log in just like normal players can edit/reprogram the world without interrupting it. Normally one admin player is a god, with maximum editing access and other admin players are ex-normal players that were promoted to wizards and have more or less limited editing access.