Skip to content

Player Parser Refactor Pt1 #9

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

Merged
merged 4 commits into from
Jan 8, 2022
Merged

Player Parser Refactor Pt1 #9

merged 4 commits into from
Jan 8, 2022

Conversation

vanceism7
Copy link
Collaborator

Beginning of refactoring player.py. It's not a very impressive refactor, we just pull a big try statement out and stick it in it's own function. This PR is WIP - if the refactor is correct, the function should still be given a more appropriate name, and we should add a doc string as well before merging. We could also continue to add more refactorings in this branch before merging if we'd prefer to not have a ton of PRs; just wanted to start small so its easy to review changes

Let me know what you think! Maybe you can enlighten me on what exactly we're doing here. Seems like it has something to do with when playback hits the end of the loop/file or something of that sort.

Vance Palacio added 2 commits January 6, 2022 19:51
Don't really know what this function does, but managed to separate it
out from the main `run` function. This should be renamed once it's
intent is clarified
@vanceism7 vanceism7 requested a review from flipcoder January 7, 2022 02:53
@flipcoder
Copy link
Owner

That block of code mainly processes things that need to happen before the prompt shows again in midi shell mode as well as the shell itself. So that involves finishing pending commands and arpeggios, as well as allowing looping. Maybe I should go through and comment that more and we can figure out the best way to break it up. Maybe we could break it into 2 functions, process_pending() and show_prompt()?

@flipcoder flipcoder merged commit c07a6a7 into develop Jan 8, 2022
@vanceism7 vanceism7 deleted the vjp/refactor-player-pt1 branch January 13, 2022 02:26
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