Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add enchantments from wiki #287

Closed
wants to merge 3 commits into from
Closed

Conversation

uncovery
Copy link
Contributor

generated with this code PrismarineJS/minecraft-wiki-extractor#27

@uncovery
Copy link
Contributor Author

uncovery commented May 20, 2020

Seems there is a required format for enchantments that I was not aware of.
Seems to be defined here: https://github.com/PrismarineJS/minecraft-data/blob/master/schemas/enchantments_schema.json

Since we have more information than that, can we change the schema instead of limiting the data availability? What is depending on that schema?

@rom1504
Copy link
Member

rom1504 commented May 27, 2020

Mineflayer is using that schema for example. If you can add property without changing current properties it's good.

@uncovery
Copy link
Contributor Author

uncovery commented May 28, 2020

Ok, sounds reasonable. Question:

how can we make sure the unique integer ID is consistent? This ID has been deprecated and now it's the name string (e.g. minecraft:efficiency). While we can take the old ID for the old enchantments, new enchantments (for crossbows, tridents etc) do not have an ID anymore and any assignment would be completely arbitrary. We can of course just assign one in a central place, but I would prefer & suggest that we use the enchantment name as ID since it's already unique. Otherwise we have to take it out since it's supposed to be unique and we cannot have 2 entries with a 0 in there. My commit below will work in any case, I can just fill in a sequential number. But that one will then likely change with the next MC release.

"id": {
        "description": "The unique identifier for an enchantment",
        "type": "integer",
        "minimum": 0
      },

I am adding a pull request below for a schema proposal, please help me verify this

I am not 100% sure if the format for primaries, secondaries and exclusions is correct. Please help me verify that this would be satisfactory to work with https://github.com/uncovery/minecraft-data/blob/master/data/pc/1.15.2/enchantments.json
we originally wrongly copied the syntax minecraft.ench from the language file, but MC is using minecraft:ench instead. same for items.
Further, I renamed the main index "name" from "itemName" since that is the way the current schema does it.
@rom1504
Copy link
Member

rom1504 commented May 28, 2020

I can't find any place in prismarinejs where we use the enchantments.
Do you have an use case in mind ?
What packets would this be used with for example ?

@Karang
Copy link
Contributor

Karang commented May 28, 2020

I can't find any place in prismarinejs where we use the enchantments.
Do you have an use case in mind ?
What packets would this be used with for example ?

It would be useful to have that in mineflayer because enchantments can affect some properties:

  • dig time is affected by the tool enchantment
  • fall damages is affected by Protection and Feather falling enchantments

Also having this data could be useful to make the bot work with enchantment tables ?

Eventually enchantments would be implemented in flying-squid ?

@rom1504
Copy link
Member

rom1504 commented May 28, 2020

Yeah, so same question then, if we want to use them in client/server, which packet is it ?
That'll make it possible to answer what data we need

@rom1504
Copy link
Member

rom1504 commented May 28, 2020

Ok so the answer is for example https://wiki.vg/Protocol#Window_Property

As you can see the numerical IDs are still used and not deprecated at all.

So we do need them.

The wikis removing them means end users may not need them, so I guess you'd need to find these IDs elsewhere. (The jar I guess ? Have a look at burger extractor maybe)

@uncovery
Copy link
Contributor Author

Ok so the answer is for example https://wiki.vg/Protocol#Window_Property

Ok, did not know that. Will take a look how to get them then.

@uncovery
Copy link
Contributor Author

uncovery commented May 29, 2020

Do you have an use case in mind ?

My use case for example is that I can now make sure that a system that dishes out random enchanted items as rewards does create any that violate the enchantment conflicts. Further it's used to display the transactions of my servers' in-game shop properly with enchantments on the servers website.

@uncovery
Copy link
Contributor Author

I can't find any place in prismarinejs where we use the enchantments.

I am a bit confused why this question is being asked. While I can understand that you guys want to have the enchantments be done properly (with the IDs and fitting here to the data schema etc, I would like to know what other prerequisites this needs - beyond just working. I have started doing this here instead of in my own systems mainly because there was already an older version doing exactly that, I am merely updating it.

If there is a requirement to have an active use in prsimarineJS or some such, then I'd rather drop the ball here and re-do it in my own system instead. I have fulfilled my own requirements on this functionality a week ago and everything further from now on is done 100% to satisfy other people's needs. In case there is no such need, please let me know ASAP so I can focus my time on more productive things.

I know I am poor at programming and sorry if my mistakes and shortcomings are creating additional work. If it's not even achieving anything once it's done, this does not make any sense at all. Sorry if this sounds like a rant, I do not mean to be guilt-tripping people into liking what I do. If you do not see a use case, just tell me and I'll move on without hard feelings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants