# Beginner object-oriented text-based RPG

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()
{

System.out.print("Choose your role (Fighter, Ranger, Arcanist): ");
while(true){
if(userRole.equalsIgnoreCase("Fighter") || userRole.equalsIgnoreCase("Ranger") || userRole.equalsIgnoreCase("Arcanist")){
break;
}else{
System.out.println("Choose a valid role");
System.out.print("Choose your role (Fighter, Ranger, Arcanist): ");
}
}
//a demo of all of the systems
System.out.println("");
Player player = new Player(userName, userRole);

scene(player, "a mansion");
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){
System.out.print("\nWhat do you want to do (Attack, Run, Status, Use Potion): ");
while(true){
if(!userChoice.equalsIgnoreCase("Status") && !userChoice.equalsIgnoreCase("Run") && !userChoice.equalsIgnoreCase("Attack") && !userChoice.equalsIgnoreCase("Use Potion")){
System.out.println("Choose a valid choice");
System.out.print("\nWhat do you want to do (Attack, Run, Status, Use Potion): ");
}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());

}
System.out.println(attack(two, one));
System.out.println(one.status());

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++){
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;
}

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;
}
}

• I wrote a series of blog articles about pitfalls in OO design in this space, and I suspect you are falling into them. ericlippert.com/2015/04/27/wizards-and-warriors-part-one -- think very hard about whether the Java type system is actually the right place to be capturing the rules of your game. – Eric Lippert Nov 15 '17 at 15:55
• Re: RoleToNumber You are making two mistakes. First, you're feeding the output of one set of case statements into another set of case statements (the RoleToNumber method is essentially a case statement). Why not eliminate the middle step? Second, you are using case statements to implement the functionality of a dictionary. – Acccumulation Nov 15 '17 at 22:48
• @EricLippert I would go further and question whether OO is really necessary at all. @WatCow what are you trying to optimise/solve by using OO? Keystrokes, code duplication (it's a fail on both, look how many times you are writing public x y for starters). Before you go too much further, I highly recommend checking out this great talk "Stop Writing Classes": youtu.be/5ZKvwuZSiyc You don't have to take it as gospel, but it's worth thinking about. The other question I would ask: Is Java the right tool? – Benjamin R Nov 15 '17 at 22:55
• @BenjaminR: Your points are well taken, but it's important to not put the cart before the horse here. If what you want to do is write awesome text adventures then run-don't-walk to Inform7.com. But the stated goal of the original poster is to learn OO programming in Java, and I think it is reasonable to propose that a decent text adventure engine could be written in OO style in Java. The key is to figure out how OO concepts work naturally with the domain without falling into the naive trap of "a sword is a kind of weapon so I need a class Weapon..." – Eric Lippert Nov 15 '17 at 22:58
• @BenjaminR Eric Lippert is correct that my purpose for this is to learn OO programming and the sort of mindset I should take when approaching it. I do appreciate the idea that maybe I don't have to use OO though, because I did tend to over complicate things that could have been done much easier with a simple function. Thank you! – WatCow Nov 15 '17 at 23:49

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!)

• Thank you! I really appreciate the in-depth reply. So in object-oriented programming it is alright to have a class that extends for this class enemy for every single type of enemy I wish to create? I wasn't sure if creating a new subclass was preferable to just having it be a special case in the main class. – WatCow Nov 15 '17 at 4:26
• I'd like to add that useHealthPotion() should probably not return a string. The intuitive understanding of doSomething() is that it does not return anything, or in some cases, and this might be one of them, you could return a boolean which tells you whether a potion has been used or not. Then you don't print out the result of useHealthPotion(), but instead print out the text depending on the boolean result of the method. Example using the ternary operator: System.out.println(useHealthPotion() ? hpPotions + " potions left." : "No potions to use."); – Raimund Krämer Nov 15 '17 at 9:22
• This is a great answer, but: "Alternatively, you could depend on some (sub)class specific methods to return values you need: maxHp = getMaxHp()" In general yes, but not in constructors. This leads to cyclic dependencies between the base class and its children. A constructor should not call any method in the first place and if it does, the called method should either be static, private or final. This is a major flaw in your otherwise great answer. – Timothy Truckle Nov 15 '17 at 9:42

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);

for (T choice : choices) {
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.

• Thank you for the in depth answer! I like your idea of a common invalid input method, I wasn't sure if using a String as the case for a switch was a correct way to approach things. And thank you for your idea on how to implement the roles, I was really not happy with how I had approached it with rileToNumber() – WatCow Nov 15 '17 at 12:35
• This is a very good answer! I particularly commend the use of enums (something I had considered putting in my answer, then discarded to make the point about subclasses) and the idea of passing in a dedicated randomizing object. This is a slightly more advanced pattern, and you will see it all over the place in OO development (Java, C++, C#, etc.). I'm a little leery of the template-based answer, simply because I don't have the impression that the OP has made it that far in the course. (I could be wrong.) Nice work. – Austin Hastings Nov 15 '17 at 23:32
• @AustinHastings You are correct that I have not yet made it that far in the course :p. However I am really grateful for all of the help you and others have posted here. My intended purpose for writing this code wasn't necessary to create a game but to learn how classes ought to be constructed and what sort of mindset I should take towards making them. – WatCow Nov 15 '17 at 23:45
• My first version of the input method didn't use templates. It had one problem though, in that it didn't even compile since I wrote it just off the top of my head. When I actually tried the code, I had to adjust it at several places. It got quite complex at the end, with several advanced features. But since I had explained in detail how to use that method, I felt it was ok. This method should probably be placed into the ConsoleProgram class from which the MyProgram` class currently extends, to make it available without further work. – Roland Illig Nov 16 '17 at 1:56

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.

• Thank you! I do need to look at classes from a more top down perspective, I'm just now getting into the construction of classes and the use of subclasses so understanding all of the relationships has been something I've been really looking forward to. Are there any sources you would recommend to learn about object oriented programming in java? I'll also look into your write up because I skimmed over it during class and it looks very interesting, I just need to sit down and be able to comprehend all of it. Thank you again! – WatCow Nov 15 '17 at 18:22

# 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.