v1.0.7 (Rework keyboard) #11
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ui_keyboard_controller lacks some typing information because the relevant libraries won't be imported on all platforms. Changes: -Handle key presses, releases, listener setup, etc. from a new module, ui_keyboard_controller, so that ui_controller doesn't become cluttered with platform-dependent variables and library calls. The new module is a wrapper that keeps things abstract and simple in the controller. -Rename _handle_hotkey_press and _execute_split_action to _react_to_hotkey_flags and _react_to_split_flags, respectively, to avoid confusion.
Changes: -Don't handle keys in _handle_key_press if they have a key code but no name in pynput -- this can lead to hotkeys that are assigned but look blank to the user -Before setting a hotkey to "pressed" in _handle_key_press, make sure the key name AND code match. Sometimes key codes can match even when names don't, leading to secret hotkeys being unexpectedly created (I'm looking at you, MacOS) -Automatically resize hotkey names in the settings menu if they are wider than the text box. To accomplish this, add a couple helper methods to KeyLineEdit --Rename PREVIOUS_HOTKEY_NAME and PREVIOUS_HOTKEY_CODE to PREV_HOTKEY_NAME and PREV_HOTKEY_CODE, respectively, for brevity
FPS settings appeared to have no logical effect on the rate that _poll was called in ui_controller. Turns out there was a logic error forcing _poll to repeat at the interval from the most recent settings change, not the actual setting. That is now fixed.
Make caffeinate calls in their own dedicated thread on MacOS since the calls take up a decent amount of time, and the overhead needed to generate a single thread is very small.
Previously, if the user was typing very quickly with a hotkey box in focus in the settings menu, a segmentation fault could rarely occur. This has been fixed: _handle_hotkey_press now sets flags instead of modifying the GUI directly (something it has done for a long time to press hotkeys, but didn't seem necessary for simply updating hotkey values). A lock is used to avoid racing when updating the box.
-Remove test_init as it was useless -Finish test_run to ensure threads are joined successfully -Rename self.gui to self.app in pilgrim_autosplitter.py -Join ui_keyboard threads when app closes -Use pytest instead of unittest for unit tests
Switch to using the QApplication.aboutToQuit signal to call splitter .safe_exit_all_threads so it is called more consistently before the app exits. Also remove the run function to simplify things.
This is mostly for testing purposes, to make it easier to test settings changes using dummy settings. This commit also cleans up the code around aspect ratio and width/ height adjustments in set_defaults to remove some redundancy.
-Make set_next_capture_index return found_valid_source to make it easy to check whether a new source was found or not -Use _compare_thread.is_alive instead of (match_percent is None) to decide what to do in toggle_suspended. It does the same thing and is much easier to understand (and is less brittle) -Break getting max fps into its own method, _get_max_fps -Break waiting on the interval in _compare and _capture into its own method, wait_for_interval
For now, it's not needed, since there is no advantage to stopping the listener from pilgrim_autosplitter.py -- the listener never causes segfaults and is killed automatically on exit because it's a daemon. Maybe someday I'll want this, but for now it's pointless.
This PR adds some unit tests for settings.py, splitter.py, and pilgrim_autosplitter.py. This whole project should have unit tests but I have limited time to devote to this right now, so it's not a priority. Changes: -pytest added to requirements.txt and requirements-linux.txt -pilgrim_autosplitter.py rewritten for clarity and neatness -settings.py refactored to allow using different QSettings in functions for testing purposes -some methods in splitter.py refactored and/or split into multiple methods for testing purposes
- Rename settings.set_defaults to settings.set_program_vals for clarity - In settings.set_program_vals, check if last-used version was earlier than v1.0.7 and unset hotkeys if so. This is to prevent a bug related to the change in hotkey implementation in v1.0.7. - Edits to settings.py for clarity and brevity - Add unit test for settings.version_ge - Change checking for updates in ui_main_window.py so it checks if there is a new version that is greater than the current version available (makes life easier when developing a new version, and is cleaner)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR shifts the code handling keyboard listener/controller backends from ui_controller to a new module called ui_keyboard_controller. This is done to keep keyboard handling in ui_controller abstract, and makes it easier to handle the complexities of using two different backends depending on the platform.
In addition, this PR adds some unit tests for settings.py, splitter.py, and pilgrim_autosplitter.py. This whole project should have unit tests but I have limited time to devote to this right now, so it's not a priority.
It also fixes several bugs, most related to hotkey implementation:
Other misc. changes/updates: