Repository: markus-wa/demoinfocs-golang
Pull request #359 (2022-09-04 11:39:58)

pkg/demoinfocs/common: fix panic in Player.Color() for old demos

codecov[bot]: # [Codecov](https://codecov.io/gh/markus-wa/demoinfocs-golang/pull/359?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Markus) Report
> Merging [#359](https://codecov.io/gh/markus-wa/demoinfocs-golang/pull/359?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Markus) (abc868b) into [master](https://codecov.io/gh/markus-wa/demoinfocs-golang/commit/64c381eeab94b3d7969670e7078a35c467f209b8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Markus) (64c381e) will **increase** coverage by `0.03%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master     #359      +/-   ##
==========================================
+ Coverage   92.19%   92.22%   +0.03%     
==========================================
  Files          32       32              
  Lines        3176     3190      +14     
==========================================
+ Hits         2928     2942      +14     
  Misses        191      191              
  Partials       57       57              
```


| [Impacted Files](https://codecov.io/gh/markus-wa/demoinfocs-golang/pull/359?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Markus) | Coverage Δ | |
|---|---|---|
| [pkg/demoinfocs/common/player.go](https://codecov.io/gh/markus-wa/demoinfocs-golang/pull/359/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Markus#diff-cGtnL2RlbW9pbmZvY3MvY29tbW9uL3BsYXllci5nbw==) | `98.83% <100.00%> (+0.10%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Markus). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Markus)


Patch:
diff --git a/pkg/demoinfocs/common/player.go b/pkg/demoinfocs/common/player.go
index 79ac354..2b24bcf 100644
--- a/pkg/demoinfocs/common/player.go
+++ b/pkg/demoinfocs/common/player.go
@@ -5,6 +5,7 @@ import (
 	"time"
 
 	"github.com/golang/geo/r3"
+	"github.com/pkg/errors"
 
 	"github.com/markus-wa/demoinfocs-golang/v3/pkg/demoinfocs/constants"
 	st "github.com/markus-wa/demoinfocs-golang/v3/pkg/demoinfocs/sendtables"
@@ -343,20 +344,24 @@ func (pf PlayerFlags) OnGround() bool {
 }
 
 // Ducking returns true if the player is/was fully crouched.
-//   Fully ducked: Ducking() && DuckingKeyPressed()
-//   Previously fully ducked, unducking in progress: Ducking() && !DuckingKeyPressed()
-//   Fully unducked: !Ducking() && !DuckingKeyPressed()
-//   Previously fully unducked, ducking in progress: !Ducking() && DuckingKeyPressed()
+//
+//	Fully ducked: Ducking() && DuckingKeyPressed()
+//	Previously fully ducked, unducking in progress: Ducking() && !DuckingKeyPressed()
+//	Fully unducked: !Ducking() && !DuckingKeyPressed()
+//	Previously fully unducked, ducking in progress: !Ducking() && DuckingKeyPressed()
+//
 // See m_fFlags FL_DUCKING https://github.com/ValveSoftware/source-sdk-2013/blob/master/mp/src/public/const.h#L146-L188
 func (pf PlayerFlags) Ducking() bool {
 	return pf.Get(flDucking)
 }
 
 // DuckingKeyPressed returns true if the player is holding the crouch key pressed.
-//   Fully ducked: Ducking() && DuckingKeyPressed()
-//   Previously fully ducked, unducking in progress: Ducking() && !DuckingKeyPressed()
-//   Fully unducked: !Ducking() && !DuckingKeyPressed()
-//   Previously fully unducked, ducking in progress: !Ducking() && DuckingKeyPressed()
+//
+//	Fully ducked: Ducking() && DuckingKeyPressed()
+//	Previously fully ducked, unducking in progress: Ducking() && !DuckingKeyPressed()
+//	Fully unducked: !Ducking() && !DuckingKeyPressed()
+//	Previously fully unducked, ducking in progress: !Ducking() && DuckingKeyPressed()
+//
 // See m_fFlags FL_ANIMDUCKING https://github.com/ValveSoftware/source-sdk-2013/blob/master/mp/src/public/const.h#L146-L188
 func (pf PlayerFlags) DuckingKeyPressed() bool {
 	return pf.Get(flAnimDucking)
@@ -406,9 +411,43 @@ func (p *Player) Score() int {
 	return getInt(p.resourceEntity(), "m_iScore."+p.entityIDStr())
 }
 
-// Color returns the players color as shown on the match.
+// Color returns the players color as shown on the minimap.
+// It will return Grey (-1) if the resource entity does not exist when the function is called or when the demo does not support player colors.
+// Deprecated: Use ColorOrErr() instead.
 func (p *Player) Color() Color {
-	return Color(getInt(p.resourceEntity(), "m_iCompTeammateColor."+p.entityIDStr()))
+	resourceEnt := p.resourceEntity()
+	if resourceEnt == nil {
+		return Grey
+	}
+
+	n, ok := resourceEnt.PropertyValue("m_iCompTeammateColor." + p.entityIDStr())
+	if !ok {
+		return Grey
+	}
+
+	return Color(n.IntVal)
+}
+
+var (
+	ErrDataNotAvailable   = errors.New("some data is not (yet) available (reading the same data later during parsing may work)")
+	ErrNotSupportedByDemo = errors.New("this data is not supported by the demo (this may be because the demos is too old)")
+)
+
+// ColorOrErr returns the players color as shown on the minimap.
+// Returns ErrDataNotAvailable if the resource entity does not exist (it may exist later during parsing).
+// Returns ErrNotSupportedByDemo if the demo does not support player colors (e.g. very old demos).
+func (p *Player) ColorOrErr() (Color, error) {
+	resourceEnt := p.resourceEntity()
+	if resourceEnt == nil {
+		return Grey, errors.Wrap(ErrDataNotAvailable, "player resource entity is nil")
+	}
+
+	colorVal, ok := resourceEnt.PropertyValue("m_iCompTeammateColor." + p.entityIDStr())
+	if !ok {
+		return Grey, errors.Wrap(ErrNotSupportedByDemo, "failed to get player color from resource entity")
+	}
+
+	return Color(colorVal.IntVal), nil
 }
 
 // Kills returns the amount of kills the player has as shown on the scoreboard.


Test patch:
diff --git a/pkg/demoinfocs/common/common_test.go b/pkg/demoinfocs/common/common_test.go
index 1abd88b..0daca05 100644
--- a/pkg/demoinfocs/common/common_test.go
+++ b/pkg/demoinfocs/common/common_test.go
@@ -220,3 +220,13 @@ func entityWithProperty(propName string, value st.PropertyValue) *stfake.Entity
 
 	return entity
 }
+
+func entityWithoutProperty(propName string) *stfake.Entity {
+	entity := entityWithID(1)
+
+	entity.On("Property", propName).Return(nil)
+	entity.On("PropertyValue", propName).Return(st.PropertyValue{}, false)
+	entity.On("PropertyValueMust", propName).Panic("property not found")
+
+	return entity
+}
diff --git a/pkg/demoinfocs/common/player_test.go b/pkg/demoinfocs/common/player_test.go
index f97d456..b9b264a 100644
--- a/pkg/demoinfocs/common/player_test.go
+++ b/pkg/demoinfocs/common/player_test.go
@@ -422,6 +422,44 @@ func TestPlayer_Color(t *testing.T) {
 	pl := playerWithResourceProperty("m_iCompTeammateColor", st.PropertyValue{IntVal: int(Yellow)})
 
 	assert.Equal(t, Yellow, pl.Color())
+
+	pl = &Player{demoInfoProvider: demoInfoProviderMock{}}
+
+	assert.Equal(t, Grey, pl.Color())
+
+	pl = &Player{
+		EntityID: 1,
+		demoInfoProvider: demoInfoProviderMock{
+			playerResourceEntity: entityWithoutProperty("m_iCompTeammateColor.001"),
+		},
+	}
+
+	assert.Equal(t, Grey, pl.Color())
+}
+
+func TestPlayer_ColorOrErr(t *testing.T) {
+	pl := playerWithResourceProperty("m_iCompTeammateColor", st.PropertyValue{IntVal: int(Yellow)})
+
+	color, err := pl.ColorOrErr()
+	assert.NoError(t, err)
+	assert.Equal(t, Yellow, color)
+
+	pl = &Player{demoInfoProvider: demoInfoProviderMock{}}
+
+	color, err = pl.ColorOrErr()
+	assert.ErrorIs(t, err, ErrDataNotAvailable)
+	assert.EqualValues(t, Grey, color)
+
+	pl = &Player{
+		EntityID: 1,
+		demoInfoProvider: demoInfoProviderMock{
+			playerResourceEntity: entityWithoutProperty("m_iCompTeammateColor.001"),
+		},
+	}
+
+	color, err = pl.ColorOrErr()
+	assert.ErrorIs(t, err, ErrNotSupportedByDemo)
+	assert.Equal(t, Grey, color)
 }
 
 func TestPlayer_Kills(t *testing.T) {