Conversation
# Conflicts: # fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/impl/attachment/sync/AttachmentSync.java # fabric-data-attachment-api-v1/src/main/resources/fabric-data-attachment-api-v1.mixins.json
| public static final int MAX_PADDING_SIZE_IN_BYTES = AttachmentTargetInfo.MAX_SIZE_IN_BYTES + MAX_IDENTIFIER_SIZE; | ||
| public static final int DEFAULT_MAX_DATA_SIZE; | ||
| public static final int DEFAULT_ATTACHMENT_SYNC_PACKET_SIZE; | ||
| private static final Set<ServerGamePacketListenerImpl> playerConnections = Collections.newSetFromMap(new IdentityHashMap<>()); |
There was a problem hiding this comment.
Just taking a quick look atm and this doesnt seem ideal to me, why is this needed? Its likely going to cause issues when multiple servers are ran from the same process. (Something we do for client tests, and technically possible to have multiple servers in one process).
I think it would be best if this could use vanilla's existing per server player list?
There was a problem hiding this comment.
The reasoning is explained here
// We don't use PlayerLookup.all() because when a player respawns,
// there is a brief period where said player will not be in the server player list.
// If a global attachment is set then, the respawning player will never receive the update.
I thought of this given the recent similar problems with player respawning like in #5155, tho I had not considered multiple servers on the same JVM.
There was a problem hiding this comment.
I've changed it to get all player connections from MinecraftServer now.
2 participants
This PR adds global (server-wide) data attachments that are not tied to any specific
Level.Global attachments are stored in
MinecraftServerandClientPacketListeneron the server and client respectively. TheGlobalAttachmentstarget can be obtained viaLevel.globalAttachments()on either side and alsoMinecraftServer.globalAttachments()on the server for convenience.