Velocity
Velocity copied to clipboard
PaperMC
feat: Add primitive support for sound api
With this PR, the Adventure-provided sound API is partially implemented.
It adds the ability to play sounds for a player (from the player itself or other players on the same server) and to stop sounds, while making the methods fail silently if the sound can't be played/stopped.
What this PR implements:
- A method to play back (custom) sounds for a specific player (from the player itself or other players) for 1.19.3+ (- This also enables the respective methods on the registered server, by forwarding it to the players)
- A method to stop (custom) sounds for players with version 1.19.3+
What this PR doesn't implement:
- A method to play back (custom) sounds for a specific player at a position or another entity for players of all versions
- A method to play back (custom) sounds (globally) at a specific location for players of all versions
- A easily way to access a list of vanilla sounds - A method to play back (custom) sounds for a specific player for players with version 1.19.2 and below - A method to stop (custom) sounds for players with version 1.19.2 and below
Clarification: This PR only implements a sound API for version 1.19.3+ because only in this version is the server able to play a (vanilla) sound for a player without requiring a version-dependent id for the sound.
From my end, it is finished, although I would think it makes more sense to make the methods fail silently again.
~~We could also look into playing sounds from another player, but if so, I think that should be discussed first.~~
What is the state of this? We are currently discussing implementing this ourselves, but we would way prefer having it implemented in upstream.
Apart from maybe adding Javadoc to the RegisteredServer, this PR is ready from my end.
@4drian3d Do you think additional Javadoc entries are required? Is there an estimate when this might get merged?
@electronicboy Sry for the ping, but what is blocking this from being merged?
Even though the sound should be ignored for invalid entity ids, I still choose to test for the current server and thus require it on the receiver because afaik they are not that unique and if there's some other entity with that id exists, it could cause unwanted consequences.
Is there any chance that this could be merged soon? I'd love to use it! :)
Is there any chance that this could be merged soon? I'd love to use it! :)
I'd wait for 1.21.6 and see how to handle the implementation of Adventure for the new "UI" sound category.
Edit: Seems like it was just added to the end of the enum, in vanilla and adventure, so it shouldn't be a problem.
I did not implement the Player#playSound(Sound) method as adventure defines it like this:
Plays a sound at the location of the recipient of the sound.
Since we can't fulfill that, would it be fine to break that contract by replacing the javadoc?
https://github.com/KyoriPowered/adventure/blob/-/api/src/main/java/net/kyori/adventure/audience/Audience.java#L629C6-L631
I'll be honest, I tried this pull request for 30 minutes without knowing why the sound wasn't playing, since stopSound was working for me, until I saw that the #playSound(Sound) method wasn't implemented
From a plugin developer's perspective, this method should be implemented, but from a contract compliance perspective, it's okay not to implement it as required by the contract. With this in mind, good documentation at https://docs.papermc.io/velocity/dev/pitfalls/#audience-operations-are-not-fully-supported would suffice, so I am now reverting my change
What do you say about stuffing override dialog api methods into this pr as well for documentation purposes?