Skip to content

Add common data API endpoint#21

Merged
nickelpro merged 1 commit intoSpockBotMC:masterfrom
sqozz:add_common
May 6, 2021
Merged

Add common data API endpoint#21
nickelpro merged 1 commit intoSpockBotMC:masterfrom
sqozz:add_common

Conversation

@sqozz
Copy link
Copy Markdown
Contributor

@sqozz sqozz commented May 6, 2021

With this change it is now possible to access the data inside the "commons" folder. By default it accesses the PC version but it fully supports PE too. It supports all files inside the common folder dynamically (With 2.84.0 comes a new file). I tried to closely follow the existing API but let me know if I should change something :)

Thanks for the project!

This fixes #20

@nickelpro
Copy link
Copy Markdown
Member

I'm fine with this approach, can you add a quick example to example.py to demonstrate the functionality?

@sqozz
Copy link
Copy Markdown
Contributor Author

sqozz commented May 6, 2021

Sure! I've added a really simple version with a basic for loop to match a protocol version to the human readable Minecraft version.

While writing the example I realized that the instantiated version of minecraft_data (e.g. mcd from the example) doesn't contain the common() method. I'm not sure if we want this behaviour. On one side, mcd.version contains almost the same information. On the other side "common" suggests it should be available on all instances of the class. So let me know what you think :)

@nickelpro
Copy link
Copy Markdown
Member

I recognized that it isn't part of the instantiated mcd object, which I assumed was intentional because "common" data doesn't belong to any single version. I debated whether it should be something like this:

import minecraft_data
mcd_common = minecraft_data("common")

or your current version

import minecraft_data
mcd_common = minecraft_data.common()

And decided your version is better, since common data is schematically distinct from the rest of the data.

The common data has enough useful stuff in it that it's worth including, and in any case this project is supposed to be a thin wrapper around those JSON files so excluding some isn't keeping with that goal.

I'm merging this, it will be pushed to PyPI in a few hours along with updating the Minecraft Data submodule. Thanks for the contribution.

@nickelpro nickelpro merged commit a7f1cf8 into SpockBotMC:master May 6, 2021
@sqozz
Copy link
Copy Markdown
Contributor Author

sqozz commented May 6, 2021

Thank you too for maintaining this and your quick and helpful response.

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.

Make data in "common" accessible

2 participants