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

Streamline api #832

Merged
merged 3 commits into from
Jul 20, 2024
Merged

Streamline api #832

merged 3 commits into from
Jul 20, 2024

Conversation

AlexProgrammerDE
Copy link
Contributor

This basically makes MinecraftCodec stateless and removes unused APIs.
The MinecraftCodecHelper has a state for an unused registry API, which is not needed at all. Developers can just send their own registry as seen in ServerListener. These only exist because there was an attempt at allowing multi-version support inside MinecraftCodecHelper, but that's not gonna be necessary if ViaVersion support gets added to MCPL.
The option to add back logic for multi-version support is fully open for the future. I didn't remove MinecraftCodecHelper being instance-based, so it's fully possible to readd the multi-version logic back in the future. Just level events and sounds being dynamic is not enough for multi-version support and thus should be removed to streamline the API.
This also removes many workarounds from the code that cause possible inconsistency. Often the MinecraftCodecHelper gets initialized in the tests with empty parameters. This removes them as parameters and removes the need to provide them by moving them to the object itself.
PacketCodecs were also improved to decrease the inheritance issues and streamline the code flow and remove the UnsupportedOperationExceptions being needed.
BufferedPacket was also removed since it's useless for Minecraft.

This basically makes MinecraftCodec stateless and removes unused APIs.
The MinecraftCodecHelper has a state for an unused registry API, which is not needed at all. Developers can just send their own registry as seen in ServerListener. These only exist because there was an attempt at allowing multi-version support inside MinecraftCodecHelper, but that's not gonna be necessary if ViaVersion support gets added to MCPL.
The option to add back logic for multi-version support is fully open for the future. I didn't remove MinecraftCodecHelper being instance-based, so it's fully possible to readd the multi-version logic back in the future. Just level events and sounds being dynamic is not enough for multi-version support and thus should be removed to streamline the API.
This basically makes MinecraftCodec stateless and removes unused APIs.
The MinecraftCodecHelper has a state for an unused registry API, which is not needed at all. Developers can just send their own registry as seen in ServerListener. These only exist because there was an attempt at allowing multi-version support inside MinecraftCodecHelper, but that's not gonna be necessary if ViaVersion support gets added to MCPL.
The option to add back logic for multi-version support is fully open for the future. I didn't remove MinecraftCodecHelper being instance-based, so it's fully possible to readd the multi-version logic back in the future. Just level events and sounds being dynamic is not enough for multi-version support and thus should be removed to streamline the API.
This also removes many workarounds from the code that cause possible inconsistency. Often the MinecraftCodecHelper gets initialized in the tests with empty parameters. This removes them as parameters and removes the need to provide them by moving them to the object itself.
PacketCodecs were also improved to decrease the inheritance issues and streamline the code flow and remove the UnsupportedOperationExceptions being needed.
BufferedPacket was also removed since it's useless for Minecraft.
Copy link
Member

@basaigh basaigh left a comment

Choose a reason for hiding this comment

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

There do seem to be some unrelated changes present, but since this is seemingly required for state switching to be fixed, approved.

@Camotoy Camotoy merged commit 41ecced into GeyserMC:master Jul 20, 2024
1 check passed
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.

3 participants