Adds MVP info #226
Adds MVP info #226
Conversation
There was a problem hiding this comment.
This already looks pretty great.
I just have a couple fairly small comments.
It would also be good if you could add test coverage to the parts you added on the python side.
Like checking the MVP scores in the stats module (download the reference demo and double check what you get with what is shown in the game and on hltv and then add that as a test).
You should also add a small test that the stats function works correctly if the MVPName and/or steamid are None or a bot is the mvp.
I think there should already be some examples that you can have a look at in that regard in the stats test file https://github.com/pnxenopoulos/awpy/blob/main/tests/test_stats.py but if you have any questions just ask.
awpy/analytics/stats.py
Outdated
| @@ -383,6 +384,7 @@ def player_stats( | |||
| for player, n_kills in round_kills.items(): | |||
| if n_kills in range(6): # 0, 1, 2, 3, 4, 5 | |||
| player_statistics[player][f"kills{n_kills}"] += 1 # type: ignore[literal-required] | |||
| player_statistics[str(r["MVPSteamID"])]["mvp"] += 1 | |||
There was a problem hiding this comment.
Would be good if you could use the same logic here that we use to check for the steamids in the other places here.
Like
thrower_key = (
g["throwerName"]
if g["throwerSteamID"] == 0
else str(g["throwerSteamID"])
)
here:
Line 353 in 9e69073
This is to handle bots.
awpy/types.py
Outdated
| @@ -482,6 +482,8 @@ class GameRound(TypedDict): | |||
| weaponFires: Optional[list[WeaponFireAction]] | |||
| flashes: Optional[list[FlashAction]] | |||
| frames: Optional[list[GameFrame]] | |||
| mvp: Players | |||
There was a problem hiding this comment.
I dont think the gameRounds dict contains a Players dict for the key mvp from your implementation.
I think it should have an MVPSteamID: Optional[int] and the same for MVPName as optional string.
awpy/parser/parse_demo.go
Outdated
| @@ -129,6 +129,9 @@ type GameRound struct { | |||
| WinningTeam *string `json:"winningTeam"` | |||
| LosingTeam *string `json:"losingTeam"` | |||
| Reason string `json:"roundEndReason"` | |||
| MVPName string `json:"MVPName"` | |||
There was a problem hiding this comment.
I think according to the demoinfocs-golang documentation the player can be nil and so we should handle this like we do the other cases where that can happen.
https://github.com/pnxenopoulos/awpy/blob/main/awpy/parser/parse_demo.go#L205
|
@agsalguero @JanEricNitschke let's go forward with this PR? 😄 |
|
@ricardoruwer There are still the open points i have mentioned + the PR has to be rebased to the current main. Sadly i dont have the time to do so at the moment. But feel free to fork their repo and finish the PR from that. |
|
I'll have some time in a week or two. I'll take a look to it. |
agsalguero
force-pushed
the
main
branch
2 times, most recently
from
9e69073 to
92ad20a
Compare
Fix #221