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 Advancements Data #583

Open
wants to merge 45 commits into
base: master
Choose a base branch
from
Open

Add Advancements Data #583

wants to merge 45 commits into from

Conversation

Lianecx
Copy link

@Lianecx Lianecx commented Jun 21, 2022

I've created an advancement data file for another project and noticed that this repo doesnt have data for advancements yet.

Eejit and I adjusted it to fit the style of other data files and we added lots of new informational properties.

  • Advancements
  • Advancement criteria
  • Older Versions
  • Added schema
  • Updated READMEs

@u9g
Copy link
Member

u9g commented Jun 21, 2022

How did you create it?

@Lianecx
Copy link
Author

Lianecx commented Jun 21, 2022

How did you create it?

At first, I just copied over each of the properties manually, but i realised I could've just used the advancements table of the fandom wiki and convert it to json using a tool.
For the criteria, Ill have to do them manually as they are not listed anywhere on the wiki.

@Lianecx
Copy link
Author

Lianecx commented Jun 21, 2022

Is there anything wrong with it? Because the CI test did fail 🤔

@Eejit43
Copy link
Contributor

Eejit43 commented Jun 21, 2022

Is there anything wrong with it? Because the CI test did fail 🤔

Only that it isn't included in dataPaths.json

@Lianecx
Copy link
Author

Lianecx commented Jun 21, 2022

Ah I see, sorry about that

@Lianecx
Copy link
Author

Lianecx commented Jun 21, 2022

I also need to create a schema right?

@Lianecx
Copy link
Author

Lianecx commented Jun 21, 2022

How did you create it?

At first, I just copied over each of the properties manually, but i realised I could've just used the advancements table of the fandom wiki and convert it to json using a tool. For the criteria, Ill have to do them manually as they are not listed anywhere on the wiki.

This could probably all be automated using the minecraft language file.

@Lianecx
Copy link
Author

Lianecx commented Jun 21, 2022

How did you create it?

At first, I just copied over each of the properties manually, but i realised I could've just used the advancements table of the fandom wiki and convert it to json using a tool. For the criteria, Ill have to do them manually as they are not listed anywhere on the wiki.

This could probably all be automated using the minecraft language file as it contains all the advancement descriptions, displaynames and IDs (resource locations).

@Eejit43
Copy link
Contributor

Eejit43 commented Jun 21, 2022

I'll work on making a tool to generate this automatically

@Lianecx
Copy link
Author

Lianecx commented Jun 21, 2022

I'll work on making a tool to generate this automatically

If you want, I can do it myself :D

@Lianecx
Copy link
Author

Lianecx commented Jun 21, 2022

Do you know what wrong in the bedrock CI test?

@Eejit43
Copy link
Contributor

Eejit43 commented Jun 21, 2022

Unsure about the CI test, but already working on that tool. I'll let you know when I make progress!

@Lianecx
Copy link
Author

Lianecx commented Jun 21, 2022

Unsure about the CI test, but already working on that tool. I'll let you know when I make progress!

Okay, sure then!

@Eejit43
Copy link
Contributor

Eejit43 commented Jun 21, 2022

Alright I've made a a tool to generate it. I'll create a separate PR and add that data there!

@Lianecx
Copy link
Author

Lianecx commented Jun 21, 2022

G

Alright I've made a a tool to generate it. I'll create a separate PR and add that data there!

Awesome!

@Eejit43 Eejit43 mentioned this pull request Jun 21, 2022
@Eejit43
Copy link
Contributor

Eejit43 commented Jun 21, 2022

See #584! Thanks for making this, that was a very good idea!

@u9g
Copy link
Member

u9g commented Jun 21, 2022

Why is this data helpful? Wouldn’t it be more helpful for you to just access the raw advancement json from a datapack?

@Lianecx
Copy link
Author

Lianecx commented Jun 21, 2022

Why is this data helpful? Wouldn’t it be more helpful for you to just access the raw advancement json from a datapack?

It's great to have every minecraft data combined in a single package. I'm not exactly sure about the raw advancement json youre talking about. Isnt every advancement stored in seperate files which makes everything very inconvenient?
Also, the displayName and description are in the language file only and not included in the raw json.

@Lianecx
Copy link
Author

Lianecx commented Jun 22, 2022

@Eejit43 Let me know if you want me to change the format to anything specific.

@Lianecx
Copy link
Author

Lianecx commented Jun 25, 2022

Should be ready to merge now

@Lianecx
Copy link
Author

Lianecx commented Jul 6, 2022

Is there still anything missing here?

@Eejit43
Copy link
Contributor

Eejit43 commented Jul 6, 2022

Is there still anything missing here?

nope we just need to wait for it to be reviewed.

@Lianecx
Copy link
Author

Lianecx commented Jul 6, 2022

Is there still anything missing here?

nope we just need to wait for it to be reviewed.

How long does that take usually?

@Eejit43
Copy link
Contributor

Eejit43 commented Jul 6, 2022

Is there still anything missing here?

nope we just need to wait for it to be reviewed.

How long does that take usually?

who knows; whenever they get around to it

@Eejit43
Copy link
Contributor

Eejit43 commented Jul 12, 2022

@Lianecx also you should update the title of this PR

@Lianecx Lianecx changed the title Add advancements data 1.19 Add Advancements Data Jul 13, 2022
| language.json | Yes | Use [minecraft-jar-extractor][10] |
| particles.json | Yes | Use [Burger][1], then use [burger-extractor][2] |
| blockLoot.json | Yes | Use [minecraft-jar-extractor][10] |
| entityLoot.json | Yes | Use [minecraft-jar-extractor][10] |
| mapIcons.json | No | Icons can be found in the Minecraft jar file where they are added as a single sprite. The file location is `/assets/minecraft/textures/map/map_icons.png`. Alternatively you might be able to look up the icons from the following page on the [Minecraft wiki][15] or from [wiki.vg][16]. | [minecraft-data pr mapIcons][14] |
| advancements.json | Yes | To be added |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you indicate here the method to add this file for new versions?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

@Eejit43
Copy link
Contributor

Eejit43 commented Jul 13, 2022

Created a PR to update this PR with that change (see here)

@Lianecx
Copy link
Author

Lianecx commented Jul 13, 2022

Created a PR to update this PR with that change (see here)

Can you make a pr in my fork to add this
Sorry i oversaw that

add advancement extractor link
@rom1504 rom1504 added this to Needs triage in PR Triage Aug 7, 2022
@rom1504
Copy link
Member

rom1504 commented Aug 7, 2022

still thinking whether this should be here or not

@u9g was thinking not because it doesn't come from the main extractor

let's think a bit more

@rom1504 rom1504 moved this from Needs triage to Waiting for user input in PR Triage Aug 7, 2022
@Eejit43
Copy link
Contributor

Eejit43 commented Oct 17, 2022

still thinking whether this should be here or not

@u9g was thinking not because it doesn't come from the main extractor

let's think a bit more

Any updates on this? It could be adapted into the main extractor

@rom1504
Copy link
Member

rom1504 commented Mar 4, 2023

not too eager to add this huge piece of data unless someone still finds it useful

is anyone using this ?

@rom1504 rom1504 moved this from Waiting for user input to Almost too old in PR Triage Mar 4, 2023
@Lianecx
Copy link
Author

Lianecx commented Mar 4, 2023

I would 100% use this

@rom1504
Copy link
Member

rom1504 commented Jul 29, 2023

ok @Lianecx can you open a corresponding PR in node-minecraft-data and then we merge this ?

@Lianecx
Copy link
Author

Lianecx commented Jul 29, 2023

Okay, Ill do that 👍
Feel free to remind me if i forget

@rom1504
Copy link
Member

rom1504 commented Dec 17, 2023

@Lianecx gentle reminder

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

Successfully merging this pull request may close these issues.

None yet

5 participants