Solved Simple NoRain plugin yet it doesn't work...

Discussion in 'Spigot Plugin Development' started by Wilsoon, Jan 18, 2020.

Thread Status:
Not open for further replies.
  1. So I am trying to make a simple NoRain plugin as practice, this is my code but whenever I do /norain on or /norain off, it just returns the usage in plugin.yml, my code has 3 returns, I didn't put a return on the other one because it would cause an error "Unreachable Code".

    The problem is at onCommand()


    Code (Java):
    package com.gmail.calorious.Rain;

    import org.bukkit.plugin.java.JavaPlugin;

    import org.bukkit.ChatColor;
    import org.bukkit.command.Command;
    import org.bukkit.command.CommandSender;
    import org.bukkit.event.EventHandler;
    import org.bukkit.event.Listener;
    import org.bukkit.event.weather.WeatherChangeEvent;

    public class Main extends JavaPlugin implements Listener {
        public static boolean enabled = true;
          @Override
          public void onEnable() {
              getLogger().info(ChatColor.GREEN + "                  [NO RAIN PLUGIN]                        ");
              getLogger().info(ChatColor.GREEN + "      This is a very lightweight plugin, made by Java     ");
              getServer().getPluginManager().registerEvents(this, this);
          }
          @Override
          public void onDisable() {
              getLogger().info(ChatColor.RED + "Plugin has been disabled successfully!");
          }
          @EventHandler
          public void onWeatherChange(WeatherChangeEvent e) {
              if(enabled = true) {
                  if(e.toWeatherState()) {
                      e.setCancelled(true);
                  }
              }
          }
          @Override
          public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args) {
              if(cmd.getName().equalsIgnoreCase("norain")) {
                  if(args.length == 0) {
                      sender.sendMessage(ChatColor.RED + "            NoRain");
                      sender.sendMessage(ChatColor.GOLD + "--------------------------------");
                      sender.sendMessage(ChatColor.GREEN + " /norain on - No Rain, foreeveerrr!");
                      sender.sendMessage(ChatColor.GREEN + " /norain off - There will be rain now.");
                      sender.sendMessage(ChatColor.GOLD + "--------------------------------");
                      return true;
                  }
                  if(args.length == 1) {
                      if(args.toString() == "on") {
                         if(enabled = true) {
                             sender.sendMessage(ChatColor.RED + "Fail: Plugin is already enabled!");
                         } else {
                         sender.sendMessage(ChatColor.GREEN + "Success: Plugin has been enabled!");
                         return true;
                      }
                      if(args.toString() == "off") {
                          if(enabled = false) {
                              sender.sendMessage(ChatColor.RED + "Fail: Plugin is already disabled!");
                              return true;
                          } else { Main.enabled = true; sender.sendMessage(ChatColor.GREEN + "Success: Plugin has been disabled!"); return true; }
                      }
                      }
                  }
              }
            return false;
          }
    }
     
     
  2. You can not compare Strings with ==. Compare them using .equals() instead.

    You cannot compare a value using the assignment operator =. Use == instead. Since you are checking a boolean, you do not need to compare to a value, you can simply pass it into the if statement like so: if (myBooleanValue) to check if myBooleanValue is true or if (!myBooleanValue) to check if it is false.

    That should get you started on fixing your code.
     
    • Agree Agree x 1
  3. Not sure, but don't you still need to register your command?
    Code (Text):
    getCommand("norain").setExecutor(this);
     
  4. He is overriding the onCommand() method from his main class so he does not need to do this in this case I am quite sure.
     
    • Agree Agree x 1
  5. arrays cant be converted to strings, you need to use arrays correctly. since this is a fundamental object, as well as the other basic level issues, do some research. google how arrays work and how to use them.
     
  6. MiniDigger

    Supporter

    Why don't you just use the gamerule?
     
  7.  
  8. 1.
    Basically you can't just make args.toString();
    args is an array, you should read and learn about arrays first.

    instead of args.toString();
    you can use args[0] in your case.


    2.
    Also, you cant compare two strings with ==
    you should use args[0].equals("on); or args[0].equalsIgnoreCase("on);
    equals for the case is not the best option, because the user can type /norain On
    so you should replace your String =='s with .equalsIgnoreCase

    3.
    Also, you were checking if the argument was "off" inside of the if where you checked if it was "on"
    like
    Example:
    Code (Text):
    if (arg[0].equalsIgnoreCase("on")){
     //some code
    if (arg[0].equalsIgnoreCase("off")){
     //some code
    }
    }
    It can turn to this:
    Example:
    Code (Text):
    if (arg[0].equalsIgnoreCase("on")){
     //some code
    }
    if (arg[0].equalsIgnoreCase("off")){
     //some code
    }
     
    But it would be best if it turned to
    Example:
    Code (Text):
    if (arg[0].equalsIgnoreCase("on")){
     //some code

    } else if (arg[0].equalsIgnoreCase("off")){
     //some code
    }
     
    Finally:

    If you want to be spoonfed, you can just copy-paste the code I have provided bellow.
    But if you want to improve read the steps upside and try to improve yourself.
    Wish you luck :)

    instead of:
    Code (Text):
    if(args.toString() == "on") {
                         if(enabled = true) {
                             sender.sendMessage(ChatColor.RED + "Fail: Plugin is already enabled!");
                         } else {
                         sender.sendMessage(ChatColor.GREEN + "Success: Plugin has been enabled!");
                         return true;
                      }
                      if(args.toString() == "off") {
                          if(enabled = false) {
                              sender.sendMessage(ChatColor.RED + "Fail: Plugin is already disabled!");
                              return true;
                          } else { Main.enabled = true; sender.sendMessage(ChatColor.GREEN + "Success: Plugin has been disabled!"); return true; }
                      }
                      }
    you can use:
    (to fix you issues)
    Code (Text):
    if (args[0].equalsIgnoreCase("on")) {
        if (enabled = true) {
            sender.sendMessage(ChatColor.RED + "Fail: Plugin is already enabled!");
        } else {
            sender.sendMessage(ChatColor.GREEN + "Success: Plugin has been enabled!");
        }
        return true;
    }else if (args[0].equalsIgnoreCase("off")) {
        if (enabled = false) {
            sender.sendMessage(ChatColor.RED + "Fail: Plugin is already disabled!");
        } else {
            Main.enabled = true;
            sender.sendMessage(ChatColor.GREEN + "Success: Plugin has been disabled!");
        }
        return true;
    }
     
     
    #8 Ablax, Jan 18, 2020
    Last edited: Jan 18, 2020
  9. his copy and paste has errors, = sets a value, == compares. use == to compare the booleans. or just dont use == and use !
    if(enabled)
    if(!enabled)
     
  10. There are more errors in onCommand than in the event listener, I explained them correctly upside :)


    (you = author of post)

    I just looked at onWeatherChange and saw the mistake, you used one = in the if check

    Code (Java):

    //code
    if(enabled = true) {
    //code
     
    you probably wanted to use

    Code (Java):

    //code
    if(enabled == true) {
    //code
     
    ^
    it will work like this but not as you made it, but it's still not a good idea, you should better use
    and learn a little about booleans :)

    Code (Java):

    //code
    if(enabled) {
    //code
     
     
  11. is it a competition? youre still allowing him to copy and paste code that is incorrect

    /e you didnt explain comparing primitives. you only explained how to compare strings

    /e2 @op why not make a toggle
    if(command)
    enabled = !enabled;
    player.msg(enabled ? "has been enabled" : "has been disabled");

    and ur done

    /e3 also if your command method is only handling one command (ie you only have your /norain command), you dont need to check for the command. its the only command that will be called.
     
    #11 Warren1001, Jan 18, 2020
    Last edited: Jan 19, 2020
  12. All primitive types (byte, char, short, int, long, float, double, boolean) can be compared with ==
    because they contain their value in the stack memory.
    All objects should be compared with .equals() because == compares their reference memory, which is incorrect. Object's stack memory contains a reference to the address in the heap memory, that's why == just compares their references, there can be 2 objects with absolutely identical fields, but different instances.

    String is semi-primitive, but it works as a object so it MUST be compared with .equals() there is a .equalsIgnoreCase() method providen for ease.
    Is that explanation enough for you?
     
    • Like Like x 1
  13. Code (Java):
    package com.gmail.calorious.Rain;

    import org.bukkit.plugin.java.JavaPlugin;

    import org.bukkit.ChatColor;
    import org.bukkit.command.Command;
    import org.bukkit.command.CommandSender;
    import org.bukkit.event.EventHandler;
    import org.bukkit.event.Listener;
    import org.bukkit.event.weather.WeatherChangeEvent;

    public class NoRain extends JavaPlugin implements Listener { // you should avoid using generic class names like "Main". if someone were wanting to use your plugin as an api, they wouldnt know what from what
     
       private boolean enabled = true; // unneeded use of static
     
       @Override
       public void onEnable() {
          getLogger().info(ChatColor.GREEN + "                  [NO RAIN PLUGIN]                        ");
          getLogger().info(ChatColor.GREEN + "      This is a very lightweight plugin, made by Java     ");
          getServer().getPluginManager().registerEvents(this, this);
       }
     
       @Override
       public void onDisable() {
          getLogger().info(ChatColor.RED + "Plugin has been disabled successfully!"); // this is alright for testing, but redundant for production code.
          // the server always calls a disable message, so youre just duplicating the message
       }
     
       @EventHandler
       public void onWeatherChange(WeatherChangeEvent e) {
          if(enabled && e.toWeatherState()) e.setCancelled(true);
       }
     
       @Override
       public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args) { // return false will display the usage help. return true wont
          // no need to check the command when you only have one command
          if(args.length == 0) { // you can leave this if you want your "on" and "off", or use whats below
             sender.sendMessage(ChatColor.RED + "            NoRain");
             sender.sendMessage(ChatColor.GOLD + "--------------------------------");
             sender.sendMessage(ChatColor.GREEN + " /norain on - No Rain, foreeveerrr!");
             sender.sendMessage(ChatColor.GREEN + " /norain off - There will be rain now.");
             sender.sendMessage(ChatColor.GOLD + "--------------------------------");
             return true;
          }
          if(args.length == 1) {
             if(args[0].equalsIgnoreCase("on")) { // this is how you use arrays. args[0] suggests the first value, at index 0. java counting starts at 0.
                if(enabled) { // you can compare booleans without ==, as seen here and below
                   sender.sendMessage(ChatColor.RED + "Fail: Plugin is already enabled!");
                   return true;
                }
                enabled = true;
                sender.sendMessage(ChatColor.GREEN + "Success: Plugin has been enabled!");
                return true;
             }
             if(args[0].equalsIgnoreCase("off")) {
                if(!enabled) {
                   sender.sendMessage(ChatColor.RED + "Fail: Plugin is already disabled!");
                   return true;
                }
                enabled = false;
                sender.sendMessage(ChatColor.GREEN + "Success: Plugin has been disabled!");
                return true;
             }
          }
          return false;
         
          // or get rid of everything above and just do this
          enabled = !enabled;
          sender.sendMessage(enabled ? ChatColor.GREEN + "Success: Plugin has been enabled!" : ChatColor.GREEN + "Success: Plugin has been disabled!");
          return true;

          // could even one liner it
          sender.sendMessage((enabled = !enabled) ? ChatColor.GREEN + "Success: Plugin has been enabled!" : ChatColor.GREEN + "Success: Plugin has been disabled!");
          return true;
       }
     
    }
    proper spoonfeed
     


  14. I disagree, you can do better, like:

    Code (Java):

       @EventHandler
       public void onWeatherChange(WeatherChangeEvent e) {
          e.setCancelled(e.isCanceled() || (enabled && e.toWeatherState()));
       }
     
       @Override
       public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args) { // return false will display the usage help. return true wont
          // no need to check the command when you only have one command
          if(args.length == 0) { // you can leave this if you want your "on" and "off", or use whats below
             sender.sendMessage(ChatColor.RED + "            NoRain");
             sender.sendMessage(ChatColor.GOLD + "--------------------------------");
             sender.sendMessage(ChatColor.GREEN + " /norain on - No Rain, foreeveerrr!");
             sender.sendMessage(ChatColor.GREEN + " /norain off - There will be rain now.");
             sender.sendMessage(ChatColor.GOLD + "--------------------------------");
             return true;
          }else
          if(args.length == 1) {
             if(args[0].equalsIgnoreCase("on")) { // this is how you use arrays. args[0] suggests the first value, at index 0. java counting starts at 0.
                if(enabled) { // you can compare booleans without ==, as seen here and below
                   sender.sendMessage(ChatColor.RED + "Fail: Plugin is already enabled!");
                   return true;
                }
                enabled = true;
                sender.sendMessage(ChatColor.GREEN + "Success: Plugin has been enabled!");
                return true;
             }else if(args[0].equalsIgnoreCase("off")) {
                if(!enabled) {
                   sender.sendMessage(ChatColor.RED + "Fail: Plugin is already disabled!");
                   return true;
                }
                enabled = false;
                sender.sendMessage(ChatColor.GREEN + "Success: Plugin has been disabled!");
                return true;
             }
          }
          sender.sendMessage(ChatColor.Red + "Please use /norain on/off");
          return true;
       }
     
     
    #14 Ablax, Jan 19, 2020
    Last edited: Jan 19, 2020
    • Optimistic Optimistic x 1
  15. oo i like that event method

    else is redundant if youre returning in the previous if statement. else prevents an the following if statement from being called if a previous one was called, but if the previous one called returns then it isnt called anyway
     
    • Like Like x 1
  16. My problem is solved and I don't wish for any of these arguments to happen in the community, I'm going to close this. I'll learn more about the arrays and everything, thanks for everyone's help!
     
    • Funny Funny x 1
    • Optimistic Optimistic x 1
Thread Status:
Not open for further replies.