Skip to content

Add dynamic uniform type detection via colors/teams.json#732

Open
jc214809 wants to merge 3 commits intoMLB-LED-Scoreboard:devfrom
jc214809:feature/custom-uniform-colors
Open

Add dynamic uniform type detection via colors/teams.json#732
jc214809 wants to merge 3 commits intoMLB-LED-Scoreboard:devfrom
jc214809:feature/custom-uniform-colors

Conversation

@jc214809
Copy link
Copy Markdown

Any non-standard key in a team's color object (beyond home/text/accent) is
automatically registered as a uniform type. The MLB API detection string is
derived from the key name (snake_case -> Title Case) with case-insensitive
matching, so city_connect matches "City Connect 2.0 Jersey" and cincy matches
"CINCY Jersey" without any hardcoding. Adds Reds CINCY alternate jersey colors
to colors/teams.example.json and updates schema to allow arbitrary uniform
type sub-objects.

Any non-standard key in a team's color object (beyond home/text/accent) is
automatically registered as a uniform type. The MLB API detection string is
derived from the key name (snake_case -> Title Case) with case-insensitive
matching, so city_connect matches "City Connect 2.0 Jersey" and cincy matches
"CINCY Jersey" without any hardcoding. Adds Reds CINCY alternate jersey colors
to colors/teams.example.json and updates schema to allow arbitrary uniform
type sub-objects. Also adds .claude/ to .gitignore.
Copy link
Copy Markdown
Contributor

@ty-porter ty-porter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great feature and future-proofs new team color additions. I think we need to make sure it'll work more holistically with our existing tooling.

I left some suggestions to cover that.

Comment thread validate_config.py
return MODIFIER + mod


def _colors_ignored_keys():
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This subverts how the validator works and seems fairly flaky -- this validator is complicated so I'm worried that changing how it works based on user input is a potential minefield. There's also an upsert feature which checks if there's a missing key and adds it if it's not there, and removes things that aren't needed any more. For better or worse, it's one of the only ways we have to automatically migrate configs when versions change.

The special ignore_keys setting overrides both of those migration features, so this is a little unsafe.


What if we added a dict of special uniforms:

"cin": { 
  "home": {
    "r": 1,
    "g": 2,
    "b": 3
  },
  "text": {
    "r": 1,
    "g": 2,
    "b": 3
  },
  "accent": {
    "r": 1,
    "g": 2,
    "b": 3
  },
  "special_uniforms": {
    "city_connect": {
      "r": 1,
      "g": 2,
      "b": 3
    },
    "cincy": {
      "r": 1,
      "g": 2,
      "b": 3
    }
  }
}

What this lets us do is mark special_uniforms unsafe:

"ignored_keys": [
  "special_uniforms" + make_modifier(IGNORE_SUBPATHS),
  "plugins" + make_modifier(IGNORE_SUBPATHS)
]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this strategy will expand the scope of this PR to touch every single team's colors entry and is also a breaking change, anyone with an existing custom city_connect setup won't get migrated automatically.

I'm personally OK with breaking those custom colors

Comment thread colors/teams.example.json
"b": 237
}
},
"cincy": {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will probably have to get added to the schema as a default value

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants