Experience bottle duplication with Xp Storer (with explanation of the cause)

[LadyCailinBot]

CRAFTBOOK-2260 - Reported by Mysteryem

(I couldn't change the project field to craftbook "Only developers can change that field, sorry.". So, please can someone change it if they can)

Having a look at the code for the Xp Storer, player.getTotalExperience is being used, which returns a cumulative total of all the Xp a player has gained in their life so far. Enchanting, repairing or renaming items does not decrease its value.

Note: While neither of player.setLevel and player.setExp actually touch the the player's XpTotal (value returned by player.getTotalExperience), they are the correct methods to set a player's experience points. (whether they set it correctly in the code for the Xp Storer, I don't know)

In summary, the plugin is using the XpTotal nbt tag in its calculation of how much exp to give the player. This is the cause for the duplication of Xp. I suggest the essentials plugin be looked at for how it handles its /exp commands (see [Commandexp.java|https://github.com/essentials/Essentials/blob/master/Essentials/src/net/ess3/commands/Commandexp.java] and [SetExpFix.java|https://github.com/essentials/Essentials/blob/master/Essentials/src/net/ess3/craftbukkit/SetExpFix.java]). Alternatively, the minecraft experience calculator at http://www.radthorne.info/exp.php appears to work just as well (though it is written in javascript).

If you want to test the duplication, you can spawn in for example, 10 bottles of enchanting, use them all, use the Xp Storer (note, for the first few times you will get less bottles back than you put in), use the bottles, use the Xp storer. Then just repeat using the bottles you get and then using the Xp Storer, voila, unlimited experience.

A few tests I did to determine this (before looking at the code), and their erroneous outcomes: (I did these tests with an NBT editing program. I used [nbtexplorer|http://www.minecraftforum.net/topic/840677-nbtexplorer-nbt-editor-for-windows-and-mac/]).

I have 3 ways to show the current issues: 1) On your server, edit your playername.dat in worldname/players/. Set XpLevel to 1, XpP to 0 and XpTotal to a fairly large number, e.g. 700. (XpLevel is 1 because the Xp Storage block won't activate unless your XpLevel>0) 2) Join the server and use the Xp Storer. 3) You now have a load more bottles of enchanting than you should have. (23.33 if 30 experience=1 bottle) 4) Clear your inventory and disconnect.

A) Edit your playername.dat, this time, set XpLevel to 30, XpP to 0 and XpTotal to 0. B) Join the server and use the Xp Storer. C) You won't be given any bottles of enchanting, as far as the Xp Storer is aware, you have no xp. D) If you feel like it, go and use your levels to enchant, repair or rename something. E) Clear your inventory and disconnect.

a) Edit your playername.dat, this time, set XpLevel to 30, XpP to 0 and XpTotal to 60. b) Join the server and use the Xp Storer. c) Feel ripped off as you receive 2 bottles of enchanting from your 30 levels :). d) Clear your inventory and disconnect.

[LadyCailinBot]

Comment by jb_aero

So in other-words, if you need to use getTotalExperience, you should also be using setTotalExperience.

btw +10 for research!

[LadyCailinBot]

Comment by Mysteryem

No, you shouldn't be using setTotalExperience either. When enchanting, repairing or renaming items, XpTotal is not reduced (a plugin with a similar goal to the Xp storer was doing this and it resulted in free enchanting basically). By normal gameplay, XpTotal should only decrease on death. It's used as a component in a player's score when they die. If you had a plugin which used the 'score' value in some sort of highscores board, you could use setTotalExperience to give users additional 'score' for completing certain actions in their life, without increasing their current experience.

Edit: Honestly, minecraft has a pretty horrible way of handling Xp gains and deductions. It would be so much easier if there was a method along the lines of getCurrentExperience to get the player's current Xp equivalent, setCurrentExperience to set the player's level and percentage of the Xp remaining for that level and addCurrentExperience to add or subtract a value of Xp from the player's level and percentage remaining.

[LadyCailinBot]

Comment by me4502

The issue is, getExp() actually obtains the percentage of the way to the next level... so we are kinda stuck with the xp.

[LadyCailinBot]

Comment by me4502

Fixed, needs verification.

[LadyCailinBot]

Comment by MUDcraft

Checked with interesting results with build 1320-57b512e.

I started with 80 levels, used the XP storer and got 54 levels. Used these and then the XP storer again and got 37 levels... Wash, rinse, repeat... 23 10 3 1

at this point one bottle dropped from the XP storer that I couldn't pick up, and one dropped that could be picked up. A single bottle.

When I used this it remained in my inventory until I had accumulated 25 or so levels of experience, then at some point the bottle displayed a number over the single bottle displayed in my hotbar, 128, so a stack of 128 bottles.

I used those until there was only a single bottle showing again and the single bottle then continued to be usable to level 46 when the single bottle rolled over again to 128.

I stopped when I reached 90 levels or experience with no sign that it was going to stop.

Still broken, I think.