Solved Dead, yet errorless code.

Discussion in 'Plugin Development' started by Build_and_Break, Jul 15, 2014.

Thread Status:
Not open for further replies.
  1. Offline

    Build_and_Break

    Hello,
    it appears my code which is supposed to "buff" the PvP strength of any person against mobs and players, has some problems. Throughout use, no errors are generated in the console. When doing /buff, I am entered into the hashmap. I can do it again and be removed. After that, it continually says I'm removed when I run the command, indicating that the if statement didn't find my name in the hashmap, even though it should have. Beyond that, the EntityDamageByEntity event handler I have set up doesn't seem to do ANYTHING. A pair of sharp eyes that could possibly scout out any problem I may have would be much obliged. Thanks, Build.

    Source Code: hastebin.com/atejenekad.avrasm
     
  2. Offline

    izarooni

    Build_and_Break
    I thi1nk you forgot your @EventHandler annotation on the EntiyDamageByEntity event

    edit:
    You also forgot to register events
     
  3. Offline

    fireblast709

    Build_and_Break line 179 puts it back in the Map, your class doesn't implement Listener, your Listener isn't registered, your event method misses the EventHandler annotation, you don't check instanceof Player before casting, you shouldn't be using the Minecraft Logger, damaged Entities are never dead (use EntityDeathEvent)
     
  4. Uh... I think you may have forgotten an
    Code:java
    1. @EventHandler
    before your
    Code:java
    1. public void onEntityDamageByEntity(EntityDamageByEntityEvent event) {


    Furthermore, your main class doesn't implement the Listener interface AND you're not registering the events on the onEnable() method.

    And god, that HashMap can be WAY more organized. Just make one that maps a Player's name to a boolean that defines whether they have the said buff. Here's a logic sequence since your code was messy to the point of being almost unreadable.

    1.Player Foo did /buff
    2. Does the HashMap contain the player Foo?
    2a. If not, add Foo to the HashMap and give him the buff status (associate him to "true"). End.
    2b. If so, does he have the buff status (true)?
    3a. If not, set his value to true (give him the buff back). End.
    3b. If so, set his value to false (take the buff away). End.

    Also, here's some tips to make your plugin, and your code overall, easier to understand.

    1. (Plugin quality) Let the console use the command too, to toggle the buff status of a player. Forcing your users to log into the server to use a command that doesn't really require a player to be sending it is bad.
    2. (Plugin quality) Let players change other players' buff statuses, with an optional argument <player> to the /buff command. If necessary, add a permission like "buff.others" so users who have permission management plugins can take advantage of that by not having to OP everybody.
    3. (Not-spamming-the-console type of quality) You don't need to say the plugin has been enabled to the console, Bukkit already does it for you.
    4. (IMPORTANT) DO NOT cast a CommandSender to Player before checking if "sender" is an instance of Player, or else you'll get a ClassCastException if the command gets sent from the console.
    5. (VERY IMPORTANT) As I can see, your commands return false. Oh wait, your entire onCommand returns false no matter what. Want a tip of mine? Always return true when the command belongs to your plugin so you can customize error messages for each kind of error when calling the command, returning false is basically telling the user they did something wrong but without telling them what. It annoys the hell out of everyone, specially if it happens to every single command.
    6. (Code readability) Don't repeat yourself (DRY). Inside your broken event handler, there's lots of huge if statements doing basically the same thing on different situations. Avoid that.
    7. (HashMap usage) Adding and removing players all the time into and out of the HashMap should be avoided. An ArrayList would have taken care of that. But the following is way better: add them to the HashMap when they first use (or, like in number 2, are the target of) the command, and remove them when they leave the server to save memory, unless you want the buff status to be kept on the event of an unexpected disconnection (not necessary in my opinion, unless there's a chance users without buff.buff will need the buff status through disconnects/reloads/restarts; in other words, if you want to make it permanent, you shouldn't be using a HashMap anyway, save it to a file, but ONLY IF you need it to be 100% permament!).
    8. (Bukkit API usage issue/suggestion) Don't save players as Strings, otherwise, you'll have to call getPlayer() every single goddamn time something needs that player. When adding them to the HashMap, getPlayer() their names once and save them in the HashMap as Player instances mapped to boolean values. On the deprecation of getPlayer(): unless you want to make something that will last more than a session, getPlayer() is completely fine. Otherwise, you'll have to use a file and, in order to do so and to avoid errors upon players changing their names, you'll have to store their UUIDs. Here's a very interesting article on the said deprecation.
    9. (EXTREMELY IMPORTANT) Just because the code compiles fine or no exceptions pop up, it doesn't mean the code is error-less. The code being able to compile means there are no code-related errors like syntax and simple logic errors. "No exceptions popping up in the console" means there are no "exceptional", unexpected, or simply impossible situations going on. Exceptions were created so the compiler/IDE wouldn't have to test every single possible state your code would get to and find the "exceptional" ones. I define error-less code as being: readable, meaning the programmer(s) can understand the code they wrote themselves; understandable, meaning programmers that didn't take part in the process of writing the code can understand what it does without thinking their asses off; compilable, meaning there can actually be called "code"; error-safe, meaning it can handle almost all, if not all, the exceptional situations the user can and will create, and finally, capable of doing what it's supposed to do as efficiently as possible. Your code is none of these; read my suggestions and your code quality will improve greatly! :D

    If you can follow those instructions, rewrite your plugin's logic and post a new example here, and I'll be happy to help you with any other problems you may find. Some useful pages are the Plugin Tutorial (which I assume you've read completely, if not, do it, if so, repeat), the Event API, which has some really important/useful tips, and this entire post also seems useful. :p

    TL;DR: Read, rinse, repeat. Oh wait.

    //edit: Looks I got ninja'd. I accidentally my entire answer way big expected. When I started writing it, there were no other answers. 2fast4me.jpg
     
    izarooni likes this.
  5. Offline

    izarooni

    bruno.paschoalinoto
    You're the good kind of guy, taking time out of your day to type out that monster.
     
  6. @izarooni
    I didn't even realize it got that big. woah. Anyways, I'm on vacations, so whee, free time is free.
    What to do with said free time? My friends would say, get a girlfriend, read a book! I'd say: stay up until 4am to fix Eclipse and convert over 30 projects back to the Eclipse-ish-kind-of-project by hand, then realize all the Bukkit plugins projects were broken since I didn't touch them for a long time. Then spent the entire following day learning how to use Maven and debug plugins on the fly. Done? Then realize you are not creative, and cannot get ideas for plugins, so go on a forum answering spree overnight! Like before, rinse and repeat.
    life is fun!
     
    izarooni and xTigerRebornx like this.
  7. Offline

    Necrodoom

    bruno.paschoalinoto likes this.
  8. The real question you should all be asking yourselves: is the OP going to read my "monster"?
    Oh, it seems I'm the only one online in here. So it's only you and me, monster.
    ain't that a cute little post...
     
  9. Offline

    Build_and_Break


    To avoid quoting all of that, I'll just reply here: wow. Very touching and insightful. Thanks for the great advice. Same goes to everyone who gave some tips, much obliged.
     
Thread Status:
Not open for further replies.

Share This Page