Skip to content
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

Add check in RGBColor constructor to account for integer colors #1609

Merged
merged 3 commits into from
Nov 24, 2021

Conversation

seanirby
Copy link
Member

I ran into this issue while trying to add some particular colors to my named_colors configuration. Values such as 990000 will be interpreted by MPFs YamlInterface as an integer rather than a string and will cause the following error when an RGBColor is instantiated with an integer value:

Traceback (most recent call last):
  File "/home/sean/dev/mpf/mpf/tests/test_RGBColor.py", line 34, in test_int_color
    color = RGBColor(990000)
  File "/home/sean/dev/mpf/mpf/core/rgb_color.py", line 181, in __init__
    self._color = (color[0], color[1], color[2])
TypeError: 'int' object is not subscriptable

Added a check in the constructor of RGBColor to fix this particular issue.

@sonarcloud
Copy link

sonarcloud bot commented Oct 17, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
1.4% 1.4% Duplication

@jabdoa2
Copy link
Collaborator

jabdoa2 commented Oct 17, 2021

Ideally you quote colors. We had similar issues with octal encoding such as "011111". Guess this is still a stop gap solution for some cases. Maybe we should warn users? Or raise a nice error?

@seanirby
Copy link
Member Author

Ideally you quote colors. We had similar issues with octal encoding such as "011111". Guess this is still a stop gap solution for some cases. Maybe we should warn users? Or raise a nice error?

Ah OK. The doc section I was referencing wasn't quoting the colors so that threw me off: https://docs.missionpinball.org/en/0.54/config/named_colors.html

I'll update to raise an error and also update that doc section.

@jabdoa2
Copy link
Collaborator

jabdoa2 commented Oct 18, 2021

Thanks for taking care!

@jabdoa2 jabdoa2 changed the title Add check in RGBColor constructor to account for integer colors [wip] Add check in RGBColor constructor to account for integer colors Nov 7, 2021
@seanirby
Copy link
Member Author

Updated to output an error message instead. Sorry for the delay @jabdoa2, forgot about this one.

@sonarcloud
Copy link

sonarcloud bot commented Nov 24, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
1.4% 1.4% Duplication

@jabdoa2
Copy link
Collaborator

jabdoa2 commented Nov 24, 2021

Awesome. Thanks!

@jabdoa2 jabdoa2 changed the title [wip] Add check in RGBColor constructor to account for integer colors Add check in RGBColor constructor to account for integer colors Nov 24, 2021
@jabdoa2 jabdoa2 merged commit 5128b91 into missionpinball:dev Nov 24, 2021
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