Fixed the enchantment table "ready" event, which has been broken for years.

[zisis912]

What used to happen was that the "ready" event would get emitted when all the level values were above 0, however 0 means that a packet hasnt been sent yet

[pokahs]

very cool yeah getting enchantment name by string is just way more convenient also nice fix :p

[u9g]

Please seperate this pr into two seperate prs, since it has two distinctly different changes.

[zisis912]

One thing I am worried about is that the packets for level could be sent before the packets for expected.enchant, resulting in something like

[
  { level: 5, expected: { enchant: 'unbreaking', level: -1 } },
  { level: 11, expected: { enchant: null, level: -1 } },
  { level: 30, expected: { enchant: null, level: -1 } }
]

Does anyone know the order with which the packets get sent? Maybe it would be smarter to make the "ready" event reliant on expected.level, depending on the order of the packets

[zisis912]

Please seperate this pr into two seperate prs, since it has two distinctly different changes.

i dont know how to do that 😭 I made a commit to my fork and it got auto-added to the pull request

I think it's okay if we keep this as a 2 in 1, just a general enchanting library fix and overhaul to modernise it.

[zisis912]

After some stress testing I saw that its possible for the level to update without the expected.level updating, so I made sure to change the condition to check for the latter.

[u9g]

Please seperate this pr into two seperate prs, since it has two distinctly different changes.

i dont know how to do that 😭 I made a commit to my fork and it got auto-added to the pull request

I think it's okay if we keep this as a 2 in 1, just a general enchanting library fix and overhaul to modernise it.

That's exactly the problem, the "overhaul" is a breaking change to anyone who uses this now.

[zisis912]

Please seperate this pr into two seperate prs, since it has two distinctly different changes.

i dont know how to do that 😭 I made a commit to my fork and it got auto-added to the pull request

I think it's okay if we keep this as a 2 in 1, just a general enchanting library fix and overhaul to modernise it.

That's exactly the problem, the "overhaul" is a breaking change to anyone who uses this now.

JavaScript is a language that requires one to actively update their code to the newest documentation, atleast as far as web development goes. I don't think this would go against that principle.

Also I would like to mention that there is not a single person that used the enchanting library, since it was broken this whole time. Think of this as a new feature getting added, instead of an old one being modified. Enchanting did not really exist in a usable form up to this point.

And the few people that did use it are certainly smart and involved enough to know where the issue lies

[rom1504]

1.8.8 enchanting test is failing can you please fix it ?