12
\$\begingroup\$

I started making a text-based RPG game and before I spend a lot of time on it, I wanted to know if there is a better way to make it. Currently I'm using switches to flow through the game, I also know some people use a lot of if statements, but is there a more efficient way?

Main

public class Main {

    public static GameSave GS = new GameSave();
    public static Scanner in = new Scanner(System.in);
    public static String question;

    public static String name;
    public static String user = "";

    public static void main(String[] args) {

        question = GS.LoadGame();
        name = GS.LoadName();
        //Game Loop
        while (true) {

            if (user.equalsIgnoreCase("exit")){
                System.out.println("Closing game.");
                System.exit(0);
            }else{
                switch (question) {
                    case "0":
                        System.out.println("Story...\n\nWhat is your name?");
                        question = "1";
                        break;

                    case "1":
                        user = in.next();
                        name = user;
                        question = "1A";
                        GS.SaveName(name);
                        GS.SaveGame("1A");
                        break;

                    case "1A":
                        System.out.println("Are you sure that is your name?\n[1]Yes\n[2]No");
                        question = "2";
                        break;

                    case "2":
                        switch (user = in.next()) {
                            case "1":
                                question = "2A";
                                GS.SaveGame(question);
                                break;

                            case "2":
                                System.out.println("\n\n");
                                question = "0";
                                break;
                            default:
                                System.out.println("\nInput not accepted, try again.");
                                question = "1A";
                                break;
                        }
                        break;

                    case "2A":
                        System.out.println("Hello, " + name + "\n[1]Hello...\n[2]Who the hell are you?");
                        question = "3";
                        break;
                    case "3":                      
                }
            }


        }

    }
}

GameSave

public class GameSave {

    public static Properties prop = new Properties();

    public void SaveGame(String point){
        try{
            prop.setProperty("Save_Point", point);
            prop.store(new FileOutputStream("config.prop"), null);
        }catch (IOException e){

        }
    }

    public void SaveName(String playerName){
        try{
            prop.setProperty("Save_Name", playerName);
            prop.store(new FileOutputStream("config.prop"), null);
        }catch (IOException e){

        }
    }

    public String LoadGame(){
        String line = "";

        try{
            prop.load(new FileInputStream("config.prop"));
            line = prop.getProperty("Save_Point");
        }catch (IOException e){
            try{
                prop.setProperty("Save_Point", "0");
                prop.store(new FileOutputStream("config.prop"), null);

                try{
                    prop.load(new FileInputStream("config.prop"));
                    line = prop.getProperty("Save_Point");
                }catch (IOException exe){

                }
            }catch (IOException ex){

            }
        }
        return line;
    }

    public String LoadName(){
        String name = "";
        try{
            prop.load(new FileInputStream("config.prop"));
            name = prop.getProperty("Save_Name");
        }catch (IOException e){
            try{
                prop.setProperty("Save_Name", "");
                prop.store(new FileOutputStream("config.prop"), null);

                try{
                    prop.load(new FileInputStream("config.prop"));
                    name = prop.getProperty("Save_Name");
                }catch (IOException exe){

                }
            }catch (IOException ex){

            }
        }
        return name;
    }

}
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! This game appears to be an interesting endeavor. I hope you get some good reviews! \$\endgroup\$ – Phrancis Nov 18 '14 at 22:59
10
\$\begingroup\$

Firstly, as a pointer, you can save yourself quite a lot of typing if you take the 2 load methods and extract what they have in common. Extracting methods is something you should always look for. Rule of thumb: if you type the same structure twice, you probably want to extract the common part into a method.

Here is an example of your method. There are a number of things you should improve in it but I'll list that later on. The reason extracting is important is that say you want to change how you handle exceptions, in your current setup you need to do it multiple times. You might also introduce small changes between the 2 methods (say in one case you catch an exception in the other you don't).

If you extract it to a method you update it once and it gets applied in the same way for all situations (which is usually what you want):

public String loadString(String propertyFileName, String key, String default) {
    String result = "";
    try{
        prop.load(new FileInputStream(propertyFileName));
        result = prop.getProperty(key);
    } catch (IOException e) {
        e.printStackTrace();
        // throw an exception or return default value
        // copying the code you had here before, though I would recommend not
        // doing main logic inside an exception. in particular as this exception
        // can be a number of things, not just filenotfound.
        prop.setProperty("Save_Point", "0");
        prop.store(new FileOutputStream(propertyFileName), null);

        try{
           prop.load(new FileInputStream(propertyFileName));
           result = prop.getProperty(key);
        }catch (IOException exe){
            exe.printStackTrace();
        }
    }
    return result;
}

Then you can use the loadGame and loadName like:

public String loadGame() {
    return loadString("config.prop", "Save_Point", "0");
}

public String loadName() {
    return loadString("config.prop", "Save_Name", "");
}

Furthermore, I would suggest you try to structure your questions / options / text in objects. If you expand it using the switch, you will get a monster of a switch statement which will be really hard to maintain.

You could do something like (assume there are getters/setters for these fields):

public class Room {
    protected int id;
    protected String description;

    protected List<Exit> exits;    
}

public class Exit {
    protected int leadsTo; // the room id
    protected String name;
}

These you could load from a file on disk (JSON format, property format or XML). You could check out jackson deserializer, though there are a number of options here.

Then in your loop you could do something like:

Map<int, Room> rooms = loadRoomsFromDisk(); // map of roomId -> Room
Room current = rooms.get(0); 
while (true) {
    // print out room description
    System.out.println(current.getDescription());
    // print out exits
    for (Exit exit : current.getExits()) {
        System.out.println(exit.getName());
    }

    // scan for line
    // if line corresponds to exit, go to exit. set current to new room.
}

This is oversimplified but to give you the idea. Now it would just be a matter of adding new room files and your gamearea would expand. You wouldn't need to update your main loop.

Keep in mind, this approach means that if you change your classes, the files on disk have to change. Initially it might be faster to create 1 levelloader class which just constructs a bunch of objects for you (a test level). Once that is working and stable (say you haven't changed the structure in a week or more) then you could consider writing it to disk/reading it from disk.

Now some general code review pointers:

  1. In Java, start a method with lowerCaseFirst and start classes with UpperCaseFirst.
  2. Never catch an exception without logging, unless you are absolutely sure that it is safe.
  3. Try to avoid putting a lot of default logic in the exception part. In this specific case you have an IOException that could be a number of things. If there is a situation you want to catch (say, file doesn't exist on disk) then test for it in the try, and act on it, rather than trying out an operation and responding to the exception.

    File f = new File(propertyFile);
    if (!f.exists()) {
        // create the file
    }
    

    Rather than:

    File f = new File(propertyFile);
    try {
        prop.load(new FileInputStream("config.prop"));
    } catch (FileNotFoundException e) {
        // create the file
    }
    

So far it looks good. I would sketch out on paper what the general flow is of your game. Designing levels can be tricky and it's easier to find out on paper what you want to build than to build stuff in code and after a lot of typing finding out you missed something. You probably will end up with some standard things like an inventory system, a player object, mobs, etc. Take it one step at a time and don't let yourself get overwhelmed by the things you want to put in.

\$\endgroup\$
  • \$\begingroup\$ Thank you so much! I really appreciate the time you took to write this out. By the way, some of your code isn't formatted(the code above "Now some general code review pointers"). But once again thank you so much! \$\endgroup\$ – Arman Nov 19 '14 at 0:19
  • \$\begingroup\$ ooops :) I'll fix that \$\endgroup\$ – Joeblade Nov 19 '14 at 0:21
  • \$\begingroup\$ @Joeblade Wouldn't it be more sensible to have a map of exit name and adjacent rooms instead of an additional class "Exit"? And assuming that there is a reason for the "Exit" class to exist, why reference a room id instead of the room itself? \$\endgroup\$ – SBoss Nov 19 '14 at 7:23
  • \$\begingroup\$ Depends if there are more attributes you want to add to exit. If it is only name/destination then a map is fine. But an exit could be locked, or support only a certain weight or have a number of other attributes. If you reference roomId rather than Room in Exit, then you don't get the references going back/forward which is something I personally like. A and B knowing of eachother is something I try to avoid, constructing objects is a bit of a pain then/loading from file etc. But this is just my approach, there's more than one way leading to rome \$\endgroup\$ – Joeblade Nov 19 '14 at 8:17
  • \$\begingroup\$ Great explanation, take my upvote :) \$\endgroup\$ – SBoss Nov 19 '14 at 8:22
3
\$\begingroup\$

I think you're on the right track. Most games have a main loop for processing input events (e.g. user input, hardware controller events), updating game state, and generating output.

According to https://docs.oracle.com/javase/7/docs/technotes/guides/language/strings-switch.html

The Java compiler generates generally more efficient bytecode from switch statements that use String objects than from chained if-then-else statements.

\$\endgroup\$
  • \$\begingroup\$ while true, a case statement usually ends up hard to read and hard to maintain. (missing break and no blocks by default means variables are all in the same scope) \$\endgroup\$ – Joeblade Nov 19 '14 at 0:14
  • 2
    \$\begingroup\$ Agreed. He could also take a Component-Entity-System approach for ultimate flexibility and maintainability. Chris Granger has a nice intro to the CES paradigm: chris-granger.com/2012/12/11/anatomy-of-a-knockout \$\endgroup\$ – erturne Nov 19 '14 at 0:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.