worldpeace-germany
commented
2 months ago In regards to the last commit fixed and clean up of color handling in segment displays
In general there are some issues when defining segment display shows due to the fact how colors are being handled and how that is being known to user and if that is perceived as a logical definition flow.
In segment display the default color (white) is used
to expand the given color array to the right until the length of the segment display is being filled up. So for a 4 digit display, specified as default color "blue, yellow" this would end up as "blue, yellow, white, white".
During initlize an empty string is sent to the segment display
, each string is colored
Since the length is 0, the left_pad_color is used to set the color. So it is not the default color being used, the color set now for this example is "blue, blue, blue, blue". Which on the first view doesn't matter since the display is blank, but at later points in time the code prefers to use the current_state color and not the default_color if no color is being sent, for example
if the other digits cannot display blue they will simply stay blank.
In other parts of the code, if you start a transition the given colors are being expanded
https://github.com/missionpinball/mpf/blob/35a84ba29c54acc6ce1afab28bb0e9edca07e301/mpf/devices/segment_display/segment_display.py#L186, where expanded could as well mean shortend to the length of the text and not the length of the display, so later the empty places are being padded with a color again (see above). If the colors need to be expanded then this
is done by taking the most right color and reuse it for all the other text to the right of this position (so no default color being used).
If a transition text is being used and no color is specified, then the color of the first character of the new text is being used (again not the default color).
In general we have default color, sometimes padded to the left and sometimes padded to the right. Question is if the user can follow that behavior or if it is over complicated and thus introduces user mistakes.
The PR will change some of the behavior to make it more consistent
- unchanged is that rather the current_state is used and not the default_color
- changed is that if color information is missing that the as the default color white will be used, padded to the left, that ensure that at least something is
visible and not that because of a wrong color invisible text is shown and confuses the user. But the PR not only makes the handling more consistent but I would consider it a bug if a color is used for padding which might be able to be used for the digits of that segment display.
Once the PR is accepted I will update the documentation to explain the color handling in detail for the users.
sonarcloud[bot]
config_spec.yaml: changed default from
not_settooff, leftnot_setin enum though I believe it can be removed. I couldn't see why not_set is a good default. If really needed for the changes in update_methodreplaceI can add code to exchangenot_setforoff.segment_display: Only changed code for update_method
replace,stackshould behave like before. I tested with various segment displays shows and it seems to behave well. Note:replacethepriorityparameter is not used (and doesn't make sense if you don't have a stack)replace, but completely misbehave if usingstackreplacein here https://github.com/missionpinball/mpf/blob/35a84ba29c54acc6ce1afab28bb0e9edca07e301/mpf/devices/segment_display/segment_display.py#L139colorchanges since my display is single color (though would believe it should work if it works forstack)flashing,transition,transition_out,expireparameter tested (and working)If the PR gets accepted I would do some additons to mpf-docs 0.80. Just would need to know how to contribute version specific changes.