-
Notifications
You must be signed in to change notification settings - Fork 167
Show countable stats in logger #484
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…re still 0, since log stat happens at different periods
|
One problem with all this is that the play-by-play texts look ugly for scoring plays, having to repeat the scoring player's name to go along with PTS. Back before I split those up into two events (shot and then make/miss, rather than all in one event) it would have been easier. But the split shooting events add some nice drama, so I like them. And for some prior art... ESPN doesn't include counts in their NBA or MLB play by plays. nba.com is probably what inspired you, since they look similar (albeit they don't have the split shot/result events, so they don't have the same problem): mlb.com includes more descriptive text, more similar to BBGM, and includes stats inline as just a number in parentheses. Simpler than the nba.com format, and maybe would look better in BBGM than including the stat like (6 PTS). But that still doesn't help with the split shot/result events.
This may depend on whether
What do you mean by "event system"? Like an event emitter that would send individual events to the UI as they happen? Or something else? What I would really like is to have one PlayByPlayLogger class for all sports, that would be very nice. |
|
We could convert the PlayByPlayLogger.ts into an abstract, and then override each function that differs between sports? I think that could be a TODO item, and then also convert event types, similar to what the refactor logger PR is doing |
|
Also! Another solution/idea I have to solve this, is what if in the player name, it creates a popover with their stats at that current moment? This is kinda of a shitty idea though since:
Maybe one thing that can be done is to reword the "Its good" text event to include the player's name in a more organic manner? e.g "playerName swishes it!"?
Yea, I meant an event Emitter which could send out events. But then again, this would probably be way too much work for this specific use case, as it would have to refactor a lot of different files just to be able to use this. Also, might ruin the ability to run offline, based on what type of event emitter system is being used. |
Line 608 in 1f2c3ff
A lot of it could be shared logic in a generic class, with the types for each sport passed in as the generic parameter.
I don't like that idea because events happen so fast that hardly anyone would use it, I think. How about this... For most things, do a minimal inline number right after both the stat and name are mentioned, regardless of order. Only include the stat if it's not explicitly mentioned. Like:
For made shots, could be like:
Then with an assist, a little ugly maybe:
I think that's a decent blend of conciseness (not repeating the stat every time) and clarity (putting the stat abbreviation there when it might be unclear).
Main problem would be that it'd require refactoring the game simulation code, since currently it sims the entire game before sending anything to the UI. I probably should have done it that way in the first place, since now I have to go through extra effort to do things like hiding notifications/results of games while you're watching the live sim. It would also allow letting the user make decisions live (like substitutions or whatever) - not sure I want to go down that path, but a lot of people would like it. But I guess now, since the hacks to hide game results are already in place, there is not a great reason to do all that refactoring work. Although if you want to try you are welcome, since it would be better! |
…g with foul logs and tov/stl logs and asts
Reworked it to look like this (updated the screenshots), and I like this style a lot actually. Its very concise and not very invasive. I could see some people being confused by numbers that don't have a stat tag associated with it at first though (especially for rebounds. Rebounds are seperated by orb and drb, may be more clear to put a tag, or just make it so its the total) |
dumbmatter
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides that one comment, the next question is, what should be done for other sports? And do you want to give that a try? As always, just let me know if you'd rather have me take it from here.
Off the top of my head for the other sports...
- football - show season totals (not game totals) for TD, INT, FMB. But they are usually two players recording stats, so might be tricky to show in the text, idk.
- baseball - show season totals for the somewhat rare/important stats only: 2B, 3B, HR, SB. Maybe also strikeouts for pitchers, but those might be better to be game totals, since there are a lot more strikeouts than any of those other stats.
- hockey - show season totals for goals and assists
I think mostly we want to show season totals, since a lot of the important events (interceptions, goals, home runs) are rare, so showing the game total would usually just be showing "1".
For implementing season totals, look at how seasonStats is populated by seasonStatsKeys for baseball only. That's used to show the batting average, ERA, etc. for the whole season for a player in the box score. That already contains most of the season stats we need, and it could be added for hockey/football too. Ideally it would be deleted when saving the final box score for hockey/football, because otherwise it'd just take up extra disk space, since it's only used for the play-by-play.
I can take a look at the other sports! But just warning it might be abit slow for me sine I am getting a bit busy with work recently, but I will try and help where I can 😄. I also want to try and unify the logger together, since I don't think it should be a big level of effort (but I might be very mistaken) |
So just to keep my thoughts in a row, in baseball there exists |
I mean when it's saved to disk in writeGameStats. Currently |
Im not too sure about mixing both season and game stats together, I feel as if this will confuse players? But im not as well versed in what are important stats for MLB... Also side thought for football stats: what if we put fantasy points instead? it would be cool, put idk how useful it would be, and how hard it would be to do this idea |
You could be right. If you look on mlb.com they show season totals for 2B/3B/HR/SB, but nothing for strikeouts. Personally if I was making a list of the most important numeric stats to show in the gamelog, I'd probably rank strikeouts (this game) ahead of 2B, but that's just my opinion. No big deal either way.
Might be too noisy, because they update on every play where yardage is gained/lost (so, most plays), sometimes for multiple players. |
dumbmatter
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides those two little comments, it looks good. I didn't test it to see if it actually works!
| this.playByPlay.logEvent({ | ||
| type: "strikeOut", | ||
| swinging, | ||
| pid: batter.id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be number of strikeouts that the pitcher has so far this game, not the batter. That's the soPit stat above, rather than so. Also in processLiveGameEvents.ts, sportState already includes the pid of the pitcher, so it doesn't need to be added to this event either. (I guess it's also in the event, from the next line below this comment, so that could also be used)
| text = `${getName( | ||
| event.pid, | ||
| )} beats the throw and is safe at ${getBaseName(event.to)}`; | ||
| )} beats the throw and is safe at ${getBaseName(event.to)} (${playersByPid[event.pid].seasonStats["sb"]})`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will fail because "sb" needs to be added to seasonStatsKeys in loatTeams.ts
|
I think I may have given you some bad advice above! I said you can get the current value of a stat from the box score, so it doesn't need to be included in the event sent from the worker. But I think that will not work for the scoring summary, since I think that shows the same text as the play-by-play but also is dynamically generated, so it'll have the wrong values. In which case the raw events need to include the stat values, right? |
|
Hey! sorry just returning to this after awhile, but quick question in regards to season stats, is it a good idea to pass in the season stats through an event? I do not think that the number of doubles/triples is updated until after a game finishes? |
|
I think the most straightforward way would be for the individual events (like "home run" or whatever) to include the current total, including the current event. So add the number so far this game to the season total, and pass that along in the event. This means the UI doesn't have to worry about combining data from multiple places (sorry for leading you down that path!) and also it simplifies the scoring log (which is based on these same events). |



Started another branch, built off the logger refactor. Related to #483.



Edit: added image for season stats for baseball, with how it would look for strikeouts