Add bedrock protocol data #445
Conversation
data/dataPaths.json
Outdated
| }, | ||
| "1.16.220": { | ||
| "blockStates": "bedrock/1.16.220", | ||
| "steve": "bedrock/1.16.201", | ||
| "steveSkin": "bedrock/1.16.201", |
There was a problem hiding this comment.
should we really have this file in mcdata ? what is it used for ?
if we want to have it, then we need to change node-minecraft-data to support non json files
There was a problem hiding this comment.
So these are skin files used by the bedrock-protocol client when sending the login packet https://github.com/PrismarineJS/bedrock-protocol/blob/master/src/handshake/login.js#L10. So I think it would be possible to put it all into one big JSON with base64 if necessary
There was a problem hiding this comment.
yeah that would be more convenient
There was a problem hiding this comment.
Done, tests are failing now with the removal but should be fixed on node-minecraft-data release
| @@ -0,0 +1,3171 @@ | |||
| # Created from MiNET and gophertunnel docs | |||
There was a problem hiding this comment.
why do we want this to be only in a latest folder ?
isn't documentation for older versions useful too ?
There was a problem hiding this comment.
what do you think about this @extremeheat ?
There was a problem hiding this comment.
On Bedrock you can’t really downgrade aside from manually installing APKs or things like that.
So how about whenever a new version comes out we copy the YML then put a copy of it (the old version) into the appropriate version folder? That way the docs/generated HTML for the old version can still be updated later on if needed. Also that way the latest file is still easy to diff/git blame and see where the protocol changes happen over time.
Otherwise if that’s an issue we can do the copy thing as usual
There was a problem hiding this comment.
what's the difference between what you are proposing and the usual copy thing ?
the latest yaml file would be in the latest version we support
I think we need to decide to support multi version or not. If we don't then we should only have one latest dir and that's it.
I think there is value to support multi version, and if we do so, I don't see why not following the same structure as the rest of mcdata.
One argument to support multi version is to allow for a smoother transition period when updating libs to work for the new versions (by just adding support for the new version instead of breaking changes)
There was a problem hiding this comment.
Oh yeah, I’m not arguing against multi version support. A lot of 3rd party things work on older versions and somethings don’t update so I think it’s important to support old versions. It’s just a matter of deciding whether to make it first class support or not, which I think it already is in a way (we do tests and all).
Anyway, the benefit I see with having a linear “latest” file is that it’s just easier to track changes to the protocol, for example when new versions come out in PR diffs. So to see where a packet or field changed in the past, it’s simple to run git blame on it. What I was mentioning was just a compromise of keeping that history like a wiki and also being able to update docs for old versions by keeping a copy of the YAML for the old versions. But if this is a maintenance burden we can do the usual duplicate + write on top approach
There was a problem hiding this comment.
ah I see your point about the git tracking.
Yeah that's not a bad idea, but then I would just like to avoid the situation where latest.yml is different from $latest/protocol.yml
So let's add a test that check the file are exactly the same and I'm ok with that solution:
- a latest dir with proto.yml and types.yml
- copy of these files in all versions
- proto.yml and types.yml of latest folder should be exactly equal (exact diff) with the types.yml and latest.yml of the latest version
if that works well, we could even consider doing the same for mcpc
There was a problem hiding this comment.
I think the having/maintaining a duplicate copy can just be avoided, for example when 1.18 comes out, the current latest files (1.17) are only then copied to the 1.17 dir. Then the 1.18 stuff is written ontop of latest. So doing it that way would avoid having to manage two copies of the same file.
There was a problem hiding this comment.
I think that's likely to make it confusing for users.
we've been doing the no latest thing since the beginning for mcpc and it worked ok
do you expect there will be many more versions for bedrock than mcpc ?
There was a problem hiding this comment.
Ok let's try your way then:
- We have a latest.yml and type.yml for the latest
- Copied for all other versions
- Data path points to it to expose it
Does that sound ok to you ?
| "steve": "bedrock/1.16.201", | ||
| "steveGeometry": "bedrock/1.16.201" | ||
| "blocksB2J": "bedrock/1.17.10", |
There was a problem hiding this comment.
Imo we should also expose the yml files here.
data/dataPaths.json
Outdated
| "steve": "bedrock/1.16.201", | ||
| "steveGeometry": "bedrock/1.16.201" | ||
| "protoDoc": "bedrock/1.17.10", | ||
| "protocolYml": "bedrock/latest", |
There was a problem hiding this comment.
Was fixed in another commit
|
I'm thinking it could be a good idea to add yml support to node-minecraft-data before merging this. |
|
yeah, opened PrismarineJS/node-minecraft-data#110 which load non-JSON files as a URL. So the handling can be moved to other code. |
|
is this ready ? looks ok to me |
Adds bedrock-protocol's protocol YML data