Skip to content

Comments

Implement pre via version injection support for spigot module#1446

Open
ytnoos wants to merge 2 commits intoretrooper:2.0from
ytnoos:pre-via
Open

Implement pre via version injection support for spigot module#1446
ytnoos wants to merge 2 commits intoretrooper:2.0from
ytnoos:pre-via

Conversation

@ytnoos
Copy link
Contributor

@ytnoos ytnoos commented Jan 25, 2026

Since Retrooper gave us the opportunity to possibly accept the pre-via patch, here we are. I merged my patch with the fixes [1] [2] made by @Axionize, whom I thank very much for contributing and making this possible. Currently, this code is used by both Grim and LPX, so it can be considered fairly safe.

By default packetevents should maintain the normal behavior. This patch works only for the spigot module and in order to use it PacketEventsSettings#preViaInjection(true) must be called. Once this is done, packetevents will inject twice into the handlers for both the decoder and the encoder, once before ViaVersion and once after.
By default, all listeners will keep the default behavior. To create a pre-via listener, simply override the method inside your listener like this:

public boolean isPreVia() {
        return true;
}

I apologize in advance for any errors and look forward to hearing from you if any changes or improvements to the code are needed.

@SpigotRCE
Copy link

Damn

@3add
Copy link

3add commented Jan 25, 2026

W

@florianreuth
Copy link

Pure ViaVersion evilness:tm:

@booky10
Copy link
Collaborator

booky10 commented Jan 25, 2026

Hm, I'm not quite sure whether it is a good idea to lock this behavior behind a setting - would it be possible to dynamically inject our handlers if someone wants to register a pre-via listener?

@ytnoos
Copy link
Contributor Author

ytnoos commented Jan 25, 2026

Hm, I'm not quite sure whether it is a good idea to lock this behavior behind a setting - would it be possible to dynamically inject our handlers if someone wants to register a pre-via listener?

Injection is mostly done during PacketEvents init, and since anyone can register listeners at any time, it would mean that in order to do it correctly, we would reinject server channel and all connected players the first time a pre-via listener is registered. I think it can be done but the possibility of creating new problems increases. Is it worth it?

@booky10
Copy link
Collaborator

booky10 commented Jan 25, 2026

The problem with settings is that they are shared globally across all plugins using packetevents (except when you're shading)
If some random plugin decides they want to disable pre-via injection, this affects all other plugins using packetevents and may lead to unexpected behavior

I can take a look at dynamic injection later

@retrooper
Copy link
Owner

After some thought, dynamic injection may actually be the smartest option here. Based on the type of listener the developer(s) register, a handler (pre or post ViaVersion) will be added to the pipeline (dynamically). If developers register both, then we will have two handlers for incoming packets and two for outgoing. I think this is actually a decent solution, the only issue is PacketEvents has an internal listener which, by default, listens post ViaVersion. So on PE installations, the post ViaVersion handler would always be there no matter what.

@ytnoos
Copy link
Contributor Author

ytnoos commented Jan 26, 2026

I haven't tested this yet, let me know if the logic is fine. I couldn't find any other way to actually intercept registered listeners without exposing methods to EventManager that anyone could call

@NikV2
Copy link
Contributor

NikV2 commented Jan 30, 2026

Looking forward towards this pull request!

@ytnoos
Copy link
Contributor Author

ytnoos commented Jan 30, 2026

Tested on 1.8.8 and 1.21.8 and works well!

@retrooper
Copy link
Owner

I suppose we can start reviewing and editing this PR. I have not received any PR from Axionize, a Grim maintainer.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants