refactored sdk functions to always return &alpha.Bool{} instead of nil

pkg/sdkserver/localsdk.go

@@ -399,7 +399,7 @@ func (l *LocalSDKServer) stopReserveTimer() {
 // [FeatureFlag:PlayerTracking]
 func (l *LocalSDKServer) PlayerConnect(ctx context.Context, id *alpha.PlayerID) (*alpha.Bool, error) {
 	if !runtime.FeatureEnabled(runtime.FeaturePlayerTracking) {
-		return nil, errors.Errorf("%s not enabled", runtime.FeaturePlayerTracking)
+		return &alpha.Bool{Bool: false}, errors.Errorf("%s not enabled", runtime.FeaturePlayerTracking)
 	}
 	l.logger.WithField("playerID", id.PlayerID).Info("Player Connected")
 	l.gsMutex.Lock()
@@ -417,7 +417,7 @@ func (l *LocalSDKServer) PlayerConnect(ctx context.Context, id *alpha.PlayerID)
 	}
 
 	if l.gs.Status.Players.Count >= l.gs.Status.Players.Capacity {
-		return &alpha.Bool{}, errors.New("Players are already at capacity")
+		return &alpha.Bool{Bool: false}, errors.New("Players are already at capacity")
 	}
 
 	l.gs.Status.Players.Ids = append(l.gs.Status.Players.Ids, id.PlayerID)
@@ -433,7 +433,7 @@ func (l *LocalSDKServer) PlayerConnect(ctx context.Context, id *alpha.PlayerID)
 // [FeatureFlag:PlayerTracking]
 func (l *LocalSDKServer) PlayerDisconnect(ctx context.Context, id *alpha.PlayerID) (*alpha.Bool, error) {
 	if !runtime.FeatureEnabled(runtime.FeaturePlayerTracking) {
-		return nil, errors.Errorf("%s not enabled", runtime.FeaturePlayerTracking)
+		return &alpha.Bool{Bool: false}, errors.Errorf("%s not enabled", runtime.FeaturePlayerTracking)
 	}
 	l.logger.WithField("playerID", id.PlayerID).Info("Player Disconnected")
 	l.gsMutex.Lock()
@@ -467,7 +467,7 @@ func (l *LocalSDKServer) PlayerDisconnect(ctx context.Context, id *alpha.PlayerI
 // [FeatureFlag:PlayerTracking]
 func (l *LocalSDKServer) IsPlayerConnected(c context.Context, id *alpha.PlayerID) (*alpha.Bool, error) {
 	if !runtime.FeatureEnabled(runtime.FeaturePlayerTracking) {
-		return nil, errors.Errorf("%s not enabled", runtime.FeaturePlayerTracking)
+		return &alpha.Bool{Bool: false}, errors.Errorf("%s not enabled", runtime.FeaturePlayerTracking)
 	}
 
 	result := &alpha.Bool{Bool: false}

pkg/sdkserver/localsdk_test.go

@@ -344,6 +344,43 @@ func TestLocalSDKServerPlayerCapacity(t *testing.T) {
 	assert.Equal(t, int64(10), gs.Status.Players.Capacity)
 }
 
+func TestLocalSDKServerPlayerConnectAndDisconnectWithoutPlayerTracking(t *testing.T) {
+	t.Parallel()
+	runtime.FeatureTestMutex.Lock()
+	defer runtime.FeatureTestMutex.Unlock()
+
+	assert.NoError(t, runtime.ParseFeatures(string(runtime.FeaturePlayerTracking) + "=false"))
+
+	l, err := NewLocalSDKServer("")
+
+	e := &alpha.Empty{}
+	capacity, err := l.GetPlayerCapacity(context.Background(), e)
+	assert.Nil(t, capacity)
+	assert.Error(t, err)
+
+	count, err := l.GetPlayerCount(context.Background(), e)
+	assert.Error(t, err)
+	assert.Nil(t, count)
+
+	list, err := l.GetConnectedPlayers(context.Background(), e)
+	assert.Error(t, err)
+	assert.Nil(t, list)
+
+	id := &alpha.PlayerID{PlayerID: "test-player"}
+
+	ok, err := l.PlayerConnect(context.Background(), id)
+	assert.Error(t, err)
+	assert.False(t, ok.Bool)
+
+	ok, err = l.IsPlayerConnected(context.Background(), id)
+	assert.Error(t, err)
+	assert.False(t, ok.Bool)
+
+	ok, err = l.PlayerDisconnect(context.Background(), id)
+	assert.Error(t, err)
+	assert.False(t, ok.Bool)
+}
+
 func TestLocalSDKServerPlayerConnectAndDisconnect(t *testing.T) {
 	t.Parallel()
 

pkg/sdkserver/sdkserver.go

@@ -568,7 +568,7 @@ func (s *SDKServer) stopReserveTimer() {
 // [FeatureFlag:PlayerTracking]
 func (s *SDKServer) PlayerConnect(ctx context.Context, id *alpha.PlayerID) (*alpha.Bool, error) {
 	if !runtime.FeatureEnabled(runtime.FeaturePlayerTracking) {
-		return nil, errors.Errorf("%s not enabled", runtime.FeaturePlayerTracking)
+		return &alpha.Bool{Bool: false}, errors.Errorf("%s not enabled", runtime.FeaturePlayerTracking)
 	}
 	s.logger.WithField("PlayerId", id.PlayerID).Debug("Player Connected")
 
@@ -583,7 +583,7 @@ func (s *SDKServer) PlayerConnect(ctx context.Context, id *alpha.PlayerID) (*alp
 	}
 
 	if int64(len(s.gsConnectedPlayers)) >= s.gsPlayerCapacity {
-		return &alpha.Bool{}, errors.New("players are already at capacity")
+		return &alpha.Bool{Bool: false}, errors.New("players are already at capacity")
 	}
 
 	// let's retain the original order, as it should be a smaller patch on data change
@@ -598,7 +598,7 @@ func (s *SDKServer) PlayerConnect(ctx context.Context, id *alpha.PlayerID) (*alp
 // [FeatureFlag:PlayerTracking]
 func (s *SDKServer) PlayerDisconnect(ctx context.Context, id *alpha.PlayerID) (*alpha.Bool, error) {
 	if !runtime.FeatureEnabled(runtime.FeaturePlayerTracking) {
-		return nil, errors.Errorf("%s not enabled", runtime.FeaturePlayerTracking)
+		return &alpha.Bool{Bool: false}, errors.Errorf("%s not enabled", runtime.FeaturePlayerTracking)
 	}
 	s.logger.WithField("PlayerId", id.PlayerID).Debug("Player Disconnected")
 
@@ -629,7 +629,7 @@ func (s *SDKServer) PlayerDisconnect(ctx context.Context, id *alpha.PlayerID) (*
 // [FeatureFlag:PlayerTracking]
 func (s *SDKServer) IsPlayerConnected(ctx context.Context, id *alpha.PlayerID) (*alpha.Bool, error) {
 	if !runtime.FeatureEnabled(runtime.FeaturePlayerTracking) {
-		return nil, errors.Errorf("%s not enabled", runtime.FeaturePlayerTracking)
+		return &alpha.Bool{Bool: false}, errors.Errorf("%s not enabled", runtime.FeaturePlayerTracking)
 	}
 	s.gsUpdateMutex.RLock()
 	defer s.gsUpdateMutex.RUnlock()

pkg/sdkserver/sdkserver_test.go

@@ -1083,6 +1083,77 @@ func TestSDKServerPlayerCapacity(t *testing.T) {
 	agtesting.AssertEventContains(t, m.FakeRecorder.Events, "PlayerCapacity Set to 20")
 }
 
+func TestSDKServerPlayerConnectAndDisconnectWithoutPlayerTracking(t *testing.T) {
+	t.Parallel()
+	agruntime.FeatureTestMutex.Lock()
+	defer agruntime.FeatureTestMutex.Unlock()
+
+	err := agruntime.ParseFeatures(string(agruntime.FeaturePlayerTracking) + "=false")
+	require.NoError(t, err, "Can not parse FeaturePlayerTracking feature")
+
+	fixture := &agonesv1.GameServer{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test",
+			Namespace: "default",
+		},
+		Status: agonesv1.GameServerStatus{
+			State: agonesv1.GameServerStateReady,
+		},
+	}
+
+	m := agtesting.NewMocks()
+	m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+		return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{*fixture}}, nil
+	})
+
+	stop := make(chan struct{})
+	defer close(stop)
+
+	sc, err := defaultSidecar(m)
+	require.NoError(t, err)
+
+	sc.informerFactory.Start(stop)
+	assert.True(t, cache.WaitForCacheSync(stop, sc.gameServerSynced))
+
+	go func() {
+		err = sc.Run(stop)
+		assert.NoError(t, err)
+	}()
+
+	// check initial value comes through
+	// async, so check after a period
+	e := &alpha.Empty{}
+	err = wait.Poll(time.Second, 10*time.Second, func() (bool, error) {
+		count, err := sc.GetPlayerCapacity(context.Background(), e)
+
+		assert.Nil(t, count)
+		return false, err
+	})
+	assert.Error(t, err)
+
+	count, err := sc.GetPlayerCount(context.Background(), e)
+	require.Error(t, err)
+	assert.Nil(t, count)
+
+	list, err := sc.GetConnectedPlayers(context.Background(), e)
+	require.Error(t, err)
+	assert.Nil(t, list)
+
+	id := &alpha.PlayerID{PlayerID: "test-player"}
+
+	ok, err := sc.PlayerConnect(context.Background(), id)
+	require.Error(t, err)
+	assert.False(t, ok.Bool)
+
+	ok, err = sc.IsPlayerConnected(context.Background(), id)
+	require.Error(t, err)
+	assert.False(t, ok.Bool)
+
+	ok, err = sc.PlayerDisconnect(context.Background(), id)
+	require.Error(t, err)
+	assert.False(t, ok.Bool)
+}
+
 func TestSDKServerPlayerConnectAndDisconnect(t *testing.T) {
 	t.Parallel()
 	agruntime.FeatureTestMutex.Lock()