check if channel is active before injecting on velocity

[linsaftw]

This fixes anti-bot support with forks like VeloFlame that act before Geyser and Floodgate and fixes a critical error during initialization if the channel was closed

[Camotoy]

Hi! Thanks for the PR. Would you be able to attach a code snippet or exception log explaining why this code breaks on your fork? Thanks!

[linsaftw]

Hi! Thanks for the PR. Would you be able to attach a code snippet or exception log explaining why this code breaks on your fork? Thanks!

https://mclo.gs/SkjhrJq Basically we apply anti-bot measures on init, so the channel is closed before floodgate handles it.

[Camotoy]

I would double-check to ensure this PR addresses those changes. We call that original ServerChannelInitializer that would have your anti-bot changes with the invoke(original, initChannel, channel); command that's below the return statement you presented. I could most certainly be wrong though.

[linsaftw]

Member

You need to skip invoking anything on initChannel if the channel is closed.

[Tim203]

Have you tried without PacketEvents? Seems like this exception is caused by PacketEvents

[Kas-tle]

Note that packetevents has specific behavior when floodgate is present: https://github.com/retrooper/packetevents/blob/2.0/spigot/src/main/java/io/github/retrooper/packetevents/injector/SpigotChannelInjector.java#L168

[linsaftw]

Have you tried without PacketEvents? Seems like this exception is caused by PacketEvents

No thats one of many exceptions. I already made a PR for PacketEvents, I also have made a PR for floodgate, and now for ViaVersion. You have to handle inactive channel from other pipeline handlers! Weird to see this hasn't been merged yet. Its not overly complicated! VeloFlame (close and dont inject minecraft decoder when vpn detected) -> Floodgate (channel was not injected, error) -> PacketEvents (same as floodgate) -> ViaVersion (same as floodgate)

[linsaftw]

Done, thank you guys for your insight, you actually right!

[linsaftw]

Works perfectly!

[Tim203]

I have to agree with the comment made on your PR to ViaVersion: ViaVersion/ViaVersion#4701 (comment)
I know you're only suggesting a minor change, but we shouldn't have to check whether the channel is closed.
You simply shouldn't call our initializer if we shouldn't init the channel (or manually clear the pipeline).
Especially as a fork you have so many options to make a generic solution that works across all plugins, instead of every plugin having to work around your behavior.

[linsaftw]

I have to agree with the comment made on your PR to ViaVersion: ViaVersion/ViaVersion#4701 (comment) I know you're only suggesting a minor change, but we shouldn't have to check whether the channel is closed. You simply shouldn't call our initializer if we shouldn't init the channel (or manually clear the pipeline). Especially as a fork you have so many options to make a generic solution that works across all plugins, instead of every plugin having to work around your behavior.

Thank you so much. We changed this on our end. Thank you so much for pointing out the issue!