Skip to content

Instantly run task after write operation#1460

Merged
booky10 merged 8 commits intoretrooper:2.0from
Beaness:instantwriteaftersend
Apr 7, 2026
Merged

Instantly run task after write operation#1460
booky10 merged 8 commits intoretrooper:2.0from
Beaness:instantwriteaftersend

Conversation

@Beaness
Copy link
Copy Markdown
Contributor

@Beaness Beaness commented Feb 17, 2026

Right now it's possible other packets get written before the promise is completed, If you want to write a packet right after another packet using getTasksAfterSend it's possible another packet gets written inbetween. This solves it for spigot (sponge needs a different type of implementation, probably overwriting the whole write function to execute the tasks).

@retrooper retrooper self-assigned this Feb 27, 2026
@retrooper retrooper added bug Something isn't working enhancement New feature or request labels Feb 27, 2026
@retrooper retrooper requested a review from booky10 April 7, 2026 01:52
@booky10 booky10 assigned booky10 and unassigned retrooper Apr 7, 2026
booky10 added 5 commits April 7, 2026 04:46
This was used previously while we still relied on netty's built-in MessageToMessageEncoder, which didn't allow to have proper "tasks-after-send"
We really need to refactor these copy-pasted handlers into a common module...
Why does this list of runnables only exist for clientbound packets? Doesn't make sense to me
Copy link
Copy Markdown
Collaborator

@booky10 booky10 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix, I also applied your changes to sponge and fabric
This promise-based handling is because our spigot/paper outbound handler used to be a MessageToMessageEncoder and never was removed when we stopped using that

Sorry for the long wait!

@booky10 booky10 merged commit 97b09b0 into retrooper:2.0 Apr 7, 2026
2 checks passed
@retrooper
Copy link
Copy Markdown
Owner

Thank you for the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants