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

Add PlayerConfigurationEvent and PlayerEnteredConfigurationEvent #1371

Merged
merged 5 commits into from
Jul 12, 2024

Conversation

Gerrygames
Copy link
Contributor

@Gerrygames Gerrygames commented Jul 5, 2024

This addresses issues #1367 and #1368

  • Configuring the player (i.e. sending resource packs) should now be done in the new PlayerConfigurationEvent.
  • The new PlayerEnteredConfigurationEvent is called when a player acknowledged the switch to configuration state.
  • The PlayerEnterConfigurationEvent is no longer called twice. It is now called when the backed wants to reconfigure the player. Velocity will wait (up to 5 seconds) for this event and only then ask the client to switch to configuration state.
  • The PlayerFinishConfigurationEvent should no longer be used to configure the player (i.e. sending resource packs). This is because since 1.20.5 the backend server can't send keep alive packets between switching state anymore and the connection will thus time out. Velocity will only wait 5 seconds for this event.
  • Improved java docs. Feedback would be appreciated.

@Gerrygames Gerrygames merged commit 6073f69 into dev/3.0.0 Jul 12, 2024
2 checks passed
@Gerrygames Gerrygames deleted the fix/config-events branch July 12, 2024 09:16
qyl27 pushed a commit to MeowCraftMC/Velocity that referenced this pull request Jul 13, 2024
…erMC#1371)

* Configuring the player (i.e. sending resource packs) should now be done in the new PlayerConfigurationEvent.

* The new PlayerEnteredConfigurationEvent is called when a player acknowledged the switch to configuration state.

* The PlayerEnterConfigurationEvent is no longer called twice. It is now called when the backed wants to reconfigure the player.

* The PlayerFinishConfigurationEvent should no longer be used to configure the player (i.e. sending resource packs). This is because since 1.20.5 the backend server can't send keep alive packets between switching state anymore and the connection will thus time out.
pull bot pushed a commit to WiIIiam278/Velocity that referenced this pull request Jul 13, 2024
…erMC#1371)

* Configuring the player (i.e. sending resource packs) should now be done in the new PlayerConfigurationEvent.

* The new PlayerEnteredConfigurationEvent is called when a player acknowledged the switch to configuration state.

* The PlayerEnterConfigurationEvent is no longer called twice. It is now called when the backed wants to reconfigure the player.

* The PlayerFinishConfigurationEvent should no longer be used to configure the player (i.e. sending resource packs). This is because since 1.20.5 the backend server can't send keep alive packets between switching state anymore and the connection will thus time out.
@Timongcraft
Copy link
Contributor

Is it intentional that the new PlayerConfigurationEvent is only called for the initial configuration and not for subsequent ones, unlike the (old) PlayerFinishConfigurationEvent?

@Gerrygames
Copy link
Contributor Author

No that's a mistake then. I will investigate that later.

@Timongcraft
Copy link
Contributor

I just noticed that the Javadoc for the PlayerEnterConfigurationEvent specifies that it is called before the client enters the configuration state. However, it also states that it only waits 5 seconds because the backend can't keep the connection alive during the configuration state. Since the client is not in the configuration state at that moment, does this mean that the backend will terminate the connection because it requested the change to the configuration state? If so, I think the wording is a bit confusing.

@Gerrygames
Copy link
Contributor Author

The "clientbound" connection between velocity and the backend is already in configuration state but the clientbound connection between velocity and the client is still in play state. The java doc states that the backend can't keep the connection alive during state changes which is correct. If you have any better wording for it, you're welcome to suggest it.

@Timongcraft
Copy link
Contributor

How about this?

 * <p>Velocity will wait for this event before asking the client to enter configuration state.
 * <br>However, for the backend server, the player is already in the configuration state. Since the server cannot maintain the connection during state changes,
 * Velocity will only wait for a maximum of 5 seconds.</p>

The "clientbound" connection between velocity and the backend is already in configuration state but the clientbound connection between velocity and the client is still in play state. The java doc states that the backend can't keep the connection alive during state changes which is correct. If you have any better wording for it, you're welcome to suggest it.

@Timongcraft
Copy link
Contributor

No that's a mistake then. I will investigate that later.

Have you found anything?

@Gerrygames
Copy link
Contributor Author

No that's a mistake then. I will investigate that later.

Have you found anything?

Fixed by d999ee2

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