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

Is indexing in items.json and blocks.json a good thing ? #14

Closed
rom1504 opened this issue Apr 21, 2015 · 10 comments
Closed

Is indexing in items.json and blocks.json a good thing ? #14

rom1504 opened this issue Apr 21, 2015 · 10 comments

Comments

@rom1504
Copy link
Member

rom1504 commented Apr 21, 2015

Currently blocks and items are indexed by id in items.json and blocks.json.
Is that really a good idea ? As seen in #7 users sometimes need to index by name or by metadata+id.

It should probably not be the responsibility of minecraft-data to choose that indexing, but the responsibility of the reader/user.

Not doing that indexing would simply mean using json arrays instead of json objects.

@roblabla
Copy link
Member

👍 to removing the indexing, the data should be as natural as possible. Building an index table isn't hard to do in most languages anyway ^^.

@Gjum
Copy link
Member

Gjum commented Apr 21, 2015

Not opposed, as it often depends on how the data are used and should be done by each application.

@rom1504
Copy link
Member Author

rom1504 commented Apr 24, 2015

So... not sure that's so easy. That means changing everywhere in mineflayer that use that indexing, also probably everywhere in stuff using mineflayer.

@roblabla
Copy link
Member

@rom1504 we just need to create a js that exposes the indexed minecraft_data, and use that in the files instead of minecraft_data, don't we ? I don't think there would be a lot of code-level changes.

@rom1504
Copy link
Member Author

rom1504 commented Apr 24, 2015

@roblabla yeah sure, it actually already kind of exist there https://github.com/andrewrk/mineflayer/blob/master/lib/item_index.js

Problem is it also exists here https://github.com/Darthfett/helperbot/blob/master/lib/items.js#L14
and there https://github.com/rom1504/rbot/blob/5802c9806077be85974c830426aa387dc1d58257/task.js#L27

Also in lot of places in mineflayer (there for example https://github.com/andrewrk/mineflayer/blob/7890055e1046098487d8ce6ac5ded811813ea7b9/lib/block.js#L14)

So : yes we should provide a wrapper in mineflayer instead of providing the raw data, but that means changing stuff in lot of places.

Not sure this https://github.com/andrewrk/mineflayer/blob/master/doc/api.md#enums should still be in mineflayer API.

@rom1504
Copy link
Member Author

rom1504 commented Apr 27, 2015

I'm using the diff to check whether the data I'm extracting looks correct or not, so I'll do this change only when #8 is done.

@rom1504
Copy link
Member Author

rom1504 commented May 2, 2015

I'm not entirely sure we should get rid of indexing in the end.
The fact that it makes the diff easier to read is really useful (see 2655321 for example)
It might be possible to define a standard way to sort the blocks/items/recipes enums so that the diff is still useful but that would mean adding that sorting to all extractors when stringify currently does that sorting automatically.

@rom1504
Copy link
Member Author

rom1504 commented May 3, 2015

If we keep the indexing, maybe also index variations by metadata.

@Gjum
Copy link
Member

Gjum commented May 3, 2015

When we start indexing everything by ID and metadata, we also should do it with recipes.json. Although there it would also involve drastic changes in the format because of multiple recipes per item (same ID and metadata).

@rom1504
Copy link
Member Author

rom1504 commented May 3, 2015

Well if we want to index recipes by id+metadata we could just index by something like "1:1" for stone with metadata=1.

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

No branches or pull requests

3 participants