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

Text ui config #1406

Merged
merged 3 commits into from Aug 21, 2019

Conversation

@avanwinkle
Copy link
Collaborator

commented Aug 14, 2019

@jabdoa2 refactored the TextUi as part of #1339, which improved the stability of the screen and provided a bunch more information (hurray!). Unfortunately for machines with 100+ machine and player variables, the overhead to maintain the UI had negative impacts on performance.

This PR introduces a config section text_ui: that allows a user to configure which machine_vars and player_vars appear in the text UI. If unspecified, all will appear.

If approved, I'll update the docs to reflect the new options. Feedback/suggestions welcome!

Anthony van Winkle added 2 commits Aug 14, 2019
@jabdoa2

This comment has been minimized.

Copy link
Collaborator

commented Aug 14, 2019

We should have a look why this hits us so hard performance wise. I already fixed a few issues upstream to improve performance.

@avanwinkle

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 16, 2019

I can't speak much to the performance, but there's also the benefit of being able to see relevant information. With 100+ variables they will never all fit on the screen, and the machine vars are cluttered with data that's not useful in many cases. Being able to customize the output seems like an easy win, and the performance boost is a freebie :)

@jabdoa2

This comment has been minimized.

Copy link
Collaborator

commented Aug 17, 2019

Yeah makes sense! Feel free to merge.

@avanwinkle avanwinkle merged commit 3bd4bfb into missionpinball:dev Aug 21, 2019

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@avanwinkle avanwinkle deleted the avanwinkle:text-ui-config branch Aug 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.