CRAFTBOOK-3175 - Reported by Treuce

On 1.9 Spigot, I don't know any more information about the server since I'm not an admin, only a player who would like to see this fixed, which is: I've noticed that when someone his level to XP potions via spawner it gives him a lot less than it used to give a week ago. Today I discovered an other bug: for level 30 it gave me one which couldn't be picked up by me or by anyone online. I've tried changing my level to potions on 50 and 70 level, and it gave me under 1 stack both times.

Comment by me4502

That number is configurable by the server admin, it's not hard coded in the plugin.

Comment by Treuce

He told me that they didnt change anything in the files, but could you tell me where that part is?

Comment by me4502

It's inside the xp-storer.yml configuration,

it's the xp-per-bottle option.

The default is 16, however that has a high loss on it IIRC.

Comment by minoneer

I am a server Admin, and I might be able to provide some more detail on this.

We have the same issue on our server: UP to 2047 (including, XP, not level), which is about 35 Levels, everything works as expected, the player receives one or two stacks of XP. But starting at 2048 XP, only a single item is dropped which cannot be picked up by the player. Further investigation showed a negative stack size.

At some amount, XP bottles start to drop again, but much less than they should (lvl 60 gives 29 bottles, while 20 lvl already result in 34 with our configuration, see below.

All in all, this very much sounds like an overflow problem in a number range somewhere, maybe that'll help to track it down.

Versions:
Server: Paper version git-Paper-612 (MC: 1.9)
CraftBook: 3.9-SNAPSHOT (I believe b24)

{code:|title=config}
XPStorer:
require-bottle: false
xp-per-bottle: 16
block: ENDER_PORTAL_FRAME
require-sneaking-state: 'no'

Comment by FatherWh0

I've experience the same on my server. Xp bottles properly up to 1050 xp but beyond that one of three things occurs, no xp bottles (but /remove command shows to remove 1 drop), a negative bottle is picked up or a drop is visible but can't be picked up (again, /remove despawns it).
CraftBook v3.9-SNAPSHOT b24
Spigot 1.9.2 git-Spigot-a1b8bf3-f01861f

XPStorer:
    require-bottle: false
    xp-per-bottle: 7
    block: EMERALD_BLOCK
    require-sneaking-state: 'yes'

Comment by FatherWh0

I've updated to the recent release 3.9u5 and the issue still exists.

Comment by me4502

I've just done another test where I gave myself intervals of 1500 xp. It worked every time.

Please ensure that you are running the latest versions of spigot.

If everything still fails with updated versions of spigot, I have added debug nodes to XPStorer in the latest build of CraftBook. If you enable debug mode in the config, and add the 'xpstorer' debug flag, it will enable it.

Once that is done, please try using xpstorer with the amounts of XP that cause issues, and send me the log.

Comment by Treuce

Not working on this version: git-Paper-689 (MC: 1.9.2)
I don't know if this is the latest update or not but it'd be cool if it was fixed for any version.

Comment by FatherWh0

Ran this test with a freshly compiled spigot build and cb build #33.

Comment by me4502

@FatherWh0
That looks perfectly fine to me. Do you have any plugins that modify items? Such as those 'lag remover' plugins? It could be one of those, they usually do more harm than good.

Comment by FatherWh0

I removed all plugins except essentials, craftbook, pex, worldedit and worlguard and the problem still existed. After extensive testing I found that restoring the vanilla settings in mechanisms.yml made the problem disappear. The results I got were not completely consistent but I found setting xp-per-bottle: 7 reproduced the problem.
note I do use Clearlagg but removed it for the test. It's mostly disabled anyway. I just use it to report where all the chickens are.

Comment by minoneer

@me4502 Just tested again with a clean and fresh setup, latest version of everything involved:

Spigot: git-Spigot-e6f93f4-935f18b (MC: 1.9.2)
CraftBook: 3.9-SNAPSHOT Build 35
WorldEdit: 6.1.1-SNAPSHOT;3482-867e59b
EssentialsX: Essentials version 2.0.1-b342

No config files touched except enabling XPStorer, debug and the debug flag.

Test steps:

1. set XP to 2047
2. rightclick spawner
   => Works as expected and results in 127 Stacks XP-Bottles
3. Clear inventory
4. set XP to 2048
5. rightclick spawner
   => Only one item appears ingame, which cannot be picked up once on floor. Stack size of -128
6. Clear inventory
7. set XP to 3000
8. rightclick spawner
   => Only on item appears ingame, which cannot be picked up once on floor. Stack size of -69

Debug log: http://pastebin.com/egBUrdPF

As far as I can tell, the debug log looks fine, so maybe the problem lays somewhere beyond that point? A limit on how many items can be dropped at once? (just a guess, might be elsewhere) It very much looks like a byte overflow issue to me.

Comment by me4502

Anything beyond that point is not handled by CraftBook, however.

Comment by minoneer

@me4502 It turns out that it is not possible to drop ItemStacks with a stack size > 127. Anything beyond that causes an overflow (confirmed with a simple test plugin).

I suggest to split the dropped stacks to a max stack size of 64 and drop them individually.

Regards, minoneer

Comment by me4502

@minoneer CraftBook already does that, and has since I was first reported the issue.

Comment by minoneer

@me4502 Well, not really. Decompiling build #35 results in the following:

    CraftBookPlugin.logDebugMessage("Bottles: " + bottleCount, "xpstorer");
    
    event.getPlayer().getInventory().removeItem(new ItemStack[] { new ItemStack(Material.GLASS_BOTTLE, bottleCount) });
    if (event.getClickedBlock() == null) {
      for (ItemStack leftOver : event.getPlayer().getInventory().addItem(new ItemStack[] { new ItemStack(Material.EXP_BOTTLE, bottleCount) }).values()) {
        event.getPlayer().getWorld().dropItemNaturally(event.getPlayer().getLocation(), leftOver);
      }
    } else {
      event.getClickedBlock().getWorld().dropItemNaturally(event.getClickedBlock().getLocation(), new ItemStack(Material.EXP_BOTTLE, bottleCount));
    }
    event.getPlayer().setLevel(0);
    event.getPlayer().setExp(0.0F);

Which clearly doesn't split the stack when dropped.

Git agrees with this: The revision for build #35 is a0fc7fc, which has the same code: https://github.com/sk89q/CraftBook/blob/a0fc7fc0e047cdd345471488269f04f71fa4072c/src/main/java/com/sk89q/craftbook/mechanics/XPStorer.java#L94

Strangely, when looking at branch 3.x, the fix is there, although it is only one commit ahead. I'm not sure how that can be, but the fix clearly didn't make it into the compiled code.

Comment by me4502

Strange... I'll have a look into it

Edit:
Yep. Wow. Turns out that the commit didn't push properly, or it may have been overwritten by something else. But basically, I did a few rebases and merges to be get the latest master, and that appears to have pushed the commit properly. So in my testing its been working, and the code I've been looking at is fine. However, the compiled code didn't contain the commit.

Comment by minoneer

@me4502 Confirmed - everything works again as it should. Thanks for your effort!

Comment by minoneer

@me4502 Hmmm, apparently now XP is not substracted any more, see http://dev.enginehub.org/youtrack/issue/CRAFTBOOK-3183