Skip to content

Conversation

@DennisOchulor
Copy link
Contributor

This PR changes the att sync payload to be splittable and allows mods to set sync size limits per AttachementType.

  1. The att sync payload max size is set to Integer.MAX_VALUE. This might be too high but I set it to that as a start.
  2. Initial syncs still manually split packets if needed, just in case multiple really large attachments go over the max payload size, however unlikely that is.
  3. The new default sync limit is 1 MiB, up from just 32502 bytes.
  4. The encoded attachement data is now truncated to the actual readable number of bytes, but unfortunately this requires copying the array, so I'm not really sure if this is the way to go.

Relates to #4669

Copy link
Contributor

@Earthcomputer Earthcomputer left a comment

Choose a reason for hiding this comment

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

Increasing the max attachment size to Integer.MAX_VALUE is definitely not the right solution. The way I would personally solve it is like this:

  1. Make every attachment sync in a separate packet. I believe the netty pipeline batches packets automatically, so I don't think the original sync mechanism joining multiple updates into one packet really saved on much.
  2. When sending the packets, put them in a bundle to make sure they are all received on the same tick.
  3. Add an API to fabric networking to update the maximum payload size of an already-registered large packet.
  4. Increase the default max attachment size to the maximum size of the payload in a large packet before it has to split, minus whatever the maximum length of the ID is.
  5. Whenever a new attachment is registered with a max size bigger than the current max size for any registered attachment, update the max size of the sync packet.

@DennisOchulor
Copy link
Contributor Author

I assume number 3 would be something like PayloadTypeRegistry.modifyLargePayloadMaxSize(id,size)? Though depending on mod load order, mods registering large attachments may cause modifyLargePayloadMaxSize() to be called before the att sync packet is even registered.

@DennisOchulor
Copy link
Contributor Author

I've implemented @Earthcomputer's suggestions, the changes to the networking API are in a separate commit to help with reviewing. Though maybe a separate PR should be made?

@DennisOchulor DennisOchulor changed the title Add per-AttachmentType sync size limits Add per-AttachmentType sync size limits & Allow modifying large payload max size Nov 13, 2025
@maityyy maityyy added the enhancement New feature or request label Nov 25, 2025
@DennisOchulor
Copy link
Contributor Author

See #5119

The data attachment changes will be made in another PR after #5119 is merged.

@DennisOchulor DennisOchulor deleted the att-sync-large-packet branch January 15, 2026 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants