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 Configuration State API #1261

Merged
merged 12 commits into from
Jun 16, 2024
Merged

Add Configuration State API #1261

merged 12 commits into from
Jun 16, 2024

Conversation

4drian3d
Copy link
Contributor

@4drian3d 4drian3d commented Mar 2, 2024

Added events for when a player enters and finishes the configuration phase.
This, along with pull request #1224 would implement a functional way to check the State in which the player is in

blocked by #1224
resolves #1120

# Conflicts:
#	proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java
@4drian3d 4drian3d added the type: feature New feature or request label Mar 2, 2024
@4drian3d 4drian3d added this to the Velocity 3.3.0 milestone Mar 2, 2024
@SasaBrix
Copy link

Are there any new updates?

@4drian3d
Copy link
Contributor Author

Are there any new updates?

I am waiting for some comments in order to improve this pull request, if you have something to contribute or any suggestion, you are welcome to do it

@SandwichBtw
Copy link

Is there anything else specifically that needs to be done before this could get merged? Just wondering if there's any plan other than just waiting on comments.

# Conflicts:
#	proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java
Copy link
Contributor

@Luccboy Luccboy left a comment

Choose a reason for hiding this comment

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

This is probably unimportant, but I noticed that Configuration phase is sometimes written with a capital c and sometimes with a lowercase c in the JavaDocs. Maybe you want to keep the spelling consistent?

@4drian3d 4drian3d marked this pull request as ready for review April 21, 2024 22:27
@Gerrygames
Copy link
Contributor

I think there is currently no way to know to which server the player is currently connecting to when PlayerFinishConfigurationEvent or PlayerFinishedConfigurationEvent are called. Should probably add the ServerConnection to all configuration events.

@electronicboy
Copy link
Member

electronicboy commented Apr 22, 2024

The players current server should be switched when they enter configuration for that server, IMHO; iirc, that was missing, however (and has generally been a source of a whole bunch of other issues over the past while)

@Gerrygames
Copy link
Contributor

Currently the current server isnt set until after ServerConnectedEvent has been called which is done when handling the join game packet.

@Gerrygames
Copy link
Contributor

Blocking the PlayerFinishConfigurationEvent for more than ~30 seconds was not possible because the backend server would experience a timeout. I fixed this with the latest commit by not disabling auto-reading and instead modifying the PlayPacketQueueHandler to also queue inbound play packets.

@ammodev
Copy link

ammodev commented May 13, 2024

Is there any updates on this? I would really enjoy using this api without having to manually build velocity. Iirc theres nothing to be done here, correct?

# Conflicts:
#	proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java
…t/config-state

# Conflicts:
#	proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/handler/LegacyResourcePackHandler.java
@twisti-dev
Copy link

Are there any plans to merge this?

@4drian3d
Copy link
Contributor Author

4drian3d commented Jun 2, 2024

Are there any plans to merge this?

Yes

@Keviro
Copy link

Keviro commented Jun 5, 2024

Are there any plans to merge this?

There are plans, there is just no one executing them xD

@Timongcraft
Copy link
Contributor

Are there any plans to merge this?

There are plans, there is just no one executing them xD

I think they want to bump the version first.

@ammodev
Copy link

ammodev commented Jun 5, 2024

Are there any plans to merge this?

There are plans, there is just no one executing them xD

I think they want to bump the version first.

What else is missing to bump the version then? https://github.com/PaperMC/Velocity/milestone/6

@twisti-dev
Copy link

Hi, just wanted to follow up and see if there are any updates on merging this PR. It looks like everything is ready. Is there anything else that needs to be done before this can be merged?

Thanks!

@4drian3d 4drian3d merged commit e60e206 into dev/3.0.0 Jun 16, 2024
2 checks passed
@4drian3d 4drian3d deleted the feat/config-state branch June 16, 2024 15:56
pull bot pushed a commit to WiIIiam278/Velocity that referenced this pull request Jun 17, 2024
* Implement Configuration State events

* Implement PlayerFinishedConfigurationEvent

* Fixed PlayerFinishConfigurationEvent execution

* Apply suggestions

* Fix keep alive when blocking PlayerFinishConfigurationEvent

* Add ServerConnection to configuration events

* Separate PlayPacketQueueHandler to fix AutoReadHolderHandler

---------

Co-authored-by: Gero <[email protected]>
qyl27 pushed a commit to MeowCraftMC/Velocity that referenced this pull request Jul 6, 2024
* Implement Configuration State events

* Implement PlayerFinishedConfigurationEvent

* Fixed PlayerFinishConfigurationEvent execution

* Apply suggestions

* Fix keep alive when blocking PlayerFinishConfigurationEvent

* Add ServerConnection to configuration events

* Separate PlayPacketQueueHandler to fix AutoReadHolderHandler

---------

Co-authored-by: Gero <[email protected]>
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 this pull request may close these issues.

Configuration State API