Merge pull request #99 from markus-wa/issue-98-unexpected-player

Parser: Player tracking improvements

Author: markus-wa

.travis.yml

@@ -52,7 +52,7 @@ script:
   - go test -v -race -run "$race_tests" -timeout 15m
 
   # We run all tests again to get full coverage, technically unncecessary tho
-  - coverpkg_ignore='/msg'
+  - coverpkg_ignore='/(msg|fake)'
   # coverpkg for multiple mains is broken in 1.12, so we need to exclude the examples from coverage
   # https://github.com/golang/go/issues/30374
   - if ! [[ "$TRAVIS_GO_VERSION" =~ ^1\.11 ]]; then coverpkg_ignore="${coverpkg_ignore}|/examples"; fi

common/player.go

@@ -25,7 +25,7 @@ type Player struct {
 	ActiveWeaponID              int                // Used internally to set the active weapon, see ActiveWeapon()
 	RawWeapons                  map[int]*Equipment // All weapons the player is currently carrying
 	AmmoLeft                    [32]int            // Ammo left in the various weapons, index corresponds to key of RawWeapons
-	Entity                      *st.Entity
+	Entity                      st.IEntity
 	AdditionalPlayerInformation *AdditionalPlayerInformation // Mostly scoreboard information such as kills, deaths, etc.
 	ViewDirectionX              float32
 	ViewDirectionY              float32

datatables.go

@@ -178,7 +178,7 @@ func (p *Parser) bindPlayers() {
 	})
 }
 
-func (p *Parser) bindNewPlayer(playerEntity *st.Entity) {
+func (p *Parser) bindNewPlayer(playerEntity st.IEntity) {
 	entityID := playerEntity.ID()
 	rp := p.rawPlayers[entityID-1]
 
@@ -193,7 +193,7 @@ func (p *Parser) bindNewPlayer(playerEntity *st.Entity) {
 			pl = common.NewPlayer()
 			pl.Name = rp.name
 			pl.SteamID = rp.xuid
-			pl.IsBot = rp.isFakePlayer
+			pl.IsBot = rp.isFakePlayer || rp.guid == "BOT"
 			pl.UserID = rp.userID
 		}
 	}
@@ -206,6 +206,7 @@ func (p *Parser) bindNewPlayer(playerEntity *st.Entity) {
 
 	playerEntity.OnDestroy(func() {
 		delete(p.gameState.playersByEntityID, entityID)
+		pl.Entity = nil
 	})
 
 	// Position

datatables_test.go

@@ -0,0 +1,86 @@
+package demoinfocs
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+
+	"github.com/markus-wa/demoinfocs-golang/events"
+	st "github.com/markus-wa/demoinfocs-golang/sendtables"
+	fakest "github.com/markus-wa/demoinfocs-golang/sendtables/fake"
+)
+
+type DevNullReader struct {
+}
+
+func (DevNullReader) Read(p []byte) (n int, err error) {
+	return len(p), nil
+}
+
+func TestParser_BindNewPlayer_Issue98(t *testing.T) {
+	p := NewParser(new(DevNullReader))
+
+	p.rawPlayers = map[int]*playerInfo{
+		0: {
+			userID: 1,
+			name:   "Zim",
+			guid:   "BOT",
+		},
+		1: {
+			userID: 2,
+			name:   "The Suspect",
+			guid:   "123",
+		},
+	}
+
+	bot := fakePlayerEntity(1)
+	p.bindNewPlayer(bot)
+	bot.Destroy()
+
+	player := fakePlayerEntity(2)
+	p.bindNewPlayer(player)
+
+	assert.Len(t, p.GameState().Participants().All(), 1)
+}
+
+func TestParser_BindNewPlayer_Issue98_Reconnect(t *testing.T) {
+	p := NewParser(new(DevNullReader))
+
+	p.rawPlayers = map[int]*playerInfo{
+		0: {
+			userID: 2,
+			name:   "The Suspect",
+			guid:   "123",
+			xuid:   1,
+		},
+	}
+
+	player := fakePlayerEntity(1)
+	p.bindNewPlayer(player)
+	player.Destroy()
+
+	p.RegisterEventHandler(func(events.PlayerConnect) {
+		t.Error("expected no more PlayerConnect events but got one")
+	})
+	p.bindNewPlayer(player)
+
+	assert.Len(t, p.GameState().Participants().All(), 1)
+
+}
+
+func fakePlayerEntity(id int) st.IEntity {
+	entity := new(fakest.Entity)
+	entity.On("ID").Return(id)
+	var destroyCallback func()
+	entity.On("OnDestroy", mock.Anything).Run(func(args mock.Arguments) {
+		destroyCallback = args.Get(0).(func())
+	})
+	entity.On("OnPositionUpdate", mock.Anything).Return()
+	entity.On("FindProperty", mock.Anything).Return(new(st.Property))
+	entity.On("BindProperty", mock.Anything, mock.Anything, mock.Anything).Return(new(st.Property))
+	entity.On("Destroy").Run(func(mock.Arguments) {
+		destroyCallback()
+	})
+	return entity
+}

game_events.go

@@ -286,7 +286,7 @@ func (geh gameEventHandler) playerBlind(desc *msg.CSVCMsg_GameEventListDescripto
 
 	// Player.FlashDuration hasn't been updated yet,
 	// so we need to wait until the end of the tick before dispatching
-	geh.parser.currentFlashEvents = append(geh.parser.currentFlashEvents, events.PlayerFlashed{
+	geh.parser.delayedEvents = append(geh.parser.delayedEvents, events.PlayerFlashed{
 		Player:   geh.playerByUserID32(data["userid"].GetValShort()),
 		Attacker: geh.gameState().lastFlasher,
 	})
@@ -390,7 +390,10 @@ func (geh gameEventHandler) playerTeam(desc *msg.CSVCMsg_GameEventListDescriptor
 			player.Team = newTeam
 
 			oldTeam := common.Team(data["oldteam"].GetValByte())
-			geh.dispatch(events.PlayerTeamChange{
+			// Delayed for two reasons
+			// - team IDs of other players changing teams in the same tick might not have changed yet
+			// - player entities might not have been re-created yet after a reconnect
+			geh.parser.delayedEvents = append(geh.parser.delayedEvents, events.PlayerTeamChange{
 				Player:       player,
 				IsBot:        data["isbot"].GetValBool(),
 				Silent:       data["silent"].GetValBool(),

game_state.go

@@ -148,7 +148,11 @@ type Participants struct {
 func (ptcp Participants) ByUserID() map[int]*common.Player {
 	res := make(map[int]*common.Player)
 	for k, v := range ptcp.playersByUserID {
-		res[k] = v
+		// We need to check if the player entity hasn't been destroyed yet
+		// See https://github.com/markus-wa/demoinfocs-golang/issues/98
+		if v.Entity != nil {
+			res[k] = v
+		}
 	}
 	return res
 }
@@ -167,8 +171,8 @@ func (ptcp Participants) ByEntityID() map[int]*common.Player {
 // All returns all currently connected players & spectators.
 // The returned slice is a snapshot and is not updated on changes.
 func (ptcp Participants) All() []*common.Player {
-	res := make([]*common.Player, 0, len(ptcp.playersByUserID))
-	for _, p := range ptcp.playersByUserID {
+	res, original := ptcp.initalizeSliceFromByUserID()
+	for _, p := range original {
 		res = append(res, p)
 	}
 	return res
@@ -177,8 +181,8 @@ func (ptcp Participants) All() []*common.Player {
 // Playing returns all players that aren't spectating or unassigned.
 // The returned slice is a snapshot and is not updated on changes.
 func (ptcp Participants) Playing() []*common.Player {
-	res := make([]*common.Player, 0, len(ptcp.playersByUserID))
-	for _, p := range ptcp.playersByUserID {
+	res, original := ptcp.initalizeSliceFromByUserID()
+	for _, p := range original {
 		if p.Team != common.TeamSpectators && p.Team != common.TeamUnassigned {
 			res = append(res, p)
 		}
@@ -189,10 +193,10 @@ func (ptcp Participants) Playing() []*common.Player {
 // TeamMembers returns all players belonging to the requested team at this time.
 // The returned slice is a snapshot and is not updated on changes.
 func (ptcp Participants) TeamMembers(team common.Team) []*common.Player {
-	res := make([]*common.Player, 0, len(ptcp.playersByUserID))
-	for _, ptcp := range ptcp.playersByUserID {
-		if ptcp.Team == team {
-			res = append(res, ptcp)
+	res, original := ptcp.initalizeSliceFromByUserID()
+	for _, p := range original {
+		if p.Team == team {
+			res = append(res, p)
 		}
 	}
 	return res
@@ -209,3 +213,8 @@ func (ptcp Participants) FindByHandle(handle int) *common.Player {
 
 	return ptcp.playersByEntityID[handle&entityHandleIndexMask]
 }
+
+func (ptcp Participants) initalizeSliceFromByUserID() ([]*common.Player, map[int]*common.Player) {
+	byUserID := ptcp.ByUserID()
+	return make([]*common.Player, 0, len(byUserID)), byUserID
+}

game_state_test.go

@@ -6,6 +6,7 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	"github.com/markus-wa/demoinfocs-golang/common"
+	st "github.com/markus-wa/demoinfocs-golang/sendtables"
 )
 
 func TestNewGameState(t *testing.T) {
@@ -41,8 +42,8 @@ func TestGameState_Participants(t *testing.T) {
 	byUserID := ptcp.ByUserID()
 
 	// Should update ptcp as well since it uses the same map
-	gs.playersByEntityID[0] = common.NewPlayer()
-	gs.playersByUserID[0] = common.NewPlayer()
+	gs.playersByEntityID[0] = newPlayer()
+	gs.playersByUserID[0] = newPlayer()
 
 	assert.Equal(t, gs.playersByEntityID, ptcp.ByEntityID())
 	assert.Equal(t, gs.playersByUserID, ptcp.ByUserID())
@@ -54,7 +55,7 @@ func TestGameState_Participants(t *testing.T) {
 
 func TestParticipants_All(t *testing.T) {
 	gs := newGameState()
-	pl := common.NewPlayer()
+	pl := newPlayer()
 	gs.playersByUserID[0] = pl
 
 	allPlayers := gs.Participants().All()
@@ -65,19 +66,19 @@ func TestParticipants_All(t *testing.T) {
 func TestParticipants_Playing(t *testing.T) {
 	gs := newGameState()
 
-	terrorist := common.NewPlayer()
+	terrorist := newPlayer()
 	terrorist.Team = common.TeamTerrorists
 	gs.playersByUserID[0] = terrorist
-	ct := common.NewPlayer()
+	ct := newPlayer()
 	ct.Team = common.TeamCounterTerrorists
 	gs.playersByUserID[1] = ct
-	unassigned := common.NewPlayer()
+	unassigned := newPlayer()
 	unassigned.Team = common.TeamUnassigned
 	gs.playersByUserID[2] = unassigned
-	spectator := common.NewPlayer()
+	spectator := newPlayer()
 	spectator.Team = common.TeamSpectators
 	gs.playersByUserID[3] = spectator
-	def := common.NewPlayer()
+	def := newPlayer()
 	gs.playersByUserID[4] = def
 
 	playing := gs.Participants().Playing()
@@ -89,19 +90,19 @@ func TestParticipants_Playing(t *testing.T) {
 func TestParticipants_TeamMembers(t *testing.T) {
 	gs := newGameState()
 
-	terrorist := common.NewPlayer()
+	terrorist := newPlayer()
 	terrorist.Team = common.TeamTerrorists
 	gs.playersByUserID[0] = terrorist
-	ct := common.NewPlayer()
+	ct := newPlayer()
 	ct.Team = common.TeamCounterTerrorists
 	gs.playersByUserID[1] = ct
-	unassigned := common.NewPlayer()
+	unassigned := newPlayer()
 	unassigned.Team = common.TeamUnassigned
 	gs.playersByUserID[2] = unassigned
-	spectator := common.NewPlayer()
+	spectator := newPlayer()
 	spectator.Team = common.TeamSpectators
 	gs.playersByUserID[3] = spectator
-	def := common.NewPlayer()
+	def := newPlayer()
 	gs.playersByUserID[4] = def
 
 	cts := gs.Participants().TeamMembers(common.TeamCounterTerrorists)
@@ -112,7 +113,7 @@ func TestParticipants_TeamMembers(t *testing.T) {
 func TestParticipants_FindByHandle(t *testing.T) {
 	gs := newGameState()
 
-	pl := common.NewPlayer()
+	pl := newPlayer()
 	pl.Team = common.TeamTerrorists
 	gs.playersByEntityID[3000&entityHandleIndexMask] = pl
 
@@ -124,11 +125,28 @@ func TestParticipants_FindByHandle(t *testing.T) {
 func TestParticipants_FindByHandle_InvalidEntityHandle(t *testing.T) {
 	gs := newGameState()
 
-	pl := common.NewPlayer()
+	pl := newPlayer()
 	pl.Team = common.TeamTerrorists
 	gs.playersByEntityID[invalidEntityHandle&entityHandleIndexMask] = pl
 
 	found := gs.Participants().FindByHandle(invalidEntityHandle)
 
 	assert.Nil(t, found)
 }
+
+func TestParticipants_SuppressNoEntity(t *testing.T) {
+	gs := newGameState()
+	pl := newPlayer()
+	gs.playersByUserID[0] = pl
+	gs.playersByUserID[1] = common.NewPlayer()
+
+	allPlayers := gs.Participants().All()
+
+	assert.ElementsMatch(t, []*common.Player{pl}, allPlayers)
+}
+
+func newPlayer() *common.Player {
+	pl := common.NewPlayer()
+	pl.Entity = new(st.Entity)
+	return pl
+}

parser.go

@@ -10,7 +10,6 @@ import (
 
 	bit "github.com/markus-wa/demoinfocs-golang/bitread"
 	common "github.com/markus-wa/demoinfocs-golang/common"
-	events "github.com/markus-wa/demoinfocs-golang/events"
 	msg "github.com/markus-wa/demoinfocs-golang/msg"
 	st "github.com/markus-wa/demoinfocs-golang/sendtables"
 )
@@ -70,7 +69,7 @@ type Parser struct {
 	gameEventDescs       map[int32]*msg.CSVCMsg_GameEventListDescriptorT // Maps game-event IDs to descriptors
 	grenadeModelIndices  map[int]common.EquipmentElement                 // Used to map model indices to grenades (used for grenade projectiles)
 	stringTables         []*msg.CSVCMsg_CreateStringTable                // Contains all created sendtables, needed when updating them
-	currentFlashEvents   []events.PlayerFlashed                          // Contains flash events that need to be dispatched at the end of a tick
+	delayedEvents        []interface{}                                   // Contains events that need to be dispatched at the end of a tick (e.g. flash events because FlashDuration isn't updated before that)
 }
 
 type bombsite struct {

parsing.go

@@ -247,10 +247,10 @@ var frameParsedToken = new(frameParsedTokenType)
 func (p *Parser) handleFrameParsed(*frameParsedTokenType) {
 	// PlayerFlashed events need to be dispatched at the end of the tick
 	// because Player.FlashDuration is updated after the game-events are parsed.
-	for _, e := range p.currentFlashEvents {
+	for _, e := range p.delayedEvents {
 		p.eventDispatcher.Dispatch(e)
 	}
-	p.currentFlashEvents = p.currentFlashEvents[:0]
+	p.delayedEvents = p.delayedEvents[:0]
 
 	p.currentFrame++
 	p.eventDispatcher.Dispatch(events.TickDone{})