search

Laarryy / AdvancedEnchantments

Better Enchantments & Curses
MIT License
4 stars 7 forks source link

Enchanting sometimes takes 30 levels instead of 3 #30

Closed I-Knight-I closed 4 years ago

I-Knight-I commented 4 years ago

Plugin version: 4.20.26 Server type: Paper #103 Server version: 1.15.2

Describe the bug As outlined in issue #28 and #26, sometimes enchanting an item takes 30 levels from a player rather than just 3. When this happens, no sound effects are played either.

Expected behavior Minecraft has not taken 30 levels from a player during enchanting since 1.9 and the introduction of lapis as a key ingredient during the enchanting process.

To Reproduce

  1. Install the plugin
  2. Start the server
  3. Enchant repeatedly until 30 levels are taken

Forgive my intrusiveness but I was forced to fix this myself by editing the code. Players losing 30 levels was frequently becoming an issue that I had to resolve manually. In EnchantingTableEvents, the code that removes the player's levels can easily be simplified to:

int levelCost = event.whichButton(); int newLevel = event.getEnchanter().getLevel() - levelCost; if (newLevel < 0) { newLevel = 0; }

The button that is clicked is either button 1, button 2 or button 3 - corresponding to the power of the enchants. Each button always removes either 1 - 3 levels, again depending on the power of the enchants. Whilst this doesn't fix the fact that the bug occurs (the sound effects still don't play in the case of it happening, proving it's still there), it does fix the more serious problem of a player losing 30 levels since it now will only ever take a maximum of 3.

This has been an issue since at least 4.19.24.

egg82 commented 4 years ago

My main issue with that fix is that it feels hacky, and I'm not sure why it wouldn't be correct in the first place. I'll continue to see what I can find, though. Thank you!

I-Knight-I commented 4 years ago

I don't disagree but it's also entirely reasonable and is the exact methods provided by the onEnchant event. It's still way better than the alternative that is players losing 30 levels though.

Agreed it's frustrating and also confusing on why it's even doing it in the first place though. As I mentioned, Minecraft shouldn't ever take 30 levels at all anymore. There shouldn't be a single piece of code that causes that to happen.

If nothing else, it'd be appreciated if you implemented a temporary, admittedly hacky, solution.