feat: Add primitive support for sound api #1422
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
Timongcraft
force-pushed
the
feature/sound-api
branch
from
4c5911e to
00d4dc6
Compare
Timongcraft
force-pushed
the
feature/sound-api
branch
2 times, most recently
from
e91c70a to
87b0e57
Compare
|
What is the state of this? I would like to have this feature very much. |
|
From my end, it is finished, although I would think it makes more sense to make the methods fail silently again. |
|
|
|
@astei Could this be reviewed pls. |
|
@4drian3d Since it has been approved, can this also be merged? |
|
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? |
fix: implement the correct playSound method fix: bumped "since" version
Timongcraft
force-pushed
the
feature/sound-api
branch
from
e85d130 to
9671cd3
Compare
|
@electronicboy |
proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java
Outdated
Show resolved
Hide resolved
| * <b>This method is not currently implemented in Velocity | ||
| * and will not perform any actions.</b> | ||
| * <p>Note: This method is currently only implemented for players from version 1.19.3 and above | ||
| * and requires a present {@link #getCurrentServer}. Additionally, it only supports {@link Sound.Emitter#self()} for now. |
There was a problem hiding this comment.
If we're tracking all the entity IDs now, other emitters could be supported right? Not a requirement for this PR, just a future thought.
There was a problem hiding this comment.
Yea, like other players but I found out this:
I did not add support to use other players as emitter, as custom sounds played like that will just appear like if they were coming from the player itself.
And thought it wasn't worth it, right?
There was a problem hiding this comment.
That sounds like you were testing with sounds with the wrong number of channels - they definitely should follow the player.
There was a problem hiding this comment.
wdym? I used the player entity sound packet with a entity id of another player, in I think 1.21.4, and it just acted like the sound was coming from the player I sent the packet to. But I could test again.
There was a problem hiding this comment.
Yea, in my testing (new setup, 1.21.4, with paper and with changed velocity code) when using another player as emitter it only is played from the other player if it is 1. a vanilla sound, and 2. uses the stream parameter like music_disc.cat but not minecraft:item.goat_horn.sound.2.
And thus I did not find it useful and too complicated to understand to include.
There was a problem hiding this comment.
If you have a stereo sound it will never play following the entity and instead will play globally. https://bugs.mojang.com/browse/MC-146721
There was a problem hiding this comment.
Ah thanks, didn't know about that.
I'll test and implement it again.
Would it make sense to add that to the adventure javadoc, I'd open a pr if you want?
There was a problem hiding this comment.
Would be a good addition to the Sound class yes.
There was a problem hiding this comment.
Would be a good addition to the Sound class yes.
Why the 'Sound' class instead of the specific 'playSound' method in 'Audience'?
|
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. |
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:
(- This also enables the respective methods on the registered server, by forwarding it to the players)
What this PR doesn't implement:
- 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.