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.

Sign up for GitHub

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

Jump to bottom

Add to Buffers to DataSlots #279

Merged
merged 13 commits into from Aug 3, 2023
Merged

Add to Buffers to DataSlots #279

merged 13 commits into from Aug 3, 2023

Conversation

ferriarnus
Copy link
Member

@ferriarnus ferriarnus commented Jul 10, 2023 •
edited

Description

Add to/from buffer methods to the dataslots to allow for more efficient networking by not using NBT.

Fixes issue(s): # (issue)

Todo

  • Add to/from buffer methods
  • Replace custom packet with buffers instead of NBT
  • Check multiplayer (Initial seems fine, but a need a second opinion)

Checklist:

  • My code follows the style guidelines of this project (.editorconfig, most IDEs will use this for you)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation

@Rover656 Rover656 added Type-Enhancement New feature or enhancement to existing feature. Area-Backend Backend work not usually visible to players. MC-1.20.1 labels Jul 10, 2023
Rover656
Rover656 requested changes Jul 10, 2023
View reviewed changes
Copy link
Member

@Rover656 Rover656 left a comment

Choose a reason for hiding this comment

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

Looking good so far, just a couple of tweaks to what you've got from me!

src/core/java/com/enderio/core/common/network/slot/FluidStackNetworkDataSlot.java Outdated Show resolved Hide resolved
src/machines/java/com/enderio/machines/common/blockentity/sync/LargeMachineEnergyDataSlot.java Outdated Show resolved Hide resolved
src/machines/java/com/enderio/machines/common/blockentity/sync/LargeMachineEnergyDataSlot.java Outdated Show resolved Hide resolved
@ferriarnus ferriarnus marked this pull request as ready for review July 10, 2023 23:03
@ferriarnus ferriarnus requested a review from Rover656 July 13, 2023 08:44
Rover656
Rover656 previously requested changes Jul 13, 2023
View reviewed changes
Copy link
Member

@Rover656 Rover656 left a comment

Choose a reason for hiding this comment

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

A handful more comments for you; will need to review properly when I can run it

src/core/java/com/enderio/core/common/blockentity/EnderBlockEntity.java Outdated Show resolved Hide resolved
src/core/java/com/enderio/core/common/network/C2SDataSlotChange.java Show resolved Hide resolved
@Rover656 Rover656 added this to the "More Features" (v6.1) milestone Jul 24, 2023
@Rover656 Rover656 self-requested a review July 29, 2023 21:46
@Rover656 Rover656 self-assigned this Jul 29, 2023
agnor99
agnor99 previously approved these changes Aug 3, 2023
View reviewed changes
Copy link
Contributor

@agnor99 agnor99 left a comment

Choose a reason for hiding this comment

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

could you fix the merge conflicts, after that this can be merged

@agnor99
Copy link
Contributor

agnor99 commented Aug 3, 2023

oh, and remove the unused import in S2CDataSlotUpdate

@agnor99 agnor99 dismissed Rover656’s stale review August 3, 2023 22:47

it's rovers fault that we can't merge this c:, no but like I'm taking over this review and after this I'm going to take over the world, muhhahaha

@agnor99 agnor99 merged commit 9b39f3e into Team-EnderIO:dev/1.20.1 Aug 3, 2023
3 checks passed
@ferriarnus ferriarnus deleted the networkbuffer branch August 16, 2023 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agnor99 agnor99 agnor99 left review comments

@Rover656 Rover656 Awaiting requested review from Rover656

Assignees

@Rover656 Rover656

Labels
Area-Backend Backend work not usually visible to players. MC-1.20.1 Type-Enhancement New feature or enhancement to existing feature.
Projects
None yet
Milestone
1.12.2 Parity Features
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ferriarnus @agnor99 @Rover656