Implemented functional anvils #521
Conversation
# Conflicts: # server/session/handler_item_stack_request.go # server/session/player.go # server/session/session.go # server/session/world.go
# Conflicts: # cmd/blockhash/main.go # server/block/wood_fence_gate.go
|
This is missing updated repair costs on result items and documentation, but aside from that, this is pretty much ready for review. |
…mpatible enchantments.
There was a problem hiding this comment.
Thanks a lot for the PR and apologies for taking so long to review.
I've got a couple of comments, particularly on the non-logic related stuff. The actual anvil logic is really quite difficult to read, so (as mentioned in one of the comments) I'd really recommend adding comments throughout the code explaining what's happening.
Also: I introduced a merge conflict with a change I made just now, shouldn't be very difficult to fix though.
# Conflicts: # server/block/coal.go
# Conflicts: # server/item/enchantment/aqua_affinity.go # server/item/enchantment/efficiency.go # server/item/enchantment/feather_falling.go # server/item/enchantment/fire_aspect.go # server/item/enchantment/knock_back.go # server/item/enchantment/protection.go # server/item/enchantment/sharpness.go # server/item/enchantment/silk_touch.go # server/item/enchantment/unbreaking.go
Co-authored-by: DaPigGuy <mcpepig123@gmail.com>
Co-authored-by: DaPigGuy <mcpepig123@gmail.com>
There was a problem hiding this comment.
With the comments and a couple of functions that were extracted from the original code, this looks a ton more readable. Still pretty long functions but acceptable, I think.
You guys have a lot better of an idea about the anvil functionality itself. I wasn't able to find any obvious logical flaws, but all I can say is make sure that the behaviour matches bedrock edition. If you think it does and is ready, please feel free to merge this on your own accord.
This reverts commit d38ba17.
|
Definitely more testing required in survival mode. Already found an issue where client/server do not agree on cost when combining two Sharpness 4 books. EDIT: Cost calculations for enchantment merging seem to differ to some extent between Java & Bedrock, particularly when both items share an enchantment of the same type. The operation on dragonfly/Java Edition costs 5 levels vs. Bedrock Edition's 1 level. |
Closes #500.