Code isn't working

Okay, so on my plugin I'm trying to make it loop through bowtypes that the player has permission for. So how would I make it skip the ones the player doesn't have permission for in the loop?

I tried this http://paste.md-5.net/sarecetaya.avrasm but it isn't working.
Please help,
Thanks!

Help?

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

PimpDuck
Please only bump once every 24 hours.

 

I just really want to finish this plugin. I've been stuck on this bug for the last 2 days

 

if/else statements are your friends. Use them.

Edit: Also by looking at your code, for loops might be a help as well.

 

I don't know how I'd do the loop

 

PimpDuck
I would change this to a switch statement, since you can switch on enumerators

Code:java

if(getNextBowType(player) == BowType.NORMAL){
    setCurrentBowType(BowType.NORMAL, player);
    player.sendMessage(plugin.prefix + ChatColor.GOLD + "You have selected " + ChatColor.GREEN + "Normal" + ChatColor.GOLD + " arrows!");
}else if(getNextBowType(player) == BowType.SLOWNESS){
    setCurrentBowType(BowType.SLOWNESS, player);
    player.sendMessage(plugin.prefix + ChatColor.GOLD + "You have selected " + ChatColor.GREEN + "Slowness" + ChatColor.GOLD + " arrows!");
}else if(getNextBowType(player) == BowType.EXPLOSION){
    setCurrentBowType(BowType.EXPLOSION, player);
    player.sendMessage(plugin.prefix + ChatColor.GOLD + "You have selected " + ChatColor.GREEN + "Explosion" + ChatColor.GOLD + " arrows!");
}else if(getNextBowType(player) == BowType.FIRE){
    setCurrentBowType(BowType.FIRE, player);
    player.sendMessage(plugin.prefix + ChatColor.GOLD + "You have selected " + ChatColor.GREEN + "Fire" + ChatColor.GOLD + " arrows!");
}else if(getNextBowType(player) == BowType.POISON){
    setCurrentBowType(BowType.POISON, player);
    player.sendMessage(plugin.prefix + ChatColor.GOLD + "You have selected " + ChatColor.GREEN + "Poison" + ChatColor.GOLD + " arrows!");
}else if(getNextBowType(player) == BowType.LIGHTNING){
    setCurrentBowType(BowType.LIGHTNING, player);
    player.sendMessage(plugin.prefix + ChatColor.GOLD + "You have selected " + ChatColor.GREEN + "Lightning" + ChatColor.GOLD + " arrows!");
}else if(getNextBowType(player) == BowType.BLINDING){
    setCurrentBowType(BowType.BLINDING, player);
    player.sendMessage(plugin.prefix + ChatColor.GOLD + "You have selected " + ChatColor.GREEN + "Blindness" + ChatColor.GOLD + " arrows!");
}else if(getNextBowType(player) == BowType.LIFESTEAL){
    setCurrentBowType(BowType.LIFESTEAL, player);
    player.sendMessage(plugin.prefix + ChatColor.GOLD + "You have selected " + ChatColor.GREEN + "Lifesteal" + ChatColor.GOLD + " arrows!");
}else if(getNextBowType(player) == BowType.DAZE){
    setCurrentBowType(BowType.DAZE, player);
    player.sendMessage(plugin.prefix + ChatColor.GOLD + "You have selected " + ChatColor.GREEN + "Daze" + ChatColor.GOLD + " arrows!");
}else if(getNextBowType(player) == BowType.BOUNCE){
    setCurrentBowType(BowType.BOUNCE, player);
    player.sendMessage(plugin.prefix + ChatColor.GOLD + "You have selected " + ChatColor.GREEN + "Bounce" + ChatColor.GOLD + " arrows!");
}else if(getNextBowType(player) == BowType.NORMAL){
    setCurrentBowType(BowType.NORMAL, player);
    player.sendMessage(plugin.prefix + ChatColor.GOLD + "You have selected " + ChatColor.GREEN + "Normal" + ChatColor.GOLD + " arrows!");
}

 

I probably would do this just because its looks more efficient for me to read:

Code:

for(BowType type : BowType.Values()) {
      if(getNextBowType(player) == type) {
              //do something
            //the break means that it stops looping.
                break;
      }
}

-edit-

it seems ive misread something, in your case you may want to remove the break; since you want to check all permissions and compair it with per enum?

 

PimpDuck xize
Should be something like this for a switch statement, I don't quite remember since I haven't used switch statements in a long while.

Code:java

BowType bowtype = getNextBowType(player);
 
switch (bowtype) {
    case NORMAL:
        setCurrentBowType(BowType.NORMAL, player);
        player.sendMessage(plugin.prefix + ChatColor.GOLD + "You have selected " + ChatColor.GREEN + "Normal" + ChatColor.GOLD + " arrows!");
        break;
    case SLOWNESS:
        setCurrentBowType(BowType.SLOWNESS, player);
        player.sendMessage(plugin.prefix + ChatColor.GOLD + "You have selected " + ChatColor.GREEN + "Slowness" + ChatColor.GOLD + " arrows!");
        break;
    default:
        break;
}

 

MrSparkzz This code is working other than not looping back to the normal bowtype after it goes through the types the player has permission for http://paste.md-5.net/kaficuwosi.avrasm

 

PimpDuck
I think what you should do with the cases other than what you have now is this

Code:java

BowType bowtype = get[I]Current[/I]BowType(player);

then when it checks the current bow type, it'll switch to the next one

Code:java

case NORMAL:
    setCurrentBowType(BowType.SLOWNESS, player);
    index = 0;
    player.sendMessage(plugin.prefix + ChatColor.GOLD + "You have selected " + ChatColor.GREEN + "Slowness" + ChatColor.GOLD + " arrows!");
    break;
case SLOWNESS:
    setCurrentBowType(BowType.EXPLOSION, player);
    index = 1;
    player.sendMessage(plugin.prefix + ChatColor.GOLD + "You have selected " + ChatColor.GREEN + "Explosive" + ChatColor.GOLD + " arrows!");
    break;

Basically if you don't understand this, it will check the current bow type, then set the current bow type to the next bow type in the list. Once you get down to the BOUNCE bow type, you can have it set it back to normal and it will just keep looping every time you click.

This also gets rid of the need for the getNextBowType method, unless it's used somewhere else or you plan on using it in the future.. if you plan on using it in the future, personally I would use the deprecation annotation just so I know not to use it for the time being.

 

What about just dropping switch and conditionals, and instead do this:

Code:java

setCurrentBowType(bowtype, player);
player.sendMessage(plugin.prefix + ChatColor.GOLD + "You have selected " + ChatColor.GREEN + bowtype.name() + ChatColor.GOLD + " arrows!");

 

Rocoty
Well I guess that could work, but he's still getting the problem of obtaining the next bow type when he gets to the end of the list. So the switch statements would just be easier and I just adore them.. your method would work if he coded in a way to start at the beginning of the list after he reaches the end.

 

MrSparkzz Well....he did. If you read the code...

 

Rocoty
Yeah, but he stated that he was having trouble looping back to the first bow type. That's why I said to do it the second way I stated. The second way I stated removed all need for the getNextBowType method. Maybe there was an error in that method which was stopping it from looping.

 

MrSparkzz Even so, that logic should be put in the getNextBowType method, don't you agree? That way you don't have to write a new switch-statement every time you want to get the next bow type.

 

Rocoty
I guess you're right. I just love using switch statements. That's how I did the looping in my ServerControl plugin.. I had no problems with it and it was very simple. I've seen some people write hundreds of lines of complicated code to do something very simple and I see no point to it. For example with plugins like Factions and such, they have hundreds of lines of code in like 3+ classes that all call eachother doing something simple that could be done in say 50 lines of code or less.

 

MrSparkzz Yes. Always look for the simplest solution unless performance is critical. That's what I code by.