Skip to content

Commit f78c743

Browse files
Fix sound player interface mismatch (minekube#601)
* feat: Add player server entity ID and match check Co-authored-by: robin.braemer <robin.braemer@web.de> * Add sound package tests and update player interface Co-authored-by: robin.braemer <robin.braemer@web.de> * Refactor sound package to use type assertions for player methods Co-authored-by: robin.braemer <robin.braemer@web.de> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
1 parent 959dc1b commit f78c743

4 files changed

Lines changed: 70 additions & 8 deletions

File tree

pkg/edition/java/sound/sound.go

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,14 @@ const (
3737

3838
// Player represents a player that can play and stop sounds.
3939
// This is typically a *connectedPlayer from the proxy package.
40+
//
41+
// The Play function uses type assertions to access additional methods
42+
// (CurrentServerEntityID, CheckServerMatch) that are not part of this
43+
// interface, allowing API users to pass proxy.Player without requiring
44+
// those methods in the proxy.Player interface.
4045
type Player interface {
4146
proto.PacketWriter
4247
Protocol() proto.Protocol
43-
// CurrentServerEntityID returns the entity ID of the player on their current server.
44-
// Returns false if the player is not connected to a server.
45-
CurrentServerEntityID() (int, bool)
46-
// CheckServerMatch checks if the other player is on the same server.
47-
CheckServerMatch(other interface{ CurrentServerEntityID() (int, bool) }) bool
4848
}
4949

5050
// ParseSource parses a sound source from a string.
@@ -110,8 +110,22 @@ func Play(player Player, sound Sound, emitter Player) error {
110110
return fmt.Errorf("%w: player is on %s", ErrUISourceUnsupported, player.Protocol())
111111
}
112112

113+
// Type assert to access server entity methods that are not part of the Player interface.
114+
// This allows API users to pass proxy.Player without requiring those methods in the interface.
115+
type entityIDProvider interface {
116+
CurrentServerEntityID() (int, bool)
117+
}
118+
type serverMatcher interface {
119+
CheckServerMatch(other interface{ CurrentServerEntityID() (int, bool) }) bool
120+
}
121+
122+
entityProvider, ok := player.(entityIDProvider)
123+
if !ok {
124+
return fmt.Errorf("player does not implement required methods for sound playback")
125+
}
126+
113127
// Get target player's entity ID
114-
targetEntityID, ok := player.CurrentServerEntityID()
128+
targetEntityID, ok := entityProvider.CurrentServerEntityID()
115129
if !ok {
116130
return ErrNotConnected
117131
}
@@ -122,11 +136,20 @@ func Play(player Player, sound Sound, emitter Player) error {
122136
// Self emitter
123137
emitterEntityID = targetEntityID
124138
} else if emitter != nil {
139+
// Type assert emitter to entityIDProvider
140+
emitterProvider, ok := emitter.(entityIDProvider)
141+
if !ok {
142+
return ErrInvalidEmitter
143+
}
125144
// Check if emitter is on the same server
126-
if !player.CheckServerMatch(emitter) {
145+
matcher, ok := player.(serverMatcher)
146+
if !ok {
147+
return fmt.Errorf("player does not implement CheckServerMatch")
148+
}
149+
if !matcher.CheckServerMatch(emitterProvider) {
127150
return ErrDifferentServers
128151
}
129-
emitterEntityID, ok = emitter.CurrentServerEntityID()
152+
emitterEntityID, ok = emitterProvider.CurrentServerEntityID()
130153
if !ok {
131154
return ErrEmitterNotConnected
132155
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package sound
2+
3+
import (
4+
"testing"
5+
6+
"go.minekube.com/gate/pkg/edition/java/proxy"
7+
)
8+
9+
// TestProxyPlayerImplementsSoundPlayer validates that proxy.Player can be used
10+
// as sound.Player, which is required for the sound package examples in the docs.
11+
//
12+
// This test ensures that the usage pattern shown in the documentation examples
13+
// (e.g., .web/docs/developers/examples/sound-example-play.go) will compile.
14+
//
15+
// The sound.Player interface only requires proto.PacketWriter and Protocol(),
16+
// which proxy.Player provides through its embedded interfaces. The additional
17+
// methods (CurrentServerEntityID, CheckServerMatch) are accessed via type
18+
// assertion to internalPlayer in the implementation.
19+
func TestProxyPlayerImplementsSoundPlayer(t *testing.T) {
20+
// This is a compile-time check: if proxy.Player doesn't implement sound.Player,
21+
// this will fail to compile.
22+
var _ Player = (proxy.Player)(nil)
23+
}
24+
25+
// TestSoundPlayWithProxyPlayer validates that sound.Play can accept proxy.Player.
26+
// This mirrors the usage in the documentation examples where:
27+
// player := e.Player() // returns proxy.Player
28+
// sound.Play(player, sound, player) // should work
29+
//
30+
// The function should compile even though proxy.Player doesn't explicitly
31+
// implement CurrentServerEntityID and CheckServerMatch in its interface.
32+
// The implementation uses type assertion to internalPlayer to access these methods.
33+
func TestSoundPlayWithProxyPlayer(t *testing.T) {
34+
// This function signature matches what's used in the docs examples.
35+
// If this compiles, then the docs examples should also compile.
36+
_ = func(player proxy.Player, snd Sound, emitter Player) error {
37+
return Play(player, snd, emitter)
38+
}
39+
}

sound.test

10.2 MB
Binary file not shown.

test_sound_interface

9.44 MB
Binary file not shown.

0 commit comments

Comments
 (0)