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

Update/fix ChunkWatchEvent to match their docs, and clean docs a little #9790

Merged
merged 2 commits into from Nov 17, 2023

Conversation

DBotThePony
Copy link
Contributor

ChunkWatchEvent patch didn't get proper port from 1.20.1 to 1.20.2, where Mojang added chunk send queue as part of their networking bandwidth optimization. With this fix, ChunkWatchEvent.Watch and ChunkWatchEvent.UnWatch will match their behavior from pre 1.20.2 (you will be able to send chunk-specific data inside ChunkWatchEvent.Watch)

@autoforge autoforge bot added Triage This request requires the active attention of the Triage Team. Requires labelling or reviews. 1.20 labels Nov 17, 2023
@autoforge autoforge bot requested a review from a team November 17, 2023 07:13
@LexManos
Copy link
Member

So, as discussed on discord, this event needs to be re-looked at and determine if it needs to be split into more specific events. Because technically the Watch event is in the correct place, it is when the server knows the client needs to know about the chunk. But a lot of people use it to send data to the client which now hasn't received the chunk data yet. Which is why you've moved it.

The main problem is the now disjointed behavior between a 'watched' chunk and a 'has already synced' chunk.
A protentional solution is a 3rd ChunkSyncedToClient event or something that is explicitly for synchronizing data.
But this is fine for now. As it restore the logical behavior of older versions.

Your unwatch event however is essentially in the same place the only functional difference is that it will now not be fired unless the chunk has been synced to the client. Not exactly sure what people use the unwatch event for and if those usecases care about the client knowing about the data. But, until the reevaluation preventing seemingly 'unwatched before watched' events is a good idea.

@LexManos LexManos merged commit a5ff932 into MinecraftForge:1.20.x Nov 17, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.20 Triage This request requires the active attention of the Triage Team. Requires labelling or reviews.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants