Build 602 busted my plugin...

Discussion in 'Plugin Development' started by Lodran, Mar 31, 2011.

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

    Lodran

    ... In a way that seems completely unnecessary.

    My plugin (creaturebox) uses a creature spawner's durability (ItemStack.getDurability()) to keep track of the spawner's type while it is in a player's inventory. Up until build 602 or thereabouts, this worked quite well.

    Now, when I query the item in hand for it's durability, it always returns zero for creature spawners, regardless of the item's actual durability.

    Further, when I query the item for it's material data, it returns null (which explains the durability issue).

    This seems to me like a completely unnecessary restriction in the way that bukkit works - the minecraft server has no difficulty keeping track of metadata for items that don't actually use it, so why does bukkit need to restrict our usage of otherwise unused metadata?

    Btw, without a reversal of this change, I'll probably abandon creaturebox, as I don't see another way to keep track of this bit of metadata.
     
  2. Offline

    Grum Bukkit Team Member

    URL to the commit of the change would be nice.

    Nothing in this area has been changed by CraftBukkit, if this has been changed it was because of the 1.3->1.4 update.
     
  3. Offline

    CorneliousJD

    Creaturebox is one of my servers favorite plugins, I'm really hoping we can at least point Lodran in the right direction on how to keep track of spawner types so he can continue development!
     
  4. Offline

    mbsuperstar1

    It is one of my favorite plugins also, without it my RPG dungeons won't be dungeons, and my monster towns will just be towns. *snivels* :'(
     
  5. Offline

    Lodran

    I thought that it was implied by the title, but build 602 was based on 1.3 - I haven't let anything in my system upgrade to 1.4 yet.

    Tonight, I'll do some testing with older builds, and see if I can figure out which build introduced this. It may be difficult though, because creaturebox won't build on older versions anymore.

    edit: @Grum - Digging through the change logs on bukkit (as opposed to craftbukkit), I found this:

    Number two looks like a strong possibility.

     
  6. Offline

    Celtic Minstrel

    No, that was just removing dedicated data classes from Material.REDSTONE (which is redstone dust, not wire) and Material.SIGN (which is the sign item, so it doesn't have data). It couldn't've caused your problem. In fact, I think I recall creaturebox working in build 592... that's CB build though.
     
  7. Offline

    Lodran

    It would appear that craftbukkit build 560 is the build that broke it.
     
  8. Offline

    Celtic Minstrel

    I'm pretty sure the problem does not lie in ItemStack; although Minecraft doesn't show a health bar, it appears that the proper data is stored in the item. Evidence: I gave myself first a pig spawner, then a creeper spawner; they did not combine into one stack. This pushes the probably location of the bug into CreatureSpawner. I can't tell how that might be the case, though.

    EDIT: Uh, okay, but I'm kinda confused how how that could be, since it just added material data classes...
     
  9. Offline

    Lodran

    At least in minecraft version 1.3, mob spawners show a health bar for me - not that it's particularly useful. :p

    I shoved this into onBlockPlace, to see where things had gone wrong.

    Regardless of the item showing damage when it's in your hand, this shows a durability of zero.
    If I uncomment the lines showing the material data, I get a null pointer exception when it tries to show data.
    I'm pretty mystified myself - I scanned through all of the changes in build 560, and couldn't see anything that had to do with durability, and not much that had to do with materials. I certainly couldn't see anything that looked specific to spawners.

    The interesting thing is that's the point at which BLOCK_PLACED became BLOCK_PLACE, so I had to build two versions of the plugin that differ only by one character to test it.
     
  10. Offline

    Celtic Minstrel

    Note: Further testing suggests that this is unlikely to be the problem and that it is fixed in the latest build.

    I found a possible culprit. This is the implementation of getItemInHand():
    Code:
        public CraftItemStack getItemInHand() {
            return new CraftItemStack( getInventory().b() );
        }
    As you can see, it calls the method net.minecraft.server.InventoryPlayer.b(). This method is different in the latest build in a very significant way.

    Prior to build 560, it looked like this:
    Code:
        public ItemStack b() {
            if (this.c < this.a.length) {
                return this.a[this.c];
            } else {
                return null;
            }
        }
    Now, it looks like this:
    Code:
        public ItemStack b() {
            return this.c < 9 && this.c >= 0 ? this.a[this.c] : null;
        }
    At first, you might think that's a good thing, because it returns null if this.c (which appears to represent the index of the inventory slot that is currently selected as being "in the hand"). But wait! I found this image!

    [​IMG]

    While the numbers in the slots here are clearly not right, since they go up to 44 rather than 35 (because this.a is an ItemStack[36]), they do give a potential clue: the quickbar slots are the last slots, not the first. So, perhaps that's where the problem is. Perhaps getItemInHand should instead be checking that 26 < c < 36.



    If I'm right, the workaround should be to do inven.getItem(inven.getHeldItemSlot()) instead of inven.getItemInHand(). If I'm wrong, well... I've got nothing.

    Okay, ignore that post. Here's another possibility.
    Code:
    [Apr 01@11:02:10pm] Verrier: Ah
    [Apr 01@11:02:13pm] Verrier: I see why
    [Apr 01@11:02:17pm] celticminstrel: You do?
    [Apr 01@11:02:32pm] Verrier: I think so, the issue is that  the block is "used" by the
    time he checks what it's durability is
    [Apr 01@11:02:41pm] Verrier: he's checking it onBlockPlace()
    [Apr 01@11:02:47pm] celticminstrel: Ahh...
    [Apr 01@11:02:50pm] Verrier: which by that point, isn't the item "gone" so he has air
    / null in hand
    [Apr 01@11:02:52pm] celticminstrel: Hm.
    [Apr 01@11:03:04pm] celticminstrel: It's a possibility.
    [Apr 01@11:03:17pm] celticminstrel: Though, I'm not really sure.
    [Apr 01@11:03:23pm] Verrier: I'm 99% sure that's what the issue is
    [Apr 01@11:03:31pm] Verrier: Fits the problem!
    [Apr 01@11:03:58pm] Verrier: So he would need to switch to onPlayerInteract with
    an action of RIGHT_CLICK_BLOCK and check it there IMO
    Something to try, at least.

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
     
    Last edited by a moderator: May 13, 2016
  11. Offline

    Drakia

  12. Offline

    Afforess

    @Celtic Minstrel ,

    I'm extremely aware of the current inventory setup, having reverse-engineered it for Backpack 2.0 - and those numbers are wrong. Slot 0 is the first action bar item, with slot 8 being the last. The armor slots are actually slots 36-39, inclusive. The Crafting table no longer uses slots anymore, since it does not store items there since 1.2. (or was it 1.1?)
     
  13. Offline

    Drakia

    @Lodran Looks like around the b560 mark they changed how block placed events are handled, which is probably why the data is discarded. I however can't figure out the exact reason the data is discarded.
    One method you could use is as Celtic Minstrel suggested, use the onPlayerInteract event to get the item data, and since the events are called sequentially you could set a variable for use in onBlockPlace that you can use to set the creature spawner type when placed.
     
  14. Offline

    Lodran

    The item in hand is "ItemStack{MOB_SPAWNER x 1}" when onBlockPlace is called, but getData() returns null. Still, grabbing the mob index before bukkit has a chance to discard the data might be the best workaround.

    Unfortunately, it looks like the MaterialData has already been discarded before onPlayerInteract is called. :'(

    I checked the inventory directly using theInventory.getItem(theInventory.getHeldItemSlot()), and the material data is missing there as well.

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
     
    Last edited by a moderator: May 13, 2016
  15. Offline

    Afforess

  16. Offline

    Drakia

    @Lodran Can you not use item.getDurability() in the playerInteract event to get the damage value?
     
  17. Offline

    Lodran

    Doesn't help - because the material data is gone (or at least inaccessible), durability returns zero.

    I'm beginning to wonder if access to durability is simply broken, although I'd figure someone besides myself would have noticed, if it was.

    I guess I should add test code to my plugin to dump durability of everything, just to see if it works...

    ...tomorrow.
     
  18. Offline

    Drakia

    It worked in my test plugin o.o

    Code:
    public class pListener extends PlayerListener {
    		@Override
    		public void onPlayerInteract(PlayerInteractEvent event) {
    			if (event.getAction() != Action.RIGHT_CLICK_BLOCK) return;
    			ItemStack item = event.getItem();
    			log.info("Interact - ItemId: " + item.getTypeId() + " Dmg: " + item.getDurability() + " Data: " + item.getData());
    			nextSpawner = item.getDurability();
    		}
    	}
    getData returned null, but getDurability returned the proper number.
     
  19. Offline

    Verrier

    As far as I know... getData is used for blocks in the world, but getDurability is used for inventory items. So you would want to use getDurability, which I also tested and can confirm with Drakia, it works as expected.
     
  20. Offline

    Lodran

    TBH, I find the MaterialData class to be tremendously confusing, both because bukkit's accessors use "getData" and "setData" to refer to different object types, and because itemStack has a "getData" accessor which seems to be useless.

    I re-ran my last tests, and I'm finding that for spawners, at least, durability is returned correctly at onPlayerInteract, and is zeroed at onBlockPlace.

    Given this, I should be able to get creaturebox running properly again.
     
  21. Offline

    Celtic Minstrel

    Seems to me that there must still be something not quite right in that image since there are only 36 slots plus armour. Still, that image fits better with other vaguely remembered stuff I have seen, so I'm willing to accept that it's (mostly) correct.

    As I recall from looking at the minecraft code, the armour is actually a separate array.

    Well, I guess the image is either out-of-date or for a different context (I did find it alongside a discussion of the packets themselves).


    For the MaterialData, I'm not sure what to think. Your way would be great for items and blocks in the inventory, since all of those only use the durability field to select from a set of variants, but for blocks in the world it won't work very well since a lot of them use the data in strange ways, such as bit fields.
     
Thread Status:
Not open for further replies.

Share This Page