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

StructureBlockUpdatePacket: Added encode/decode #3148

Open
wants to merge 6 commits into
base: stable
from

Conversation

@yoraze
Copy link
Contributor

yoraze commented Oct 15, 2019

Introduction

Added encode decode for StructureBlockUpdatePacket, which can be used for StructureBlockActor!

The player sends it to the server when changing the settings of StructureBlockActor.

Changes

API changes

Without API changes

Behavioural changes

Without behavioural changes

Tests

I don’t think that he is really needed here. However, I will try later to add StructureBlockActor and using the packets I will test it myself.

TODO: Find constants for StructureBlockType
@yoraze yoraze requested a review from dktapps Oct 16, 2019
@dktapps

This comment has been minimized.

Copy link
Member

dktapps commented Oct 16, 2019

It would be nice if someone could peer-review this and confirm the changes.

@yoraze

This comment has been minimized.

Copy link
Contributor Author

yoraze commented Oct 16, 2019

It would be nice if someone could peer-review this and confirm the changes.

I have no acquaintances who could confirm this. Usually I check that others write about the protocol, and for this reason they often started contacting me for advice. :^/

@yoraze yoraze requested a review from dktapps Oct 16, 2019
@yoraze

This comment has been minimized.

Copy link
Contributor Author

yoraze commented Oct 22, 2019

I don’t know if you are interested or not, but I found similar information about this packet from Sandertv.
(https://github.com/Sandertv/gophertunnel/blob/master/minecraft/protocol/packet/structure_block_update.go)

Maybe ask him to confirm or deny this PR?
I just think you know each other and he could test this PR.

Copy link
Contributor

Sandertv left a comment

I've had a look over it and it looks good. We had a talk about it on Discord.

yoraze added 2 commits Dec 10, 2019
@dktapps

This comment has been minimized.

Copy link
Member

dktapps commented Mar 9, 2020

This should have been targeted at stable.
We really want to keep the protocol as consistent as possible between the branches, and since PM is a reference implementation, it's confusing to people if our implementation isn't available on the default branch.

This also needs to be rechecked against Minecraft 1.14 to ensure that it's still in line with latest changes.

@yoraze

This comment has been minimized.

Copy link
Contributor Author

yoraze commented Mar 9, 2020

This also needs to be rechecked against Minecraft 1.14 to ensure that it's still in line with latest changes.

Okay, I rechecked the read/write methods of StructureBlockUpdatePacket in BDS 1.14.30.2 and made sure that nothing has changed there. I also rechecked serialize/unserialize methods of StructureEditorData and StructureSettings - there are no changes either.

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

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.