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

Expose forwarding related configuration data in ProxyConfig. #1063

Open
kyngs opened this issue Aug 20, 2023 · 5 comments · May be fixed by #1064
Open

Expose forwarding related configuration data in ProxyConfig. #1063

kyngs opened this issue Aug 20, 2023 · 5 comments · May be fixed by #1064
Labels
type: feature New feature or request

Comments

@kyngs
Copy link
Contributor

kyngs commented Aug 20, 2023

Currently, the ProxyConfig interface does not expose anything related to player info forwarding. This is quite unfortunate if you need to access the forwarding mode (and secret) from your plugin. (For example, to send the secret to a built-in limbo).

The workaround to this issue is quite simple, import the velocity API implementation, cast the configuration to VelocityConfiguration, and then you can get the forwarding config. But, I believe that it is quite unfortunate to need to use the velocity API implementation directly.

@kyngs
Copy link
Contributor Author

kyngs commented Aug 21, 2023

I've just looked into implementing this (as it seemed like a simple thing), but it appears the PlayerInfoForwarding enum is declared inside the velocity-proxy module and thus not available from the API module.
Moving the enum into the API module would cause incompatibilities with the plugins using it. (This should be defensible considering plugins should not use things from the proxy module, but I still don't want to break the plugins)

I'm kinda not sure what to do tbh

@hugmanrique
Copy link
Contributor

Plugin authors know that everything in the proxy module is not subject to API versioning guarantees, and so may change at any moment. I doubt the list of plugins that depend on this enum is that long anyways.

@kyngs
Copy link
Contributor Author

kyngs commented Aug 21, 2023

Mine for example does and this places me in a kinda awkward situation. Either I can use the internal module which will work for all versions of Velocity up before this got resolved, or I can use the new API which will only work on the latest versions

@hugmanrique
Copy link
Contributor

There's ways to conditionally use one enum or another (for example, creating an interface in your plugin that abstracts the logic common to both enums and creating implementations in two different classes; making sure you don't load one or the other accidentally and using reflection to detect whether the Velocity instance provides this new feature in API). Still, your plugin depends on something that was never guaranteed to exist forever; the responsibility to fix this falls on plugin authors, not Velocity contributors.

@kyngs
Copy link
Contributor Author

kyngs commented Aug 21, 2023

Fair ig, I'll just move the class into the api

kyngs added a commit to kyngs/Velocity that referenced this issue Aug 21, 2023
@kyngs kyngs linked a pull request Aug 21, 2023 that will close this issue
@4drian3d 4drian3d added the type: feature New feature or request label Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants