0

Hello Everyone i am creating a game mode for Bukkit and am encountering a problem that at the moment seems to baffling me. Perhaps i am simply over looking something so i would very much appreciate it if i could have another pair of eyes look at this segment of code. Here is my situation i have a command that creates a Game. This is a class Type. Every time the /newgame command is used it adds it to a ArrayList. Now it is supposed to check to see if a game with its name already exists. It only works for the first game name. so if i make a game called "game1" and then try to make "game1" again it returns "Can not create game with the name of game1" but when i make another one for example if i add the game"game2" and then i make "game2" again it allows it to be created. It only appears to working on the first game made. If someone could help it would be of much help so thanks in advance.

Note: The Main.games.size() always goes up so the games are being created but only the first game can not be created more then once any game after that can have the same name for some reason.

Here is the code Snippet in my CommandExecuter

if (cmd.equalsIgnoreCase("newgame")){
        Player player = sender.getServer().getPlayer(sender.getName());
            if (Main.games.size() == 0){
                Main.games.add(new Game(args[0]));
                player.sendMessage(ChatColor.GREEN + "Game Created. To join Type " + ChatColor.ITALIC + "/join " + args[0]);
                return true;
            }else{
                //Loop and Check
                Game game;
                for (int i = 0; i < Main.games.size(); i++){
                    game = Main.games.get(i);
                    if (game.getName().equalsIgnoreCase(args[0]) == false){
                        Main.games.add(new Game(args[0]));
                        player.sendMessage(ChatColor.GREEN + "Game Created. To join Type " + ChatColor.ITALIC + "/join " + args[0]);
                        //debug
                        player.sendMessage(Main.games.size() + ""); // + "" id
                        return true;
                    }else{
                        //Tells that a game already exists with that name.
                        player.sendMessage(ChatColor.RED + "Can not create game with the name of " + args[0]);
                        return true;
                    }
                }   
            }
Devin Wall
  • 180
  • 1
  • 16

2 Answers2

0

Your code only checks the name of the first game in the list. If the first game matches, the code is correct in returning because it already knows the name is duplicate. However, if the name of the first game does not match, the code should not return. We only know that the new game is not a duplicate of the first game but we do not know if it is a duplicate of the other games in the list. The code should go on to check the rest of the games in the list. Only after none of the game names match should this code return.

Change your else clause to the following.

Game game;
for (int i = 0; i < Main.games.size(); i++){
    game = Main.games.get(i);
    if (game.getName().equalsIgnoreCase(args[0]) == false){
        // Do not return here. Instead, continue to check the rest of the list
        continue;
    }else{
        //Tells that a game already exists with that name.
        player.sendMessage(ChatColor.RED + "Can not create game with the name of " + args[0]);
        return true;
    }
}
// We have reached the end of the loop so we now know that none of the 
// games in the list match. Now we can return.
Main.games.add(new Game(args[0]));
player.sendMessage(ChatColor.GREEN + "Game Created. To join Type " + ChatColor.ITALIC + "/join " + args[0]);
//debug
player.sendMessage(Main.games.size() + "");
return true;
iforapsy
  • 302
  • 2
  • 5
  • Thank you so much iforapsy. I thought that it might only be checking the first one but was not sure. – Devin Wall Aug 21 '13 at 05:34
  • Note that you do not need to use continue. As long as you do not return when the game names are not equal, the loop will continue onto the next element of the list. I added the continue statement to show that nothing more needs to be done when the names are not equal. – iforapsy Aug 21 '13 at 05:37
  • iforapsy One more question if i have my /join command like so for (Game game: Main.games){ if (game.getName().equalsIgnoreCase(args[0])){ //would still only chek the first }else{ //Cant find game. sender.sendMessage(ChatColor.RED + "Cant find a game with the name " + args[0]); return true; } } – Devin Wall Aug 21 '13 at 05:52
0

Seems like you still learn Java, so I don't want to give you a full code solution, just an explanation.

When iterating over a loop to see if something is in an array, you cannot return a false result until you checked all the items.

You error here - you compare new name with first item. If it is not the same name - you create a new game.

What you should do - you should add a boolean variable isFound and set it to false. Iterate over all games and check if the name exists. If so - change isFound to true and break the loop. After the loop ends you should check isFound state.

BobTheBuilder
  • 18,858
  • 6
  • 40
  • 61