Unexplainable bug

Discussion in 'Plugin Development' started by xDevious, May 6, 2014.

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

    xDevious

    Hello,

    I am updating my plugin, Minepay.

    The plugin's function is to pay out a configurable amount of money when a player mines an ore.

    The class for this is here.

    My indentations are correct it is because i pasted the code in that things have changed.

    Code:
    package me.styltwist.minepay;
     
    import java.util.ArrayList;
    import java.util.HashMap;
     
    import net.milkbowl.vault.chat.Chat;
     
    import org.bukkit.enchantments.Enchantment;
     
    import org.bukkit.Material;
    import org.bukkit.entity.Player;
    import org.bukkit.event.EventHandler;
    import org.bukkit.event.EventPriority;
    import org.bukkit.event.Listener;
    import org.bukkit.event.block.BlockBreakEvent;
    import org.bukkit.inventory.ItemStack;
     
    public class MyBlockListener implements Listener {
        public static Chat chat = null;
        public static MinePay plugin;
        public static HashMap<Material, Integer> orelist = new HashMap<Material, Integer>();
        ArrayList<Material> ingot = new ArrayList<Material>();
        
        public MyBlockListener(MinePay instance) {
    plugin = instance;
    int diamond = plugin.getConfig().getInt("diamond");
    int coal = plugin.getConfig().getInt("coal");
    int emerald = plugin.getConfig().getInt("emerald");
    int redstone = plugin.getConfig().getInt("redstone");
    int lapis = plugin.getConfig().getInt("lapis");
    int quartz = plugin.getConfig().getInt("quartz");
     
    orelist.put(Material.COAL_ORE, coal);
    orelist.put(Material.DIAMOND_ORE, diamond);
    orelist.put(Material.REDSTONE_ORE, redstone);
    orelist.put(Material.LAPIS_ORE, lapis);
    orelist.put(Material.EMERALD_ORE, emerald);
    orelist.put(Material.QUARTZ_ORE, quartz);
     
    }
     
    @EventHandler(priority=EventPriority.HIGH)
        public void onBlockBreak(BlockBreakEvent event){
       Material block = event.getBlock().getType();
                Player player = event.getPlayer();
                if(event.isCancelled()){
               
                }else{
               if(player.hasPermission("minepay.reward")) {
               if(orelist.containsKey(block)){
               int Money = orelist.get(block);
                   if(player.getItemInHand().containsEnchantment(Enchantment.SILK_TOUCH)){
                   }else{
                   plugin.econ.depositPlayer(player.getName(), Money);
                   player.sendMessage("§3You recieved §b" + Money + " §3for mining §b" + block + "§3.");
                   }
               }else if(block == Material.IRON_ORE){
               if(player.hasPermission("minepay.reward")) {
               int iron = plugin.getConfig().getInt("iron");
               if(player.getItemInHand().containsEnchantment(Enchantment.SILK_TOUCH)){
                   
               }else{
               player.getWorld().dropItemNaturally(event.getBlock().getLocation(), new ItemStack(Material.IRON_INGOT, 1));
               event.setCancelled(true);
               event.getBlock().setType(Material.AIR);
               plugin.econ.depositPlayer(player.getName(), iron);
               player.sendMessage("§3You recieved §b" + iron + " §3for mining §b" + block + "§3.");
               }
               }
               }else if(block == Material.GOLD_ORE){
               if(player.hasPermission("minepay.reward")) {
               int gold = plugin.getConfig().getInt("gold");
               if(player.getItemInHand().containsEnchantment(Enchantment.SILK_TOUCH)){
                       
               }else{
               player.getWorld().dropItemNaturally(event.getBlock().getLocation(), new ItemStack(Material.GOLD_INGOT, 1));
               event.setCancelled(true);
               event.getBlock().setType(Material.AIR);
               plugin.econ.depositPlayer(player.getName(), gold);
               player.sendMessage("§3You recieved §b" + gold + " §3for mining §b" + block + "§3.");
               }
               }
               }
               }
               }
    }
    }
    The issue I am having is that when the player is in survival mode, the payout for redstone doesn't happen, however in creative mode it does.

    I can't understand how this happens because redstone runs on the exact same code as all the other ores other than gold or iron.

    Anyone have any ideas?
     
  2. Offline

    EnderTroll68

    Your problem is that when someone starts to mine Redstone ore, it turns into lit redstone, which has a different item id than normal redstone.

    http://www.minecraftinfo.com/IDList.htm

    See items 73 and 74
     
    xDevious likes this.
  3. Offline

    xDevious

    Thank you so much, I never thought about it like that, I've been trying to fix this bug for a long time :)
     
  4. Offline

    EnderTroll68

    I had the same thing, it drove me insane. When you have a problem like this, add debug code that broadcasts to chat with what you are breaking or doing
     
  5. Offline

    xDevious

    Before I used:

    Code:
    orelist.put(Material.REDSTONE_ORE, redstone);
    But there is no "Material.LIT_REDSTONE_ORE" or anything of the sort so how could I fix this?
     
  6. Offline

    GeorgeeeHD

    You could change the orelist to be type Integer, Integer and store the blocks id number instead of material name. then store for redstone 73 and 74
     
  7. Offline

    xDevious

    I began to do this, and realised the amount of code that I would have to change was enormous, I later discovered 'Material.GLOWING_REDSTONE_ORE' and added that to redstone, however the results were the same, there was still no payout when redstone ore was mined?;/
     
  8. Offline

    minoneer

    what does your material list look like? Something like this?

    Code:java
    1. orelist.put(Material.COAL_ORE, coal);
    2. orelist.put(Material.DIAMOND_ORE, diamond);
    3. orelist.put(Material.REDSTONE_ORE, redstone);
    4. orelist.put(Material.GLOWING_REDSTONE_ORE, redstone);
    5. orelist.put(Material.LAPIS_ORE, lapis);
    6. orelist.put(Material.EMERALD_ORE, emerald);
    7. orelist.put(Material.QUARTZ_ORE, quartz);


    Not a good idea at all - Item ID's are deprecated and will be removed soon - this would break the whole plugin.
     
    Mrawesomecookie and Garris0n like this.
  9. Offline

    xDevious

    Yes exactly that, is there something wrong with it?;/

    - BUMP -

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
     
    Last edited by a moderator: Jun 8, 2016
  10. Offline

    minoneer

    Nope, it should work this way. Try to add some debug output - maybe that'll show the cause.
     
  11. Offline

    GeorgeeeHD

    there actually might be a problem with it. you put the same number into the map for both redstone materials. its possible that messes it up, but im not sure as i didnt read the whole code to see if that would matter, but you would probably know as you wrote it xD

    EDIT: derp just read it and realized its just the money you get paid for breaking the block, sorry :S Does anything happen when you break the redstone? Any errors or something like that
     
  12. Offline

    xize

    What happens if you just use the EnumMap instead of a HashMap for the Material enum?
    from looking to the code I don't see the error for a 100%, but of course enums as keys in a HashMap can cause some problems.
     
    Garris0n and xTigerRebornx like this.
  13. Offline

    EnderTroll68

    Please do not tell people false information. They are never removing item ids. They are deprecated because they are able to change. If they change, he would have to change ONE LINE
     
  14. Offline

    coasterman10

    EnderTroll68 It's still a better idea to use the enum values whenever possible, since they are a lot more descriptive than the integer values. A plain integer is a magic value, and someone who has never read this code before will have no idea what the significance of it is, as opposed to the enum value in which case they will easily understand what it means. In the worst case, the number should be defined as a final static integer (effectively a constant) and referred to by that variable.
     
  15. EnderTroll68 Mojang say that they're removing IDs in favour of the names. Which makes sense, magic numbers aren't good. Granted they've been saying it for a while... But I believe they're slowly being phased out and eventually won't mean anything.
     
  16. Offline

    RawCode

    i already bored to explain why mojang can't remove IDs and nobody can.

    learn java, read about arrays.
     
  17. Offline

    minoneer

    Since this is not the subject of this thread, it's the wrong place to discuss this. Just two things:
    * Mojang is removing item ID's. Having internal indices mapping to a block is not the same as a global, fix and unique Block/Item ID.
    * It is (if the API is written correctly) generally a bad idea to use deprecated methods if there is a replacement. That's the whole point of deprecation.

    Now back to topic:
    xDevious were you able to fix that problem? Or is it still not working with glowing redstone ore?
     
  18. Offline

    Garris0n

    They won't remove them, but the idea is that between worlds ids will not be consistent because of however the mod/plugin API will work. They've already possibly broken data values, I wouldn't be too surprised if they broke anything else. There was also talk of getting rid of color codes completely and going full JSON.
     
  19. Offline

    xDevious

    No errors at all, redstone still works in creative mode, but when in survival when it changes to glowing redstone it still doesn't pay out, i can't understand it;/

    minoneer nope;/
     
  20. Offline

    minoneer

    Ok, then I suggest adding a debug line - something like System.out.pringln(event.getBlock().getType()); - and check what the output is for the glowing redstone ore.
     
  21. Offline

    xDevious

    Ive added this, just going to test now:

    @EventHandler(priority=EventPriority.HIGH)
    public void onBlockBreak(BlockBreakEvent event){
    Material block = event.getBlock().getType();
    Player player = event.getPlayer();
    player.sendMessage("You mined" + block); <------- Debug Line
    if(event.isCancelled()){

    AFTER TEST:

    Debug line isn't even appearing on any blocks? Perhaps the file exportion isn't completing correctly?
     
  22. Offline

    minoneer

    Did you move/export your new jar to the plugins folder?
    Did you delete/replace the old jar?
    Did you stop and then start your server?
     
  23. Offline

    xDevious

    I had the server off when i exported it, and I replaced the current file.

    It also notifies me that the exportion finished with a warning, which is this:

    Problem writing MinePay/.DS_Store to JAR: duplicate entry: .DS_Store
     
  24. Offline

    xize

    xDevious

    have you tried to use a EnumMap rather than a HashMap?

    you know that most Enums share the same hashcodes?, a hashcode is used to identify the object which type object something is but because a enum is already a type from it self it means that it even could accept other enums to.

    think on DyeColor for example it means it could return for everything true or false.

    on the other hand an EnumMap would prevent this kind of behaviour because its specially designed for enums!.

    you can instance it like this:

    Code:
    //the Material class is needed in this constructor in order things will be alright and the keys can only be used from that Enum class.
    EnumMap<Material, Integer> orelist = new EnumMap<Material, Integer>(Material.class);
    
     
  25. Offline

    xDevious

    No I haven't, but I will try that when I have made sure the exportion is working correctly if the error still isn't fixed.
     
  26. Offline

    minoneer

    Have you tried deleting the current file before you export the new one?

    You could also try to increaes the version number in your plugin.yml and check if the output on startup changes aswell. Then you can be sure it is the new compiled version that is being loaded.

    Where did you get that information from? It is just wrong in multiple aspects. HashMaps work perfectly fine with Enums:

    "you know that most Enums share the same hashcodes?"
    Not true. Enum inherits it's hashcode method from the "Object" class, which by default returns different integers for different objects - and therefore for different enum values.

    "a hashcode is used to identify the object which type object something is but because a enum is already a type from it self it means that it even could accept other enums to."
    Not 100% sure what you mean by that, but a hashcode is NOT used to identify which type of object something is. It is an index that allows fast access in certain data structures - nothing more. It is by NO means an unique identification key - two keys in a HashMap can have the same hashcode and it would still only return the correct value. You also can not put other enum types in a hashmap which has Material as a key.

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
     
    Last edited by a moderator: Jun 8, 2016
  27. Offline

    xize

    minoneer
    you could test it yourself by putting multiple Material enums inside the HashMap and then checking the length if I'm correct it would override the same key rather than making a another key, and also Enumurations are made to be abstract which means they don't have a object id, their id is straight from the Enum class and obtained from java and only in rare cases you would add your own custom things, an HashCode is the jvm memory id or better said reference to the type so that type A is still different than type B though its the same object and since Enums are immutabele objects you cannot expect this to work see example below these are immutable objects to:

    Code:
    String a = "hi";
    String b = "hi";
    hash.put(a, "test");
    hash.put(b, "test");
    
    do you really believe this would work if you add this as keys in a HashMap?, the length would be 1 and not 2 as expected because a HashMap cannot add duplicates.

    you can find some alternative info here http://eclipsesource.com/blogs/2012/09/04/the-3-things-you-should-know-about-hashcode/ all enums have actually the same type hashcode and the HashMap will see it as 1 thing how many enums you try to put in even different enums.
     
  28. Offline

    minoneer


    No, I don't expect that to work. "hi" and "hi" are equal, therefor it is considdered the same key. However, two different Enum values are actually two different key, because no eaquals method will ever return true for them. Some very interesting explanations on the topic:

    http://leakfromjavaheap.blogspot.de/2013/05/why-can-we-use-enums-as-hashmap-keys.html

    However, I do agree that it makes sense to use an EnumMap when using Enums as key - but the other way is still possible.
     
  29. Offline

    xize

    minoneer

    readed it, but what I try to explain is this:

    Material enum looks like this, (this totally doesn't look like the orginal bukkit Material enum though):

    Code:
    public enum Material {
              COBBLESTONE,
              REDSTONE
    }
    
    now when you have a HashMap and put as key Material.COBBLESTONE and followed by adding Material.REDSTONE both got the same object litterly this cause a conflict correct?, because a Enum is a singleton and called in a abstract way it could only work when you have fields in it because then it is doing a chance due the HashCode which got calculated by those fields, an HashMap checks this through HashCode but if the HashCode is the same it won't add it because it simply won't allow you to add duplicates.

    in this case you should really use EnumMap because its designed especially for Enumuration and to completely avoid this kind of problem because it doesn't check on the HashCode but rather on the Entrys, thats also the reason when you try to instance such Map you need to add the Enum class inside the constructor.
     
  30. Offline

    minoneer

    xize
    I absolutely agree it's much better to use an enum map. No question about that. BUT

    That is not true. Two Objects can very well have the same HashCode and both be added to a HashMap. HashMap determines if it is the same Object using the .equals method and not the HashCode. I could, in theory, implement a HashCode method always returning the same integer (i.e. 1). That would not make any sense, but the HashMap would still work - just a lot less performance friendly.
     
Thread Status:
Not open for further replies.

Share This Page