MLB-LED-Scoreboard / mlb-led-scoreboard

An LED scoreboard for Major League Baseball :baseball:
GNU General Public License v3.0
590 stars 105 forks source link

Added Pitch Count #407

Closed tracerrx closed 1 year ago

tracerrx commented 2 years ago

Added pitch count to the pitcher scroll. Set show_pitch_count to true/false in config.json to enable/disable

WardBrian commented 2 years ago

I think we should avoid using config inside the game data. We should just fetch it always and use config in the display side

ty-porter commented 2 years ago

Agreed, Game objects are data containers. Turning config on or off should swap display options, not what the underlying data includes (unless there's a good reason for that)

WardBrian commented 2 years ago

Storing it alongside balls/strikes/pitch speed also allows easier control over where the user would like it to be. For example, I think I'd want it as a separate text element rather than part of the pitcher's name

tracerrx commented 2 years ago

I think this moves it to the right place

WardBrian commented 2 years ago

How do you feel about this setting being in the layout rather than the config?

ty-porter commented 2 years ago

Correct me if I'm wrong... The reason it's a config setting is because we want it to scroll with the existing pitcher text?

So our options are:

WardBrian commented 2 years ago

I wouldn’t want it on the pitcher text I think, I would want it like the speeds (probably under them, on my layout), but it could be an “off” “pitcher name” “separate” setting in the layout

e.g. just ignore the x and y if it’s set to “name”

ty-porter commented 2 years ago

I wouldn’t want it on the pitcher text I think, I would want it like the speeds (probably under them, on my layout), but it could be an “off” “pitcher name” “separate” setting in the layout

e.g. just ignore the x and y if it’s set to “name”

I see, it's been a while since I've written any new layouts. I think this is a good approach.

@tracerrx Does this change make sense to you?

tracerrx commented 2 years ago

I understand the logic of having it an independent and being able to move it around the layout... But wouldn't that prohibit it from being part of the pitcher scroll?

WardBrian commented 2 years ago

I don’t think so, you should be able to append it right before rendering.

I can sketch this out in my own fork if it’s helpful, you already did the hard part of finding where the data lives in the API

tracerrx commented 2 years ago

So in layout file, under atbat something like "pitch_count": { "font_name": "4x6", "x": 1, "y": 50, "enabled": true, "append_pitcher_Name": true },

And if append pitch name==true, don't render separately but append to the pitcher name.

WardBrian commented 2 years ago

That sounds good to me

tracerrx commented 1 year ago

Agreed.. Not sure how that got there… must have autocompleted in my ide

On Sep 1, 2022 at 9:45:34 PM, Tyler Porter @.***> wrote:

@.**** commented on this pull request.

In renderers/games/game.py https://github.com/MLB-LED-Scoreboard/mlb-led-scoreboard/pull/407#discussion_r961221305 :

@@ -3,6 +3,7 @@ except ImportError: from RGBMatrixEmulator import graphics

+from distutils import config

This import is not needed, probably breaks some stuff

— Reply to this email directly, view it on GitHub https://github.com/MLB-LED-Scoreboard/mlb-led-scoreboard/pull/407#pullrequestreview-1094221689, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJTDESWR4RQLWUDMZWXRG3V4FL35ANCNFSM6AAAAAAQCSQJOY . You are receiving this because you authored the thread.Message ID: @.*** com>

tracerrx commented 1 year ago

Sure.. I prob can’t get around to it until Tuesday though

On Sep 1, 2022 at 9:24:24 PM, Brian Ward @.***> wrote:

How do you feel about this setting being in the layout rather than the config?

— Reply to this email directly, view it on GitHub https://github.com/MLB-LED-Scoreboard/mlb-led-scoreboard/pull/407#issuecomment-1234963099, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJTDEVBLXCBAXQIQTQRCWTV4FJMRANCNFSM6AAAAAAQCSQJOY . You are receiving this because you authored the thread.Message ID: @.***>

pjockey commented 1 year ago

What are the variables that need to changed in order to see the pitch count? I tried some of the above and not the board loops at start up.

WardBrian commented 1 year ago

431 implemented this feature in a way that only uses the coordinates file, not the config. You can see the example here:

https://github.com/MLB-LED-Scoreboard/mlb-led-scoreboard/blob/0fee496dfa68232ab07daba329f899f70aeada50/coordinates/w64h32.json.example#L135-L140