From c1a1a0139c1566570445c7370fdffbec3c141d67 Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Mon, 29 Jul 2024 15:36:54 -0600 Subject: [PATCH 01/22] Split keyboard calls into separate module 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. --- src/ui/ui_controller.py | 88 ++++++------------- src/ui/ui_keyboard_controller.py | 140 +++++++++++++++++++++++++++++++ 2 files changed, 166 insertions(+), 62 deletions(-) create mode 100644 src/ui/ui_keyboard_controller.py diff --git a/src/ui/ui_controller.py b/src/ui/ui_controller.py index 6123aa1..e522fe0 100644 --- a/src/ui/ui_controller.py +++ b/src/ui/ui_controller.py @@ -48,16 +48,10 @@ import settings import ui.ui_style_sheet as style_sheet from splitter.splitter import Splitter +from ui.ui_keyboard_controller import UIKeyboardController from ui.ui_main_window import UIMainWindow from ui.ui_settings_window import UISettingsWindow -if platform.system() == "Windows" or platform.system() == "Darwin": - # Don't import the whole pynput library since that takes a while - from pynput import keyboard as pynput_keyboard -else: - # Pynput doesn't work well on Linux, so use keyboard instead - import keyboard - class UIController: """Manage the passing of information from the splitter to the UI, and from @@ -129,6 +123,8 @@ def __init__(self, application: QApplication, splitter: Splitter) -> None: if platform.system() == "Darwin": # See _wake_display for why we store this value here self._caffeinate_path = self._get_exec_path("caffeinate") + else: + self._caffeinate_path = None ###################### # # @@ -232,14 +228,8 @@ def __init__(self, application: QApplication, splitter: Splitter) -> None: ####################################### # Start keyboard listener - if platform.system() == "Windows" or platform.system() == "Darwin": - self._keyboard_controller = pynput_keyboard.Controller() - self._keyboard_listener = pynput_keyboard.Listener( - on_press=self._handle_key_press, on_release=None - ) - self._keyboard_listener.start() - else: - keyboard.on_press(self._handle_key_press) + self._keyboard = UIKeyboardController() + self._keyboard.start_listener(on_press=self._handle_key_press, on_release=None) # Start poller self._poller = QTimer() @@ -286,7 +276,7 @@ def _attempt_undo_hotkey(self) -> None: """ key_code = settings.get_str("UNDO_HOTKEY_CODE") if len(key_code) > 0: - self._press_hotkey(key_code) + self._keyboard.press_and_release(key_code) else: self._request_previous_split() @@ -310,7 +300,7 @@ def _attempt_skip_hotkey(self) -> None: """ key_code = settings.get_str("SKIP_HOTKEY_CODE") if len(key_code) > 0: - self._press_hotkey(key_code) + self._keyboard.press_and_release(key_code) else: self._request_next_split() @@ -334,7 +324,7 @@ def _attempt_reset_hotkey(self) -> None: """ key_code = settings.get_str("RESET_HOTKEY_CODE") if len(key_code) > 0: - self._press_hotkey(key_code) + self._keyboard.press_and_release(key_code) else: self._request_reset_splits() @@ -1609,8 +1599,8 @@ def _get_interval(self) -> int: def _update_from_keyboard(self) -> None: """Use flags set in _handle_key_press to split and do other actions.""" - self._handle_hotkey_press() - self._execute_split_action() + self._react_to_hotkey_flags() + self._react_to_split_flags() def _handle_key_press(self, key) -> None: """Process key presses, setting flags if the key is a hotkey. @@ -1624,22 +1614,15 @@ def _handle_key_press(self, key) -> None: We set flags when hotkeys are pressed instead of directly calling a method because PyQt5 doesn't allow other threads to manipulate the UI. - Doing so almost always causes a trace trap error / crash. + Doing so almost always causes a trace trap / crash. Args: - key (pynput.keyboard.Key) -- Windows, MacOS - (keyboard.KeyboardEvent) -- Linux: - Wrapper containing info about the key that was pressed. + key: Wrapper containing info about the key that was pressed. For + more information, see the ui_keyboard_controller module. """ # Get the key's name and internal value. If the key is not an # alphanumeric key, the try block throws AttributeError. - if platform.system() == "Windows" or platform.system() == "Darwin": - try: - key_name, key_code = key.char, key.vk - except AttributeError: - key_name, key_code = str(key).replace("Key.", ""), key.value.vk - else: - key_name = key_code = key.name + key_name, key_code = self._keyboard.parse_key_info(key) # Use #1 (set hotkey settings in settings window) for hotkey_box in [ @@ -1658,7 +1641,7 @@ def _handle_key_press(self, key) -> None: hotkey_box.key_code = key_code return - # Use #2 (set "hotkey pressed" flag for _handle_hotkey_press) + # Use #2 (set "hotkey pressed" flag for _react_to_hotkey_flags) if not self._settings_window.isVisible(): for hotkey_pressed, settings_string in { "_split_hotkey_pressed": "SPLIT_HOTKEY_CODE", @@ -1674,7 +1657,7 @@ def _handle_key_press(self, key) -> None: # Use setattr because that allows us to use this dict format setattr(self, hotkey_pressed, True) - def _handle_hotkey_press(self) -> None: + def _react_to_hotkey_flags(self) -> None: """React to the flags set in _handle_key_press. When a hotkey is pressed, do its action if hotkeys are allowed now. @@ -1729,36 +1712,19 @@ def _handle_hotkey_press(self) -> None: self._main_window.screenshot_button.click() self._screenshot_hotkey_pressed = False - def _press_hotkey(self, key_code: str) -> None: - """Press and release a hotkey. - - Args: - key_code (str): A string representation of a pynput.keyboard.Key.vk - value (or a keyboard.KeyboardEvent.name value on Linux). - Passed as a string because we use QSettings, which converts all - types to strings on some backends. - """ - if platform.system() == "Windows" or platform.system() == "Darwin": - key_code = int(key_code) - key = pynput_keyboard.KeyCode(vk=key_code) - self._keyboard_controller.press(key) - self._keyboard_controller.release(key) - else: - keyboard.send(key_code) - - def _execute_split_action(self) -> None: - """Send a hotkey press and request the next split image when the - splitter finds a matching image. + def _react_to_split_flags(self) -> None: + """Press a hotkey & go to next split when self._splitter sets flags. - If no hotkey is set for a given action, ignore the hotkey press and - request the next split image anyway. + If the normal_split_action flag is set but no split hotkey is assigned, + or the hotkey wasn't heard by the application, request the next split + image manually. """ # Pause split (press pause hotkey) if self._splitter.pause_split_action: self._splitter.pause_split_action = False key_code = settings.get_str("PAUSE_HOTKEY_CODE") if len(key_code) > 0: - self._press_hotkey(key_code) + self._keyboard.press_and_release(key_code) self._request_next_split() # Dummy split (silently advance to next split) @@ -1771,7 +1737,7 @@ def _execute_split_action(self) -> None: self._splitter.normal_split_action = False key_code = settings.get_str("SPLIT_HOTKEY_CODE") if len(key_code) > 0: - self._press_hotkey(key_code) + self._keyboard.press_and_release(key_code) # If key didn't get pressed, OR if it did get pressed but global # hotkeys are off and the app isn't in focus, move the split image # forward, since pressing the key on its own won't do that @@ -1836,15 +1802,13 @@ def _wake_display(self): or (self._splitter.delaying and delay is not None) ): key = "a" # Something arbitrary; it shouldn't matter - if platform.system() == "Windows": - self._keyboard_controller.release(key) - elif platform.system() == "Darwin": + if platform.system() == "Darwin": if self._caffeinate_path is not None: subprocess.Popen([self._caffeinate_path, "-d", "-t", "1"]) else: - self._keyboard_controller.release(key) + self._keyboard.release(key) else: - keyboard.release(key) + self._keyboard.release(key) def _get_exec_path(self, name: str) -> str | None: """Return the path to an executable file, if it exists. diff --git a/src/ui/ui_keyboard_controller.py b/src/ui/ui_keyboard_controller.py new file mode 100644 index 0000000..fc2913e --- /dev/null +++ b/src/ui/ui_keyboard_controller.py @@ -0,0 +1,140 @@ +# Copyright (c) 2024 pilgrim_tabby +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +"""Wrapper for separate keyboard manip libraries to support cross-platform dev. +""" + + +import platform +if platform.system() == "Windows" or platform.system() == "Darwin": + # Don't import the whole pynput library since that takes a while + from pynput import keyboard as pynput_keyboard +else: + # Pynput doesn't work well on Linux, so use keyboard instead + import keyboard + + +class UIKeyboardController: + """Basic frontend wrapper for controlling the keyboard cross-platform. + + On Windows and MacOS, use pynput; on Linux, use keyboard. This wrapper + lacks many basic functionalities of both libraries because it was designed + to meet the current needs of Pilgrim Autosplitter. It provides a layer of + abstraction to simplify development and maintanence in ui_controller.py. + """ + def __init__(self) -> None: + """Initialize the keyboard controller (required for pynput backend). + """ + if platform.system() == "Windows" or platform.system() == "Darwin": + self._controller = pynput_keyboard.Controller() + else: + self._controller = None + + def start_listener(self, on_press, on_release) -> None: + """Start a keyboard listener. + + This method doesn't return the listener, because it's not required + (yet) for this project, but it can/should be modified to do so if it + becomes necessary (only possible w/ pynput). + + Args: + on_press (function | None): Function to be executed on key down. If + None, call _do_nothing (pass). + on_release (function | None): Function to be executed on key up. If + None, call _do_nothing (pass). + """ + if on_press is None: + on_press = self._do_nothing + if on_release is None: + on_release = self._do_nothing + + if platform.system() == "Windows" or platform.system() == "Darwin": + keyboard_listener = pynput_keyboard.Listener( + on_press=on_press, on_release=on_release + ) + keyboard_listener.start() + else: + keyboard.on_press(on_press) + keyboard.on_release(on_release) + + def press_and_release(self, key_code: str) -> None: + """Press and release a hotkey. + + Args: + key_code (str): A string representation of a pynput.keyboard.Key.vk + value (or a keyboard.KeyboardEvent.name value on Linux). + Passed as a string because this project uses QSettings, which + converts all types to strings on some backends. + """ + if platform.system() == "Windows" or platform.system() == "Darwin": + key_code = int(key_code) + key = pynput_keyboard.KeyCode(vk=key_code) + self._controller.press(key) + self._controller.release(key) + else: + keyboard.send(key_code) + + def release(self, key) -> None: + """Release a key. + + Args: + key (str): A string representation of a key. This method WILL FAIL + when using pynput if the key is not an alphanumeric key (e.g. + passing "f12" or "enter" will not work). + """ + if platform.system() == "Windows" or platform.system() == "Darwin": + self._controller.release(key) + else: + keyboard.release(key) + + def parse_key_info(self, key) -> tuple[str, str | int]: + """Return a key's string name and its internal integer value. + + Args: + key: A wrapper whose structure and contents depend on the backend. + With pynput (Windows / MacOS), it's a pynput.keyboard.Key; with + keyboard (Linux) it's a keyboard.KeyboardEvent). + + Returns: + key_name (str): Name of the key in a human-readable format. + key_code (str | int): With pynput, it's a "vk" code, or an integer + representation of a key that varies by machine. With keybaord, + it's the same as key_name. + """ + if platform.system() == "Windows" or platform.system() == "Darwin": + try: + return key.char, key.vk + # Thrown when the key isn't an alphanumeric key + except AttributeError: + return str(key).replace("Key.", ""), key.value.vk + else: + return key.name, key.name + + def _do_nothing(self, *args, **kwargs) -> None: + """Dummy method for when you don't want anything to happen.""" + pass From 22f4f05ca324e7c20fe184039a0b7e5766d1ac69 Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Mon, 29 Jul 2024 16:55:00 -0600 Subject: [PATCH 02/22] Fix some edge-case broken hotkey mechanics 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 --- src/settings.py | 4 +-- src/ui/ui_controller.py | 54 +++++++++++++++++++++++--------- src/ui/ui_keyboard_controller.py | 7 +++-- src/ui/ui_settings_window.py | 16 ++++++++++ 4 files changed, 62 insertions(+), 19 deletions(-) diff --git a/src/settings.py b/src/settings.py index 0a2bb28..c45dd0d 100644 --- a/src/settings.py +++ b/src/settings.py @@ -189,7 +189,7 @@ def set_defaults() -> None: set_value("SKIP_HOTKEY_NAME", "") # The text value of the previous split hotkey - set_value("PREVIOUS_HOTKEY_NAME", "") + set_value("PREV_HOTKEY_NAME", "") # The text value of the next split hotkey set_value("NEXT_HOTKEY_NAME", "") @@ -216,7 +216,7 @@ def set_defaults() -> None: set_value("SKIP_HOTKEY_CODE", "") # The pynput.keyboard.Key.vk value of the previous split hotkey - set_value("PREVIOUS_HOTKEY_CODE", "") + set_value("PREV_HOTKEY_CODE", "") # The pynput.keyboard.Key.vk value of the next split hotkey set_value("NEXT_HOTKEY_CODE", "") diff --git a/src/ui/ui_controller.py b/src/ui/ui_controller.py index e522fe0..3b988c1 100644 --- a/src/ui/ui_controller.py +++ b/src/ui/ui_controller.py @@ -564,8 +564,8 @@ def _reset_settings(self) -> None: settings.get_str("SKIP_HOTKEY_CODE"), ), self._settings_window.previous_hotkey_box: ( - settings.get_str("PREVIOUS_HOTKEY_NAME"), - settings.get_str("PREVIOUS_HOTKEY_CODE"), + settings.get_str("PREV_HOTKEY_NAME"), + settings.get_str("PREV_HOTKEY_CODE"), ), self._settings_window.next_hotkey_box: ( settings.get_str("NEXT_HOTKEY_NAME"), @@ -674,8 +674,8 @@ def _save_settings(self) -> None: "SKIP_HOTKEY_CODE", ), self._settings_window.previous_hotkey_box: ( - "PREVIOUS_HOTKEY_NAME", - "PREVIOUS_HOTKEY_CODE", + "PREV_HOTKEY_NAME", + "PREV_HOTKEY_CODE", ), self._settings_window.next_hotkey_box: ( "NEXT_HOTKEY_NAME", @@ -1624,6 +1624,13 @@ def _handle_key_press(self, key) -> None: # alphanumeric key, the try block throws AttributeError. key_name, key_code = self._keyboard.parse_key_info(key) + # Some keys aren't handled very well by pynput -- they return a key + # code but no name. I don't have the resources to compile an exhaustive + # list of codes that correspond to names on different platforms, so I + # think it's better to just have them do nothing for now. + if key_name is None: + return + # Use #1 (set hotkey settings in settings window) for hotkey_box in [ self._settings_window.split_hotkey_box, @@ -1639,21 +1646,40 @@ def _handle_key_press(self, key) -> None: if hotkey_box.hasFocus(): hotkey_box.setText(key_name) hotkey_box.key_code = key_code + + # Reset font size to the default + f_size = self._settings_window.fontInfo().pointSize() + hotkey_box.setStyleSheet(f"KeyLineEdit{{font-size: {f_size}pt;}}") + + # If the key name is too big for the box, resize the font down + # until it fits. Subtract 10 from the width so that there's a + # little bit of padding on the right-hand side of the box. + while hotkey_box.get_text_width() >= hotkey_box.width() - 10: + f_size = hotkey_box.get_font_size() - 1 + hotkey_box.setStyleSheet(f"KeyLineEdit{{font-size: {f_size}pt;}}") return # Use #2 (set "hotkey pressed" flag for _react_to_hotkey_flags) if not self._settings_window.isVisible(): - for hotkey_pressed, settings_string in { - "_split_hotkey_pressed": "SPLIT_HOTKEY_CODE", - "_reset_hotkey_pressed": "RESET_HOTKEY_CODE", - "_undo_hotkey_pressed": "UNDO_HOTKEY_CODE", - "_skip_hotkey_pressed": "SKIP_HOTKEY_CODE", - "_previous_hotkey_pressed": "PREVIOUS_HOTKEY_CODE", - "_next_hotkey_pressed": "NEXT_HOTKEY_CODE", - "_screenshot_hotkey_pressed": "SCREENSHOT_HOTKEY_CODE", - "_toggle_hotkeys_hotkey_pressed": "TOGGLE_HOTKEYS_HOTKEY_CODE", + for hotkey_pressed, setting in { + "_split_hotkey_pressed": ("SPLIT_HOTKEY_NAME", "SPLIT_HOTKEY_CODE"), + "_reset_hotkey_pressed": ("RESET_HOTKEY_NAME", "RESET_HOTKEY_CODE"), + "_undo_hotkey_pressed": ("UNDO_HOTKEY_NAME", "UNDO_HOTKEY_CODE"), + "_skip_hotkey_pressed": ("SKIP_HOTKEY_NAME", "SKIP_HOTKEY_CODE"), + "_previous_hotkey_pressed": ("PREV_HOTKEY_NAME", "PREV_HOTKEY_CODE"), + "_next_hotkey_pressed": ("NEXT_HOTKEY_NAME", "NEXT_HOTKEY_CODE"), + "_screenshot_hotkey_pressed": ( + "SCREENSHOT_HOTKEY_NAME", + "SCREENSHOT_HOTKEY_CODE", + ), + "_toggle_hotkeys_hotkey_pressed": ( + "TOGGLE_HOTKEYS_HOTKEY_NAME", + "TOGGLE_HOTKEYS_HOTKEY_CODE", + ), }.items(): - if str(key_code) == settings.get_str(settings_string): + settings_name = settings.get_str(setting[0]) + settings_code = settings.get_str(setting[1]) + if str(key_name) == settings_name and str(key_code) == settings_code: # Use setattr because that allows us to use this dict format setattr(self, hotkey_pressed, True) diff --git a/src/ui/ui_keyboard_controller.py b/src/ui/ui_keyboard_controller.py index fc2913e..d2f2107 100644 --- a/src/ui/ui_keyboard_controller.py +++ b/src/ui/ui_keyboard_controller.py @@ -31,6 +31,7 @@ import platform + if platform.system() == "Windows" or platform.system() == "Darwin": # Don't import the whole pynput library since that takes a while from pynput import keyboard as pynput_keyboard @@ -47,14 +48,14 @@ class UIKeyboardController: to meet the current needs of Pilgrim Autosplitter. It provides a layer of abstraction to simplify development and maintanence in ui_controller.py. """ + def __init__(self) -> None: - """Initialize the keyboard controller (required for pynput backend). - """ + """Initialize the keyboard controller (required for pynput backend).""" if platform.system() == "Windows" or platform.system() == "Darwin": self._controller = pynput_keyboard.Controller() else: self._controller = None - + def start_listener(self, on_press, on_release) -> None: """Start a keyboard listener. diff --git a/src/ui/ui_settings_window.py b/src/ui/ui_settings_window.py index 8cb2b15..1bde011 100644 --- a/src/ui/ui_settings_window.py +++ b/src/ui/ui_settings_window.py @@ -702,3 +702,19 @@ def __init__(self, parent=None) -> None: """ QLineEdit.__init__(self, parent) self.key_code = "" + + def get_text_width(self) -> int: + """Get the current width in pixels of the current text. + + Returns: + int: The width of the text. + """ + return self.fontMetrics().boundingRect(self.text()).width() + + def get_font_size(self) -> int: + """Shortcut method to get the current font size. + + Returns: + int: The font size. + """ + return self.fontInfo().pointSize() From f32ba905926258c38413118f03690466a32418ea Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Tue, 30 Jul 2024 02:14:07 -0600 Subject: [PATCH 03/22] Fix FPS not updating correctly in UI 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. --- src/ui/ui_controller.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ui/ui_controller.py b/src/ui/ui_controller.py index 3b988c1..8e8bea1 100644 --- a/src/ui/ui_controller.py +++ b/src/ui/ui_controller.py @@ -617,14 +617,13 @@ def _save_settings(self) -> None: value = float(spinbox.value()) / 100 else: value = spinbox.value() + settings.set_value(setting_string, value) # Send new FPS value to controller and splitter if spinbox == self._settings_window.fps_spinbox: self._poller.setInterval(self._get_interval()) self._splitter.target_fps = value - settings.set_value(setting_string, value) - self._splitter.splits.set_default_threshold() self._splitter.splits.set_default_delay() self._splitter.splits.set_default_pause() From 8f57b7670ca2cd227c872d9673edda443141fb9c Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Tue, 30 Jul 2024 15:25:24 -0600 Subject: [PATCH 04/22] Improve display wake performance on MacOS 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. --- src/ui/ui_controller.py | 142 ++++++++++++++++++++++++---------------- 1 file changed, 85 insertions(+), 57 deletions(-) diff --git a/src/ui/ui_controller.py b/src/ui/ui_controller.py index 8e8bea1..c8f34e8 100644 --- a/src/ui/ui_controller.py +++ b/src/ui/ui_controller.py @@ -39,6 +39,7 @@ import time import webbrowser from pathlib import Path +from threading import Thread import cv2 from PyQt5.QtCore import QRect, Qt, QTimer @@ -117,14 +118,14 @@ def __init__(self, application: QApplication, splitter: Splitter) -> None: self._screenshot_hotkey_pressed = False self._toggle_hotkeys_hotkey_pressed = False - # Store values for keeping display awake + # Values for keeping display awake (see _wake_display) self._last_wake_time = time.perf_counter() - self._wake_interval = 45 # Attempt wake after this many seconds - if platform.system() == "Darwin": - # See _wake_display for why we store this value here - self._caffeinate_path = self._get_exec_path("caffeinate") - else: - self._caffeinate_path = None + # Attempt wake after this many seconds. Should be < 1 min, since that's + # the minimum allowed time to trigger display sleep on most OSs + self._wake_interval = 45 + # MacOS-specific values (MacOS uses caffeinate) + self._caffeinate_thread = Thread(target=self._caffeinate) + self._caffeinate_thread_finished = True ###################### # # @@ -1774,66 +1775,93 @@ def _react_to_split_flags(self) -> None: self._request_next_split() def _wake_display(self): - """Keep the display awake by sending a virtual key release action. - - This should probably happen anyway (I'm assuming most use cases will - involve LiveSplit or some other app that keeps the screen on), but just - in case, this app should also keep the screen alive when it's being - actively used. - - Each time this method is called, check if more than _wake_interval - seconds have passed. If so, update _last_wake_time to the current time. - - This method works differently depending on the platform. On Windows, - if _splitter._compare_thread is alive, send a key release. Why a key - release? It's the least invasive solution I could find that didn't - require using C to permanently change some system value. I want to make - sure the system will sleep normally if this program exits abruptly. - Releasing a key doesn't require it to be pressed; it also does NOT - interrupt an actual, physical keypress by the user -- so most users - should never notice that anything is happening behind the scenes. Hacky - but it works. - - On MacOS, use the built-in `caffeinate` command to keep the display - alive. Helpfully, calling it for even 1 second, as we do, resets the - display sleep timer, so we can open a 1-sec process in the background - with subprocess.Popen every interval. If caffeinate isn't available, - fall back to the "release key method" (caffeinate doesn't force the - keyboard's backlight to stay on, but the "release key" method does, so - caffeinate is preferred even though it's slower. Another option would - be to call caffeinate -d just once in a separate thread and then - terminate the thread) - - Note: We store self._caffeinate_path so we only have to check once if - caffeinate is available on the user's machine. If it's not available, - skipping directly to keyboard.release saves about .009 seconds per call - (on my machine), which isn't nothing considering that's a good chunk of - a frame at 60fps. - Note2: Using caffeinate doesn't force the keyboard backlight to stay on, - but keyboard.release does -- another reason caffeinate is desirable. - - On Linux, we use the "release key" approach too. I don't have the right - testing environment to see if this works, but it definitely works on - Windows, so might as well put this down for now... + """Keep the display awake when the splitter is active. + + Each time this method is called, check if at least _wake_interval + seconds have passed. If so, update _last_wake_time to the current time, + and check if the splitter is active or delaying / suspending with a + countdown. If it is, attempt to wake the display (the method differs by + operating system). + + MacOS method: caffeinate (key release as fallback, see below) + Even if called with a 1 second timeout, as we do here, caffeinate + resets the OS's sleep countdown, so we only need to run it about once a + minute (1 minute is the lowest sleep timeout on MacOS) to prevent the + screen from dimming. We use a separate thread to reduce overhead + (creating a new thread requires less overhead than calling + subprocess.Popen, once, and we only need to do it once, instead of once + every _wake_interval). + If caffeinate isn't available, fall back to the "release key" method. + (see below). Caffeinate is preferred because, unlike releasing a key, + it doesn't force the keyboard's backlight to stay on, but releasing a + key at least keeps the display alive. + + Windows method: Key release + This is the least invasive solution that doesn't require permanently + changing a registry or system value, which would prevent the machine + from sleeping normally if this program exits abruptly. Key releases + instead of key presses or mouse clicks / wiggles are ideal because + releasing a key doesn't require the key to be pressed, and does NOT + interrupt an actual, physical keypress being held by the user, so users + should never notice that anything is happening behind the scenes. It's + hacky but hopefully relatively uninvasive, and its effects subside as + soon as the program stops running. + + Linux: Key release (untested, may not be reliable) """ if time.perf_counter() - self._last_wake_time >= self._wake_interval: self._last_wake_time = time.perf_counter() suspend = self._splitter.suspend_remaining delay = self._splitter.delay_remaining - - if ( + splitter_active = ( not self._splitter.suspended or (self._splitter.suspended and suspend is not None) or (self._splitter.delaying and delay is not None) - ): - key = "a" # Something arbitrary; it shouldn't matter - if platform.system() == "Darwin": - if self._caffeinate_path is not None: - subprocess.Popen([self._caffeinate_path, "-d", "-t", "1"]) - else: + ) + + # Key should be alphanumeric to work cross platform; beyond that it + # doesn't matter, since the user won't detect its release + key = "a" + + # MacOS: Try caffeinate, fall back to key release if necessary + if platform.system() == "Darwin": + if splitter_active: + + caffeinate_path = self._get_exec_path("caffeinate") + + # No caffeinate, use fallback + if caffeinate_path is None: self._keyboard.release(key) + + # Caffeinate exists; start thread if it hasn't been started + elif not self._caffeinate_thread.is_alive(): + self._caffeinate_thread_finished = False + self._caffeinate_thread = Thread( + target=self._caffeinate, args=(caffeinate_path,) + ) + self._caffeinate_thread.daemon = True + self._caffeinate_thread.start() + + # Splitter inactive; kill thread (join not needed, will die) else: - self._keyboard.release(key) + self._caffeinate_thread_finished = True + + # Winodws / Linux: release a key if splitter active + elif splitter_active: + self._keyboard.release(key) + + def _caffeinate(self, caffeinate_path: str) -> None: + """Use built-in caffeinate to keep the machine's display on (MacOS). + + A 1-second timeout is used because caffeinate resets the sleep timer + (i.e. it doesn't need to be constantly running). This is good because + it's hard to reliably terminate an ongoing caffeinate process; this way + we guarantee that users' sleep settings will return to normal after the + program terminates. + """ + while not self._caffeinate_thread_finished: + subprocess.Popen([caffeinate_path, "-d", "-t", "1"]) + time.sleep(self._wake_interval) def _get_exec_path(self, name: str) -> str | None: """Return the path to an executable file, if it exists. From 5ff72c90f994263c09031ada527409d12a72e21a Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Sun, 4 Aug 2024 23:37:19 -0600 Subject: [PATCH 05/22] Fix typehints for compatibility w/ Python 3.8+ --- src/splitter/split_dir.py | 8 ++++---- src/ui/ui_controller.py | 7 +++++-- src/ui/ui_keyboard_controller.py | 19 ++++++++++++++----- src/ui/ui_main_window.py | 7 ++++--- src/ui/ui_settings_window.py | 6 +++--- 5 files changed, 30 insertions(+), 17 deletions(-) diff --git a/src/splitter/split_dir.py b/src/splitter/split_dir.py index 66ad3bb..1fda351 100644 --- a/src/splitter/split_dir.py +++ b/src/splitter/split_dir.py @@ -36,7 +36,7 @@ import re from multiprocessing import freeze_support from multiprocessing.dummy import Pool as ThreadPool -from typing import Tuple +from typing import List, Tuple import cv2 import numpy @@ -64,7 +64,7 @@ class SplitDir: exists. current_loop (int): The current split image's current loop, if it exists. - list (list[_SplitImage]): A list of all split images in the directory + List[List[_SplitImage]]: A list of all split images in the directory settings.get_str("LAST_IMAGE_DIR"). """ @@ -158,7 +158,7 @@ def resize_images(self) -> None: # # ############### - def _get_split_images(self) -> list["_SplitImage"]: + def _get_split_images(self) -> List["_SplitImage"]: """Get a list of SplitImage objects from a directory. Currently supported image types include .png, .jpg, and .jpeg. Only @@ -169,7 +169,7 @@ def _get_split_images(self) -> list["_SplitImage"]: lot when there are lots of images. Returns: - list[_SplitImage]: The list of SplitImage objects. + List[_SplitImage]: The list of SplitImage objects. """ def get_split_image(index: int, path: str): diff --git a/src/ui/ui_controller.py b/src/ui/ui_controller.py index c8f34e8..8b9d58d 100644 --- a/src/ui/ui_controller.py +++ b/src/ui/ui_controller.py @@ -37,6 +37,7 @@ import platform import subprocess import time +from typing import Optional, Union import webbrowser from pathlib import Path from threading import Thread @@ -1602,7 +1603,9 @@ def _update_from_keyboard(self) -> None: self._react_to_hotkey_flags() self._react_to_split_flags() - def _handle_key_press(self, key) -> None: + def _handle_key_press( + self, key: Union["pynput.keyboard.key", "keyboard.KeyboardEvent"] + ) -> None: """Process key presses, setting flags if the key is a hotkey. Called each time any key is pressed, whether or not the program is in @@ -1863,7 +1866,7 @@ def _caffeinate(self, caffeinate_path: str) -> None: subprocess.Popen([caffeinate_path, "-d", "-t", "1"]) time.sleep(self._wake_interval) - def _get_exec_path(self, name: str) -> str | None: + def _get_exec_path(self, name: str) -> Optional[str]: """Return the path to an executable file, if it exists. Args: diff --git a/src/ui/ui_keyboard_controller.py b/src/ui/ui_keyboard_controller.py index d2f2107..ad80300 100644 --- a/src/ui/ui_keyboard_controller.py +++ b/src/ui/ui_keyboard_controller.py @@ -31,6 +31,7 @@ import platform +from typing import Callable, Optional, Tuple, Union if platform.system() == "Windows" or platform.system() == "Darwin": # Don't import the whole pynput library since that takes a while @@ -56,7 +57,11 @@ def __init__(self) -> None: else: self._controller = None - def start_listener(self, on_press, on_release) -> None: + def start_listener( + self, + on_press: Optional[Callable[..., None]], + on_release: Optional[Callable[..., None]], + ) -> None: """Start a keyboard listener. This method doesn't return the listener, because it's not required @@ -64,9 +69,9 @@ def start_listener(self, on_press, on_release) -> None: becomes necessary (only possible w/ pynput). Args: - on_press (function | None): Function to be executed on key down. If + on_press (callable | None): Function to be executed on key down. If None, call _do_nothing (pass). - on_release (function | None): Function to be executed on key up. If + on_release (callable | None): Function to be executed on key up. If None, call _do_nothing (pass). """ if on_press is None: @@ -100,7 +105,9 @@ def press_and_release(self, key_code: str) -> None: else: keyboard.send(key_code) - def release(self, key) -> None: + def release( + self, key: Union["pynput_keyboard.key", "keyboard.KeyboardEvent"] + ) -> None: """Release a key. Args: @@ -113,7 +120,9 @@ def release(self, key) -> None: else: keyboard.release(key) - def parse_key_info(self, key) -> tuple[str, str | int]: + def parse_key_info( + self, key: Union["pynput_keyboard.key", "keyboard.KeyboardEvent"] + ) -> Tuple[str, Union[str, int]]: """Return a key's string name and its internal integer value. Args: diff --git a/src/ui/ui_main_window.py b/src/ui/ui_main_window.py index 9dc3a6f..30bba4b 100644 --- a/src/ui/ui_main_window.py +++ b/src/ui/ui_main_window.py @@ -32,6 +32,7 @@ import platform +from typing import Optional from PyQt5.QtCore import QRect, Qt, pyqtSignal from PyQt5.QtGui import QMouseEvent @@ -578,7 +579,7 @@ class ClickableLineEdit(QLineEdit): clicked = pyqtSignal() - def __init__(self, parent=None) -> None: + def __init__(self, parent: Optional[QWidget] = None) -> None: """Inherit from QLineEdit and set attributes to None. Args: @@ -586,7 +587,7 @@ def __init__(self, parent=None) -> None: """ QLineEdit.__init__(self, parent) - def mouseMoveEvent(self, a0: QMouseEvent | None) -> None: + def mouseMoveEvent(self, a0: Optional[QMouseEvent]) -> None: """Prevent the mouse moving from having any effect. I override this method specifically to prevent selecting text by @@ -599,7 +600,7 @@ def mouseMoveEvent(self, a0: QMouseEvent | None) -> None: """ pass - def mouseDoubleClickEvent(self, a0: QMouseEvent | None) -> None: + def mouseDoubleClickEvent(self, a0: Optional[QMouseEvent]) -> None: """Prevent a double click from having any effect. I override this method specifically to prevent double-clicking to diff --git a/src/ui/ui_settings_window.py b/src/ui/ui_settings_window.py index 1bde011..5212388 100644 --- a/src/ui/ui_settings_window.py +++ b/src/ui/ui_settings_window.py @@ -32,7 +32,7 @@ import platform -from typing import Union +from typing import Optional, Union from PyQt5.QtCore import QEvent, QRect, Qt from PyQt5.QtGui import QColor @@ -659,7 +659,7 @@ def __init__(self) -> None: self.save_button.setGeometry(QRect(459 + self._LEFT, 326 + self._TOP, 111, 31)) self.save_button.setFocusPolicy(Qt.NoFocus) - def event(self, event) -> Union[bool, QWidget.event]: + def event(self, event: QWidget.event) -> Union[bool, QWidget.event]: """Allow the user to take focus off a widget by clicking somewhere else. @@ -693,7 +693,7 @@ class KeyLineEdit(QLineEdit): depending on the backend used). """ - def __init__(self, parent=None) -> None: + def __init__(self, parent: Optional[QWidget] = None) -> None: """Initialize a KeyLineEdit with a blank key_code value. Args: From 9f8a2d689ea24fd5e7e619033ce8f2970971dfa8 Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Mon, 5 Aug 2024 01:46:29 -0600 Subject: [PATCH 06/22] Fix occasional crash when trying to bind hotkeys 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. --- src/ui/ui_controller.py | 87 +++++++++++++++++++++++++++++++---------- 1 file changed, 66 insertions(+), 21 deletions(-) diff --git a/src/ui/ui_controller.py b/src/ui/ui_controller.py index 8b9d58d..750e503 100644 --- a/src/ui/ui_controller.py +++ b/src/ui/ui_controller.py @@ -40,7 +40,7 @@ from typing import Optional, Union import webbrowser from pathlib import Path -from threading import Thread +from threading import Lock, Thread import cv2 from PyQt5.QtCore import QRect, Qt, QTimer @@ -104,6 +104,13 @@ def __init__(self, application: QApplication, splitter: Splitter) -> None: # Should be set whenever the split image is modified self._redraw_split_labels = True + # Values for updating hotkeys in settings menu + # (see _react_to_settings_menu_flags) + self._hotkey_box_to_change = None + self._hotkey_box_key_code = None + self._hotkey_box_key_name = None + self._hotkey_box_lock = Lock() + # Flags to disable hotkeys self._split_hotkey_enabled = False self._undo_hotkey_enabled = False @@ -1601,6 +1608,7 @@ def _get_interval(self) -> int: def _update_from_keyboard(self) -> None: """Use flags set in _handle_key_press to split and do other actions.""" self._react_to_hotkey_flags() + self._react_to_settings_menu_flags() self._react_to_split_flags() def _handle_key_press( @@ -1611,13 +1619,19 @@ def _handle_key_press( Called each time any key is pressed, whether or not the program is in focus. This method has two main uses: 1) Updates users' custom hotkey bindings. It does this by checking - if a given hotkey "line edit" has focus and, if so, changing - its key_code and text to match the key. + if a given hotkey "line edit" has focus and, if so, setting + flags so its name and key code are updated. + + Uses a lock so the poller doesn't try to update these values as + they're being written (the worst case would be setting a hotkey + with the correct name but wrong key code -- unlikely but not + impossible). + 2) If a hotkey is pressed, sets a flag indicating it was pressed. - We set flags when hotkeys are pressed instead of directly calling a - method because PyQt5 doesn't allow other threads to manipulate the UI. - Doing so almost always causes a trace trap / crash. + We set flags when keys are pressed instead of directly calling a method + because PyQt5 doesn't play nice when other threads try to manipulate + the GUI. Doing so often causes a trace trap / segmentation fault. Args: key: Wrapper containing info about the key that was pressed. For @@ -1647,20 +1661,11 @@ def _handle_key_press( self._settings_window.toggle_global_hotkeys_hotkey_box, ]: if hotkey_box.hasFocus(): - hotkey_box.setText(key_name) - hotkey_box.key_code = key_code - - # Reset font size to the default - f_size = self._settings_window.fontInfo().pointSize() - hotkey_box.setStyleSheet(f"KeyLineEdit{{font-size: {f_size}pt;}}") - - # If the key name is too big for the box, resize the font down - # until it fits. Subtract 10 from the width so that there's a - # little bit of padding on the right-hand side of the box. - while hotkey_box.get_text_width() >= hotkey_box.width() - 10: - f_size = hotkey_box.get_font_size() - 1 - hotkey_box.setStyleSheet(f"KeyLineEdit{{font-size: {f_size}pt;}}") - return + with self._hotkey_box_lock: + self._hotkey_box_to_change = hotkey_box + self._hotkey_box_key_code = key_code + self._hotkey_box_key_name = key_name + return # Use #2 (set "hotkey pressed" flag for _react_to_hotkey_flags) if not self._settings_window.isVisible(): @@ -1687,7 +1692,7 @@ def _handle_key_press( setattr(self, hotkey_pressed, True) def _react_to_hotkey_flags(self) -> None: - """React to the flags set in _handle_key_press. + """React to the flags set in _handle_key_press for hotkeys. When a hotkey is pressed, do its action if hotkeys are allowed now. Then, unset the flag no matter what. @@ -1741,6 +1746,46 @@ def _react_to_hotkey_flags(self) -> None: self._main_window.screenshot_button.click() self._screenshot_hotkey_pressed = False + def _react_to_settings_menu_flags(self) -> None: + """React to the flags set in _handle_key_press for updating hotkeys. + + If _handle_hotkey_press has set self._hotkey_box_to_change to a hotkey + box, use the values stored in _hotkey_box_key_name and _hotkey_box_key + _code to update the hotkey box. Then, reset _hotkey_box_to_change back + to None. + + We use _hotkey_box_lock to make sure the values passed from _handle_key + _press aren't overwritten mid-method. + + Setting the hotkey box's attributes directly from _handle_hotkey_press + is simpler, but PyQt5 doesn't like it and it occasionally causes a + segmentation fault. + """ + with self._hotkey_box_lock: + if self._hotkey_box_to_change is None: + return + + hotkey_box = self._hotkey_box_to_change + key_name = self._hotkey_box_key_name + key_code = self._hotkey_box_key_code + + # Set box attributes + hotkey_box.setText(key_name) + hotkey_box.key_code = key_code + + # Reset font size to the default + f_size = self._settings_window.fontInfo().pointSize() + hotkey_box.setStyleSheet(f"KeyLineEdit{{font-size: {f_size}pt;}}") + + # If the key name is too big for the box, resize the font down + # until it fits. Subtract 10 from the width so that there's a + # little bit of padding on the right-hand side of the box. + while hotkey_box.get_text_width() >= hotkey_box.width() - 10: + f_size = hotkey_box.get_font_size() - 1 + hotkey_box.setStyleSheet(f"KeyLineEdit{{font-size: {f_size}pt;}}") + + self._hotkey_box_to_change = None + def _react_to_split_flags(self) -> None: """Press a hotkey & go to next split when self._splitter sets flags. From 09cb84d2b26291451fbcb917b15d08c14760273d Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Mon, 5 Aug 2024 01:57:21 -0600 Subject: [PATCH 07/22] Update README.md for Python 3.8+ compatibility --- README.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 0b8d2fe..1ed73f7 100644 --- a/README.md +++ b/README.md @@ -47,19 +47,20 @@ Download the latest Windows build (Pilgrim.Autosplitter.Windows.v1.x.x) from the If you're familiar with Python: -* Make sure you have Python 3.10+ installed +* Make sure you have Python 3.8+ installed (Python 3.11+ is recommended for an optimal experience) * Run `python -m pip install -r requirements.txt` > [!Note] > If installation hangs up when trying to download PyQt5, try running the following command: `python -m pip install pyqt5 --config-settings --confirm-license= --verbose`. > Some users (including myself) have experienced a softlock when verifying the PyQt5 license; this should solve that problem. +> In addition, if using Python <=3.10, you may need to manually install PyQt5 using `python -m pip install pyqt5-tools`. * Open the app with `python pilgrim_autosplitter.py` If this is your first time using Python: -* Install the latest version of Python (3.12 at this time) by clicking [here](https://www.python.org/downloads/). +* Install the latest stable Python release (3.12 at this time) by clicking [here](https://www.python.org/downloads/). > [!Important] > When installing Python using the Python installer, you MUST check the box next to `Add Python to PATH`. If you don't, the next part won't work. @@ -100,19 +101,20 @@ Known limitations: If you're familiar with Python: -* Make sure you have Python 3.10+ installed +* Make sure you have Python 3.8+ installed (Python 3.11+ is recommended for an optimal experience) * Run `pip3 install -r requirements.txt` > [!Note] > If installation hangs up when trying to download PyQt5, try running the following command: `python -m pip install pyqt5 --config-settings --confirm-license= --verbose`. > Some users (including myself) have experienced a softlock when verifying the PyQt5 license; this should solve that problem. +> In addition, if using Python <=3.10, you may need to manually install PyQt5 using `python -m pip install pyqt5-tools`. * Open the app with `python3 pilgrim_autosplitter.py` If this is your first time using Python: -* Install the latest version of Python (3.12 at this time) by clicking [here](https://www.python.org/downloads/). +* Install the latest stable Python release (3.12 at this time) by clicking [here](https://www.python.org/downloads/). * Download Pilgrim Autosplitter's source code from the [most recent release](https://github.com/pilgrimtabby/pilgrim-autosplitter/releases/latest) (click on `Source code (zip)` or `Source code (tar.gz)`). Extract the files. @@ -134,8 +136,9 @@ Known limitations: ### Method 2: Run from source with Python -If you're on Linux, I assume you know what you're doing. Get the latest release [here](https://github.com/pilgrimtabby/pilgrim-autosplitter/releases/latest), use pip to install the dependencies in `requirements-linux.txt`, and run `src/pilgrim_autosplitter.py` as root. Python >=3.10 is required. +If you're on Linux, I assume you know what you're doing. Get the latest release [here](https://github.com/pilgrimtabby/pilgrim-autosplitter/releases/latest), use pip to install the dependencies in `requirements-linux.txt`, and run `src/pilgrim_autosplitter.py` as root. Python >=3.8 is required (Python 3.11+ is recommended for an optimal experience). > [!Note] > If installation hangs up when trying to download PyQt5, try running the following command: `python -m pip install pyqt5 --config-settings --confirm-license= --verbose`. > Some users (including myself) have experienced a softlock when verifying the PyQt5 license; this should solve that problem. +> In addition, if using Python <=3.10, you may need to manually install PyQt5 using `python -m pip install pyqt5-tools`. From d3777bc5d0b50d7d185887efd417681da4454525 Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Mon, 5 Aug 2024 02:08:02 -0600 Subject: [PATCH 08/22] Bump version to v1.0.7 --- src/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/settings.py b/src/settings.py index c45dd0d..b68aec1 100644 --- a/src/settings.py +++ b/src/settings.py @@ -43,7 +43,7 @@ COMPARISON_FRAME_HEIGHT = 240 # Pilgrim Autosplitter's current version number -VERSION_NUMBER = "v1.0.6" +VERSION_NUMBER = "v1.0.7" # The URL of Pilgrim Autosplitter's GitHub repo REPO_URL = "https://github.com/pilgrimtabby/pilgrim-autosplitter/" From 0d15be1e11e70236cedb4038dd2d97778d158292 Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Mon, 5 Aug 2024 02:10:08 -0600 Subject: [PATCH 09/22] Update .gitignore --- .gitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitignore b/.gitignore index b9d7465..32626b4 100644 --- a/.gitignore +++ b/.gitignore @@ -8,4 +8,3 @@ **/_site **/.jekyll-cache pilgrim_autosplitter.spec - From 35d4f2e0a0ca92fb1bce93274ea2a75c334f1f84 Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Wed, 21 Aug 2024 16:25:33 -0600 Subject: [PATCH 10/22] Begin writing pigrilm_autosplitter.py test --- src/pilgrim_autosplitter.py | 182 ++++++++++++++--------------- tests/test_pilgrim_autosplitter.py | 55 +++++++++ 2 files changed, 142 insertions(+), 95 deletions(-) create mode 100644 tests/test_pilgrim_autosplitter.py diff --git a/src/pilgrim_autosplitter.py b/src/pilgrim_autosplitter.py index c2e8a55..6ccbba4 100644 --- a/src/pilgrim_autosplitter.py +++ b/src/pilgrim_autosplitter.py @@ -26,108 +26,100 @@ # OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -"""Initialize and run Pilgrim Autosplitter. -""" +"""Initialize and run Pilgrim Autosplitter.""" -def main(): - """Initialize PilgrimAutosplitter (with logging if not on Windows). - - This module is in an unusual format, with all the import statements behind - main() and the class declaration inside of main(), to accomodate the time - required to boot up the executable if this program is run after being built - with PyInstaller. To reassure the user, I've placed print messages before - and after the import statements, which take a very long time (sometimes as - long as 30 seconds). +import os +import platform +import sys + + +class PilgrimAutosplitter: + """Initialize and run Pilgrim Autosplitter. + + Import statements are in __init__ because I only expect this class to be + instantiated once per session and I want the print statements in main to + appear before the import statements are run (they can take a long time to + complete, especially using PyInstaller). + + Attributes: + pilgrim_autosplitter (QApplication): The application container that + allows QObjects, including the UI, to be initialized. + splitter (Splitter): Backend for capturing and comparing images to + video. + ui_controller (UIController): Backend for updating the UI and handling + user input. """ - # Initial setup - import os + def __init__(self) -> None: + """Initialize splitter and controller to run Pilgrim Autosplitter.""" + from PyQt5.QtGui import QIcon, QPixmap + from PyQt5.QtWidgets import QApplication + + import settings + from splitter.splitter import Splitter + from ui.ui_controller import UIController + + program_directory = os.path.dirname(os.path.abspath(__file__)) + + if platform.system() == "Windows": + # Force title bar to follow system theme + extra_args = ["-platform", "windows:darkmode=1"] + else: + extra_args = [] + self.gui = QApplication(sys.argv + extra_args) + self.gui.setStyle("fusion") + self.gui.setApplicationName("Pilgrim Autosplitter") + + # Set taskbar icons. Doesn't seem to really do anything, but it's a + # work in progress so I'll leave it for now + if platform.system() == "Windows": + import ctypes + + self.gui.setWindowIcon( + QIcon(QPixmap(f"{program_directory}/../resources/icon-windows.png")) + ) + # Tell Windows this app is its own process so icon shows up + app_id = "pilgrim_tabby.pilgrim_autosplitter.latest" + ctypes.windll.shell32.SetCurrentProcessExplicitAppUserModelID(app_id) + # Without the absolute path, the icon only shows up when running + # the program from the same directory /resources is in. This makes + # it show up regardless (at least when ran from source, not build) + else: + self.gui.setWindowIcon( + QIcon(QPixmap(f"{program_directory}/../resources/icon-macos.png")) + ) + + settings.set_defaults() + + self.splitter = Splitter() + if settings.get_bool("START_WITH_VIDEO"): + self.splitter.start() + + self.ui_controller = UIController(self.gui, self.splitter) + + def run(self) -> None: + """Start and, when finished, safely quit the autosplitter.""" + self.gui.exec() + # Prevent segmentation fault or other clumsy errors on exit. + # Subthreads won't persist since they're daemons, but this helps + # make sure they stop BEFORE the main thread ends. + self.splitter.safe_exit_all_threads() + # TODO: make sure controller threads exit too - os.system("cls || clear") # This works cross-platform +def main(): + """Initialize PilgrimAutosplitter.""" + os.system("cls || clear") # Cross-platform clear screen print("Welcome to Pilgrim Autosplitter!") - print("You can minimize this window, but DO NOT close it.\n") - - print("Importing third-party packages...") - - from PyQt5.QtGui import QIcon, QPixmap - from PyQt5.QtWidgets import QApplication - - print("Importing built-in packages...") - - import platform - import sys - - print("Initializing Pilgrim Autosplitter...") - - # Class definition - class PilgrimAutosplitter: - """Initialize and run Pilgrim Autosplitter. - - Attributes: - pilgrim_autosplitter (QApplication): The application container that - allows QObjects, including the UI, to be initialized. - splitter (Splitter): Backend for capturing and comparing images to - video. - ui_controller (UIController): Backend for updating the UI and handling - user input. - """ - - def __init__(self) -> None: - """Initialize splitter and controller to run Pilgrim Autosplitter.""" - import settings - from splitter.splitter import Splitter - from ui.ui_controller import UIController - - program_directory = os.path.dirname(os.path.abspath(__file__)) - - if platform.system() == "Windows": - # Force title bar to follow system theme - extra_args = ["-platform", "windows:darkmode=1"] - else: - extra_args = [] - self.pilgrim_autosplitter = QApplication(sys.argv + extra_args) - self.pilgrim_autosplitter.setStyle("fusion") - self.pilgrim_autosplitter.setApplicationName("Pilgrim Autosplitter") - - # Set taskbar icons. Doesn't seem to really do anything, but it's a - # work in progress so I'll leave it for now - if platform.system() == "Windows": - import ctypes - - self.pilgrim_autosplitter.setWindowIcon( - QIcon(QPixmap(f"{program_directory}/../resources/icon-windows.png")) - ) - # Tell Windows this app is its own process so icon shows up - app_id = "pilgrim_tabby.pilgrim_autosplitter.latest" - ctypes.windll.shell32.SetCurrentProcessExplicitAppUserModelID(app_id) - # Without the absolute path, the icon only shows up when running - # the program from the same directory /resources is in. This makes - # it show up regardless (at least when ran from source, not build) - else: - self.pilgrim_autosplitter.setWindowIcon( - QIcon(QPixmap(f"{program_directory}/../resources/icon-macos.png")) - ) - - settings.set_defaults() - - self.splitter = Splitter() - if settings.get_bool("START_WITH_VIDEO"): - self.splitter.start() - - self.ui_controller = UIController(self.pilgrim_autosplitter, self.splitter) - - self.pilgrim_autosplitter.exec() - - # Prevent segmentation fault or other clumsy errors on exit - # The threads won't persist since they're daemons, but this helps - # make sure they stop BEFORE the main thread ends - self.splitter.safe_exit_all_threads() - - # Open application - print("Starting Pilgrim Autosplitter...\n") - PilgrimAutosplitter() + print("You may minimize this window, but DO NOT close it.\n") + print("Loading Pilgrim Autosplitter (this may take a few minutes)...") + + app = PilgrimAutosplitter() + + print("Starting...") + print(app.gui.arguments()) + # app.run() if __name__ == "__main__": diff --git a/tests/test_pilgrim_autosplitter.py b/tests/test_pilgrim_autosplitter.py new file mode 100644 index 0000000..7d97783 --- /dev/null +++ b/tests/test_pilgrim_autosplitter.py @@ -0,0 +1,55 @@ +# Copyright (c) 2024 pilgrim_tabby +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +"""Test pilgrim_autosplitter.py.""" + + +import platform +import unittest + +import pilgrim_autosplitter + + +class TestPilgrimAutosplitter(unittest.TestCase): + + def setUp(self) -> None: + self.app = pilgrim_autosplitter.PilgrimAutosplitter() + + def test_init(self): + if platform.system() == "Wimdows": + self.assertEqual(self.app.gui.arguments, True) + + def test_split(self): + s = 'hello world' + self.assertEqual(s.split(), ['hello', 'world']) + # check that s.split fails when the separator is not a string + with self.assertRaises(TypeError): + s.split(2) + +if __name__ == '__main__': + unittest.main() From d0757044f043c6ac1504b0dfdbd1e011cc3ee661 Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Wed, 21 Aug 2024 18:42:14 -0600 Subject: [PATCH 11/22] Complete test_pilgrim_autosplitter.py -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 --- src/pilgrim_autosplitter.py | 21 +++++++-------- src/ui/ui_controller.py | 20 +++++++------- src/ui/ui_keyboard_controller.py | 11 ++++++-- tests/__init__.py | 6 +++++ tests/test_pilgrim_autosplitter.py | 43 +++++++++++++++++++----------- 5 files changed, 62 insertions(+), 39 deletions(-) create mode 100644 tests/__init__.py diff --git a/src/pilgrim_autosplitter.py b/src/pilgrim_autosplitter.py index 6ccbba4..ee6b570 100644 --- a/src/pilgrim_autosplitter.py +++ b/src/pilgrim_autosplitter.py @@ -67,16 +67,16 @@ def __init__(self) -> None: extra_args = ["-platform", "windows:darkmode=1"] else: extra_args = [] - self.gui = QApplication(sys.argv + extra_args) - self.gui.setStyle("fusion") - self.gui.setApplicationName("Pilgrim Autosplitter") + self.app = QApplication(sys.argv + extra_args) + self.app.setStyle("fusion") + self.app.setApplicationName("Pilgrim Autosplitter") # Set taskbar icons. Doesn't seem to really do anything, but it's a # work in progress so I'll leave it for now if platform.system() == "Windows": import ctypes - self.gui.setWindowIcon( + self.app.setWindowIcon( QIcon(QPixmap(f"{program_directory}/../resources/icon-windows.png")) ) # Tell Windows this app is its own process so icon shows up @@ -86,7 +86,7 @@ def __init__(self) -> None: # the program from the same directory /resources is in. This makes # it show up regardless (at least when ran from source, not build) else: - self.gui.setWindowIcon( + self.app.setWindowIcon( QIcon(QPixmap(f"{program_directory}/../resources/icon-macos.png")) ) @@ -96,16 +96,16 @@ def __init__(self) -> None: if settings.get_bool("START_WITH_VIDEO"): self.splitter.start() - self.ui_controller = UIController(self.gui, self.splitter) + self.ui_controller = UIController(self.app, self.splitter) def run(self) -> None: """Start and, when finished, safely quit the autosplitter.""" - self.gui.exec() + self.app.exec() # Prevent segmentation fault or other clumsy errors on exit. # Subthreads won't persist since they're daemons, but this helps # make sure they stop BEFORE the main thread ends. self.splitter.safe_exit_all_threads() - # TODO: make sure controller threads exit too + self.ui_controller.keyboard.stop_listener() def main(): """Initialize PilgrimAutosplitter.""" @@ -115,11 +115,10 @@ def main(): print("You may minimize this window, but DO NOT close it.\n") print("Loading Pilgrim Autosplitter (this may take a few minutes)...") - app = PilgrimAutosplitter() + pilgrim_autosplitter = PilgrimAutosplitter() print("Starting...") - print(app.gui.arguments()) - # app.run() + pilgrim_autosplitter.run() if __name__ == "__main__": diff --git a/src/ui/ui_controller.py b/src/ui/ui_controller.py index 750e503..6957a19 100644 --- a/src/ui/ui_controller.py +++ b/src/ui/ui_controller.py @@ -237,8 +237,8 @@ def __init__(self, application: QApplication, splitter: Splitter) -> None: ####################################### # Start keyboard listener - self._keyboard = UIKeyboardController() - self._keyboard.start_listener(on_press=self._handle_key_press, on_release=None) + self.keyboard = UIKeyboardController() + self.keyboard.start_listener(on_press=self._handle_key_press, on_release=None) # Start poller self._poller = QTimer() @@ -285,7 +285,7 @@ def _attempt_undo_hotkey(self) -> None: """ key_code = settings.get_str("UNDO_HOTKEY_CODE") if len(key_code) > 0: - self._keyboard.press_and_release(key_code) + self.keyboard.press_and_release(key_code) else: self._request_previous_split() @@ -309,7 +309,7 @@ def _attempt_skip_hotkey(self) -> None: """ key_code = settings.get_str("SKIP_HOTKEY_CODE") if len(key_code) > 0: - self._keyboard.press_and_release(key_code) + self.keyboard.press_and_release(key_code) else: self._request_next_split() @@ -333,7 +333,7 @@ def _attempt_reset_hotkey(self) -> None: """ key_code = settings.get_str("RESET_HOTKEY_CODE") if len(key_code) > 0: - self._keyboard.press_and_release(key_code) + self.keyboard.press_and_release(key_code) else: self._request_reset_splits() @@ -1639,7 +1639,7 @@ def _handle_key_press( """ # Get the key's name and internal value. If the key is not an # alphanumeric key, the try block throws AttributeError. - key_name, key_code = self._keyboard.parse_key_info(key) + key_name, key_code = self.keyboard.parse_key_info(key) # Some keys aren't handled very well by pynput -- they return a key # code but no name. I don't have the resources to compile an exhaustive @@ -1798,7 +1798,7 @@ def _react_to_split_flags(self) -> None: self._splitter.pause_split_action = False key_code = settings.get_str("PAUSE_HOTKEY_CODE") if len(key_code) > 0: - self._keyboard.press_and_release(key_code) + self.keyboard.press_and_release(key_code) self._request_next_split() # Dummy split (silently advance to next split) @@ -1811,7 +1811,7 @@ def _react_to_split_flags(self) -> None: self._splitter.normal_split_action = False key_code = settings.get_str("SPLIT_HOTKEY_CODE") if len(key_code) > 0: - self._keyboard.press_and_release(key_code) + self.keyboard.press_and_release(key_code) # If key didn't get pressed, OR if it did get pressed but global # hotkeys are off and the app isn't in focus, move the split image # forward, since pressing the key on its own won't do that @@ -1879,7 +1879,7 @@ def _wake_display(self): # No caffeinate, use fallback if caffeinate_path is None: - self._keyboard.release(key) + self.keyboard.release(key) # Caffeinate exists; start thread if it hasn't been started elif not self._caffeinate_thread.is_alive(): @@ -1896,7 +1896,7 @@ def _wake_display(self): # Winodws / Linux: release a key if splitter active elif splitter_active: - self._keyboard.release(key) + self.keyboard.release(key) def _caffeinate(self, caffeinate_path: str) -> None: """Use built-in caffeinate to keep the machine's display on (MacOS). diff --git a/src/ui/ui_keyboard_controller.py b/src/ui/ui_keyboard_controller.py index ad80300..9aa174d 100644 --- a/src/ui/ui_keyboard_controller.py +++ b/src/ui/ui_keyboard_controller.py @@ -52,6 +52,7 @@ class UIKeyboardController: def __init__(self) -> None: """Initialize the keyboard controller (required for pynput backend).""" + self._keyboard_listener = None if platform.system() == "Windows" or platform.system() == "Darwin": self._controller = pynput_keyboard.Controller() else: @@ -80,14 +81,20 @@ def start_listener( on_release = self._do_nothing if platform.system() == "Windows" or platform.system() == "Darwin": - keyboard_listener = pynput_keyboard.Listener( + self._keyboard_listener = pynput_keyboard.Listener( on_press=on_press, on_release=on_release ) - keyboard_listener.start() + self._keyboard_listener.start() else: keyboard.on_press(on_press) keyboard.on_release(on_release) + def stop_listener(self) -> None: + if platform.system() == "Windows" or platform.system() == "Darwin": + self._keyboard_listener.stop() + else: + pass + def press_and_release(self, key_code: str) -> None: """Press and release a hotkey. diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..726a5b6 --- /dev/null +++ b/tests/__init__.py @@ -0,0 +1,6 @@ +"""Add files in /src to PATH so they can be tested.""" + +import sys + + +sys.path.append("./src") diff --git a/tests/test_pilgrim_autosplitter.py b/tests/test_pilgrim_autosplitter.py index 7d97783..9f2204f 100644 --- a/tests/test_pilgrim_autosplitter.py +++ b/tests/test_pilgrim_autosplitter.py @@ -30,26 +30,37 @@ import platform -import unittest +import time +from threading import active_count, Thread -import pilgrim_autosplitter +from PyQt5.QtWidgets import QApplication +from src.pilgrim_autosplitter import PilgrimAutosplitter -class TestPilgrimAutosplitter(unittest.TestCase): - def setUp(self) -> None: - self.app = pilgrim_autosplitter.PilgrimAutosplitter() +def test_run() -> None: + """Make sure app opens correctly and shuts down cleanly (kills threads).""" + def quit_app(app: QApplication) -> None: + time.sleep(2) + app.quit() - def test_init(self): - if platform.system() == "Wimdows": - self.assertEqual(self.app.gui.arguments, True) + pilgrim_autosplitter = PilgrimAutosplitter() - def test_split(self): - s = 'hello world' - self.assertEqual(s.split(), ['hello', 'world']) - # check that s.split fails when the separator is not a string - with self.assertRaises(TypeError): - s.split(2) + exit_gui_thread = Thread(target=quit_app, args=(pilgrim_autosplitter.app,)) + exit_gui_thread.daemon = True + exit_gui_thread.start() -if __name__ == '__main__': - unittest.main() + pilgrim_autosplitter.run() + exit_gui_thread.join() + + # Give joined threads time to clear from active_count + time.sleep(1) + + if platform.system() == "Darwin": + # On MacOS, allow caffeinate thread (see ui_controller.py) to continue + # because it won't cause a segfault and takes a long time to exit + baseline_thread_count = 2 + else: + baseline_thread_count = 1 + + assert active_count() <= baseline_thread_count From a2fe153d8d65a993b3d607831168397cc5031b60 Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Thu, 22 Aug 2024 02:50:13 -0600 Subject: [PATCH 12/22] Add pytest to requirements --- requirements-linux.txt | 1 + requirements.txt | 1 + 2 files changed, 2 insertions(+) diff --git a/requirements-linux.txt b/requirements-linux.txt index 62b6a97..ce24078 100644 --- a/requirements-linux.txt +++ b/requirements-linux.txt @@ -3,4 +3,5 @@ opencv-python-headless>=4.9.0.80 keyboard>=0.13.5 PyQt5>=5.15.10 PyQt5-sip>=12.13.0 +pytest>=8.3.2 requests>=2.32.3 diff --git a/requirements.txt b/requirements.txt index 5ae2560..7e4fca4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,4 +3,5 @@ opencv-python>=4.9.0.80 pynput>=1.7.6 PyQt5>=5.15.10 PyQt5-sip>=12.13.0 +pytest>=8.3.2 requests>=2.32.3 From a4ff03b1a42f7ba5c1299779e9d0b473b1bfb213 Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Thu, 22 Aug 2024 02:52:37 -0600 Subject: [PATCH 13/22] Clean up pilgrim_autosplitter.py for cleaner exit 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. --- src/pilgrim_autosplitter.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/pilgrim_autosplitter.py b/src/pilgrim_autosplitter.py index ee6b570..f5fad21 100644 --- a/src/pilgrim_autosplitter.py +++ b/src/pilgrim_autosplitter.py @@ -28,7 +28,6 @@ """Initialize and run Pilgrim Autosplitter.""" - import os import platform import sys @@ -98,14 +97,6 @@ def __init__(self) -> None: self.ui_controller = UIController(self.app, self.splitter) - def run(self) -> None: - """Start and, when finished, safely quit the autosplitter.""" - self.app.exec() - # Prevent segmentation fault or other clumsy errors on exit. - # Subthreads won't persist since they're daemons, but this helps - # make sure they stop BEFORE the main thread ends. - self.splitter.safe_exit_all_threads() - self.ui_controller.keyboard.stop_listener() def main(): """Initialize PilgrimAutosplitter.""" @@ -117,8 +108,16 @@ def main(): pilgrim_autosplitter = PilgrimAutosplitter() + # Close threads safely (these sometimes cause segfaults otherwise), even + # though they are daemons. + # Other app threads don't risk segfaults and are daemons, so leave them + # alone. + pilgrim_autosplitter.app.aboutToQuit.connect( + pilgrim_autosplitter.splitter.safe_exit_all_threads + ) + print("Starting...") - pilgrim_autosplitter.run() + pilgrim_autosplitter.app.exec() if __name__ == "__main__": From 9afe9dec1c818ace12fb02613e023090d871c324 Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Thu, 22 Aug 2024 02:54:45 -0600 Subject: [PATCH 14/22] Allow using different QSettings in settings.py 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. --- src/settings.py | 113 +++++++++++++++++++++++------------------------- 1 file changed, 54 insertions(+), 59 deletions(-) diff --git a/src/settings.py b/src/settings.py index b68aec1..93577c9 100644 --- a/src/settings.py +++ b/src/settings.py @@ -26,8 +26,7 @@ # OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -"""Persist and reference user settings and key values. -""" +"""Persist and reference user settings and key values.""" import os @@ -64,7 +63,7 @@ MAX_THRESHOLD = 1 -def get_str(key: str) -> str: +def get_str(key: str, settings: QSettings = settings) -> str: """Return a str from settings, regardless of the stored value's type. Args: @@ -76,7 +75,7 @@ def get_str(key: str) -> str: return str(settings.value(key)) -def get_bool(key: str) -> bool: +def get_bool(key: str, settings: QSettings = settings) -> bool: """Return a bool from settings, regardless of the stored value's type. Args: @@ -91,7 +90,7 @@ def get_bool(key: str) -> bool: return False -def get_int(key: str) -> int: +def get_int(key: str, settings: QSettings = settings) -> int: """Return an int from settings, regardless of the stored value's type. This should only be used on settings for which is_digit would return True, @@ -106,7 +105,7 @@ def get_int(key: str) -> int: return int(settings.value(key)) -def get_float(key: str) -> float: +def get_float(key: str, settings: QSettings = settings) -> float: """Return a float from settings, regardless of the stored value's type. This should only be used to retrieve settings for which float(foo) would @@ -121,7 +120,7 @@ def get_float(key: str) -> float: return float(settings.value(key)) -def set_value(key: str, value: any) -> None: +def set_value(key: str, value: any, settings: QSettings = settings) -> None: """Persist a setting as a str, regardless of the value's type. Strings are preferred because QSettings doesn't remember types on all @@ -134,7 +133,7 @@ def set_value(key: str, value: any) -> None: settings.setValue(key, str(value)) -def set_defaults() -> None: +def set_defaults(settings: QSettings = settings) -> None: """Ensure that settings values make sense before they are used. Populates settings with default values if they have not yet been set. @@ -147,142 +146,138 @@ def set_defaults() -> None: Makes sure that the correct aspect ratio is shown. """ home_dir = get_home_dir() - if not get_bool("SETTINGS_SET"): + if not get_bool("SETTINGS_SET", settings=settings): # Indicate whether default settings have been populated - set_value("SETTINGS_SET", True) + set_value("SETTINGS_SET", True, settings=settings) # The default minimum match percent needed to force a split action - set_value("DEFAULT_THRESHOLD", 0.90) + set_value("DEFAULT_THRESHOLD", 0.90, settings=settings) # The default delay (seconds) before a split - set_value("DEFAULT_DELAY", 0.0) + set_value("DEFAULT_DELAY", 0.0, settings=settings) # The default pause (seconds) after a split - set_value("DEFAULT_PAUSE", 1.0) + set_value("DEFAULT_PAUSE", 1.0, settings=settings) # The FPS used by splitter and ui_controller - set_value("FPS", 30) + set_value("FPS", 30, settings=settings) # The location of split images - set_value("LAST_IMAGE_DIR", home_dir) + set_value("LAST_IMAGE_DIR", home_dir, settings=settings) # Determine whether screenshots should be opened using the machine's # default image viewer after capture - set_value("OPEN_SCREENSHOT_ON_CAPTURE", False) + set_value("OPEN_SCREENSHOT_ON_CAPTURE", False, settings=settings) # The number of decimal places shown when displaying match percents - set_value("MATCH_PERCENT_DECIMALS", 0) + set_value("MATCH_PERCENT_DECIMALS", 0, settings=settings) # The text value of the split hotkey - set_value("SPLIT_HOTKEY_NAME", "") + set_value("SPLIT_HOTKEY_NAME", "", settings=settings) # The text value of the reset hotkey - set_value("RESET_HOTKEY_NAME", "") + set_value("RESET_HOTKEY_NAME", "", settings=settings) # The text value of the pause hotkey - set_value("PAUSE_HOTKEY_NAME", "") + set_value("PAUSE_HOTKEY_NAME", "", settings=settings) # The text value of the undo hotkey - set_value("UNDO_HOTKEY_NAME", "") + set_value("UNDO_HOTKEY_NAME", "", settings=settings) # The text value of the skip split hotkey - set_value("SKIP_HOTKEY_NAME", "") + set_value("SKIP_HOTKEY_NAME", "", settings=settings) # The text value of the previous split hotkey - set_value("PREV_HOTKEY_NAME", "") + set_value("PREV_HOTKEY_NAME", "", settings=settings) # The text value of the next split hotkey - set_value("NEXT_HOTKEY_NAME", "") + set_value("NEXT_HOTKEY_NAME", "", settings=settings) # The text value of the screenshot hotkey - set_value("SCREENSHOT_HOTKEY_NAME", "") + set_value("SCREENSHOT_HOTKEY_NAME", "", settings=settings) # The text value of the toggle global hotkeys hotkey - set_value("TOGGLE_HOTKEYS_HOTKEY_NAME", "") + set_value("TOGGLE_HOTKEYS_HOTKEY_NAME", "", settings=settings) # The pynput.keyboard.Key.vk value of the split hotkey set_value("SPLIT_HOTKEY_CODE", "") # The pynput.keyboard.Key.vk value of the reset hotkey - set_value("RESET_HOTKEY_CODE", "") + set_value("RESET_HOTKEY_CODE", "", settings=settings) # The pynput.keyboard.Key.vk value of the pause hotkey - set_value("PAUSE_HOTKEY_CODE", "") + set_value("PAUSE_HOTKEY_CODE", "", settings=settings) # The pynput.keyboard.Key.vk value of the undo split hotkey - set_value("UNDO_HOTKEY_CODE", "") + set_value("UNDO_HOTKEY_CODE", "", settings=settings) # The pynput.keyboard.Key.vk value of the skip split hotkey - set_value("SKIP_HOTKEY_CODE", "") + set_value("SKIP_HOTKEY_CODE", "", settings=settings) # The pynput.keyboard.Key.vk value of the previous split hotkey - set_value("PREV_HOTKEY_CODE", "") + set_value("PREV_HOTKEY_CODE", "", settings=settings) # The pynput.keyboard.Key.vk value of the next split hotkey - set_value("NEXT_HOTKEY_CODE", "") + set_value("NEXT_HOTKEY_CODE", "", settings=settings) # The pynput.keyboard.Key.vk value of the screenshot hotkey - set_value("SCREENSHOT_HOTKEY_CODE", "") + set_value("SCREENSHOT_HOTKEY_CODE", "", settings=settings) # The pynput.keyboard.Key.vk value of the toggle global hotkeys hotkey - set_value("TOGGLE_HOTKEYS_HOTKEY_CODE", "") + set_value("TOGGLE_HOTKEYS_HOTKEY_CODE", "", settings=settings) # The UI theme - set_value("THEME", "dark") + set_value("THEME", "dark", settings=settings) # The aspect ratio - set_value("ASPECT_RATIO", "4:3 (480x360)") + set_value("ASPECT_RATIO", "4:3 (480x360)", settings=settings) # Always on top value - set_value("ALWAYS_ON_TOP", False) + set_value("ALWAYS_ON_TOP", False, settings=settings) # The last cv2 capture source index. This is an imperfect way to # remember the last video source used, since the indexes can reference # different sources at different times, but it's the best I can do in a # cross-platform environment - set_value("LAST_CAPTURE_SOURCE_INDEX", 0) + set_value("LAST_CAPTURE_SOURCE_INDEX", 0, settings=settings) # Whether the program should try to open video on startup, or wait for # the user to press "reconnect video" - set_value("START_WITH_VIDEO", False) + set_value("START_WITH_VIDEO", False, settings=settings) # Whether the minimal view should be showing - set_value("SHOW_MIN_VIEW", False) + set_value("SHOW_MIN_VIEW", False, settings=settings) # Whether global hotkeys are enabled (default) or only local hotkeys - set_value("GLOBAL_HOTKEYS_ENABLED", True) + set_value("GLOBAL_HOTKEYS_ENABLED", True, settings=settings) # Whether program checks for updates on launch - set_value("CHECK_FOR_UPDATES", True) + set_value("CHECK_FOR_UPDATES", True, settings=settings) # Make sure image dir exists and is within the user's home dir # (This limits i/o to user-controlled areas) - last_image_dir = get_str("LAST_IMAGE_DIR") + last_image_dir = get_str("LAST_IMAGE_DIR", settings=settings) if not last_image_dir.startswith(home_dir) or not Path(last_image_dir).is_dir(): - set_value("LAST_IMAGE_DIR", home_dir) + set_value("LAST_IMAGE_DIR", home_dir, settings=settings) # Always start in full view if video doesn't come on automatically - if not get_bool("START_WITH_VIDEO"): - set_value("SHOW_MIN_VIEW", False) + if not get_bool("START_WITH_VIDEO", settings=settings): + set_value("SHOW_MIN_VIEW", False, settings=settings) # Set correct video, split image width and height relative to aspect ratio - aspect_ratio = get_str("ASPECT_RATIO") + aspect_ratio = get_str("ASPECT_RATIO", settings=settings) if aspect_ratio == "4:3 (480x360)": - set_value("ASPECT_RATIO", "4:3 (480x360)") - set_value("FRAME_WIDTH", 480) - set_value("FRAME_HEIGHT", 360) + set_value("FRAME_WIDTH", 480, settings=settings) + set_value("FRAME_HEIGHT", 360, settings=settings) elif aspect_ratio == "4:3 (320x240)": - set_value("ASPECT_RATIO", "4:3 (320x240)") - set_value("FRAME_WIDTH", 320) - set_value("FRAME_HEIGHT", 240) + set_value("FRAME_WIDTH", 320, settings=settings) + set_value("FRAME_HEIGHT", 240, settings=settings) elif aspect_ratio == "16:9 (512x288)": - set_value("ASPECT_RATIO", "16:9 (512x288)") - set_value("FRAME_WIDTH", 512) - set_value("FRAME_HEIGHT", 288) + set_value("FRAME_WIDTH", 512, settings=settings) + set_value("FRAME_HEIGHT", 288, settings=settings) elif aspect_ratio == "16:9 (432x243)": - set_value("ASPECT_RATIO", "16:9 (432x243)") - set_value("FRAME_WIDTH", 432) - set_value("FRAME_HEIGHT", 243) + set_value("FRAME_WIDTH", 432, settings=settings) + set_value("FRAME_HEIGHT", 243, settings=settings) def get_latest_version() -> str: From 353ebbad82d1d77ef50dc6abbfd10d4c4f406643 Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Thu, 22 Aug 2024 02:56:47 -0600 Subject: [PATCH 15/22] Refactor splitter.py -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 --- src/splitter/splitter.py | 99 +++++++++++++++++++++++----------------- 1 file changed, 58 insertions(+), 41 deletions(-) diff --git a/src/splitter/splitter.py b/src/splitter/splitter.py index 154f44b..9e18edf 100644 --- a/src/splitter/splitter.py +++ b/src/splitter/splitter.py @@ -26,14 +26,12 @@ # OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -"""Capture video and compare it to a template image. -""" - +"""Capture video and compare it to a template image.""" import platform import threading import time -from typing import Tuple +from typing import Optional, Tuple import cv2 import numpy @@ -170,7 +168,7 @@ def safe_exit_compare_thread(self) -> None: self._compare_thread.join() self.suspended = True - def set_next_capture_index(self) -> None: + def set_next_capture_index(self) -> bool: """Try to find the next valid cv2 capture index, if it exists. cv2 capture indexes can be separated by invalid indexes (for example, 0 @@ -178,6 +176,9 @@ def set_next_capture_index(self) -> None: method will try 3 invalid indexes before returning to index 0. Saves the new index to settings, has no return value. + + Returns: + bool: True if a new source was found. """ source = settings.get_int("LAST_CAPTURE_SOURCE_INDEX") + 1 found_valid_source = False @@ -199,18 +200,15 @@ def set_next_capture_index(self) -> None: source = 0 # Give up, go back to first possible index settings.set_value("LAST_CAPTURE_SOURCE_INDEX", source) + return found_valid_source + def toggle_suspended(self) -> None: - """Stop _compare_thread if it's running; else, start it if possible. + """Stop _compare_thread if running, or start it if there are splits.""" + compare_thread_was_alive = self._compare_thread.is_alive() + self.safe_exit_compare_thread() - Use self.match_percent, since it will never be None if compare_thread - is alive. - """ - if self.match_percent is None: - self.safe_exit_compare_thread() - if len(self.splits.list) > 0: - self.start_compare_thread() - else: - self.safe_exit_compare_thread() + if not compare_thread_was_alive and len(self.splits.list) > 0: + self.start_compare_thread() ################################### # # @@ -225,6 +223,7 @@ def _start_capture_thread(self) -> None: safely exited before calling this method. """ self._cap = self._open_capture() + self._capture_max_fps = self._get_max_fps(self._cap) self.capture_thread = threading.Thread(target=self._capture) self._capture_thread_finished = False self.capture_thread.daemon = True @@ -240,12 +239,6 @@ def _open_capture(self) -> cv2.VideoCapture: forces the capture source to choose the next-closest value, which in some cases is quite a lot smaller than the default. This saves CPU. - Set self._capture_max_fps to the capture's maximum FPS on platforms - where this is supported. This is done to prevent unnecessary - comparisons from being generated in _compare due to a mismatch between - the user's selected framerate and a capture-imposed cap which is lower. - This also saves CPU. - Returns: cv2.VideoCapture: The initialized and configured VideoCapture. """ @@ -260,19 +253,35 @@ def _open_capture(self) -> cv2.VideoCapture: cap.set(cv2.CAP_PROP_BUFFERSIZE, 1) cap.set(cv2.CAP_PROP_FRAME_WIDTH, COMPARISON_FRAME_WIDTH) cap.set(cv2.CAP_PROP_FRAME_HEIGHT, COMPARISON_FRAME_HEIGHT) + return cap + + def _get_max_fps(self, cap: cv2.VideoCapture) -> int: + """Get the max FPS of a capture source. + + Set self._capture_max_fps to the capture's maximum FPS on platforms + where this is supported. This is done to prevent unnecessary + comparisons from being generated in _compare due to a mismatch between + the user's selected framerate and a capture-imposed cap which is lower. + This also saves CPU. + + Args: + cap (cv2.VideoCapture): The capture source to check. + + Returns: + int: The max frames per second. + """ try: - capture_max_fps = cap.get(cv2.CAP_PROP_FPS) - if capture_max_fps == 0: - self._capture_max_fps = 60 - else: - self._capture_max_fps = capture_max_fps + max_fps = cap.get(cv2.CAP_PROP_FPS) + if max_fps == 0: + max_fps = 60 except cv2.error: - self._capture_max_fps = 60 - return cap + max_fps = 60 + + return max_fps def _capture(self) -> None: """Read frames from a capture source, resize them, and expose them to - _compare and ui_controller. + _compare and ui_controller in a loop. self.comparison_frame should always be 320x240. This helps with results consistency when matching split images; it also saves a lot of time and @@ -292,20 +301,17 @@ def _capture(self) -> None: which makes it a good choice for image matching. This method continues indefinitely until self._capture_thread_finished - is set. + is set to True. """ start_time = time.perf_counter() while not self._capture_thread_finished: # We don't need to wait if we are hitting the max capture fps, # since cap.read() blocks until the next frame is available anyway. - # So we only do this if we are targeting a framerate that is lower - # than the max fps + # So we only wait on the interval if we are targeting a framerate + # that is lower than the max fps. if self.target_fps < self._capture_max_fps: - current_time = time.perf_counter() - if current_time - start_time < self._interval: - time.sleep(self._interval - (current_time - start_time)) - start_time = time.perf_counter() + start_time = self._wait_for_interval(start_time) frame = self._cap.read()[1] if frame is None: # Video feed is down, kill the thread @@ -355,7 +361,7 @@ def _capture(self) -> None: # Kill comparer if capture goes down self.safe_exit_compare_thread() - def _frame_to_pixmap(self, frame: numpy.ndarray) -> QPixmap: + def _frame_to_pixmap(self, frame: Optional[numpy.ndarray]) -> QPixmap: """Generate a QPixmap instance from a 3-channel image stored as a numpy array. @@ -373,6 +379,20 @@ def _frame_to_pixmap(self, frame: numpy.ndarray) -> QPixmap: frame_img = QImage(frame, frame.shape[1], frame.shape[0], QImage.Format_BGR888) return QPixmap.fromImage(frame_img) + def _wait_for_interval(self, start_time: float): + """Sleep until self._interval seconds has passed since start_time. + + Args: + start_time (float): The starting time. + + Returns: + float: The current time after sleeping. + """ + current_time = time.perf_counter() + if current_time - start_time < self._interval: + time.sleep(self._interval - (current_time - start_time)) + return time.perf_counter() + ################################### # # # Private _compare_thread Methods # @@ -423,10 +443,7 @@ def _look_for_match(self) -> bool: start_time = frame_counter_start_time = time.perf_counter() while not self._compare_thread_finished: - current_time = time.perf_counter() - if current_time - start_time < self._interval: - time.sleep(self._interval - (current_time - start_time)) - start_time = time.perf_counter() + start_time = self._wait_for_interval(start_time) # Update self._interval to better track target framerate frames_this_second, frame_counter_start_time = self._update_fps_factor( From e63c554a39555da6b3e3ffa04fc579b742fa9bad Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Thu, 22 Aug 2024 03:00:55 -0600 Subject: [PATCH 16/22] Remove stop_listener from keyboard_controller 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. --- src/ui/ui_keyboard_controller.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/ui/ui_keyboard_controller.py b/src/ui/ui_keyboard_controller.py index 9aa174d..ad80300 100644 --- a/src/ui/ui_keyboard_controller.py +++ b/src/ui/ui_keyboard_controller.py @@ -52,7 +52,6 @@ class UIKeyboardController: def __init__(self) -> None: """Initialize the keyboard controller (required for pynput backend).""" - self._keyboard_listener = None if platform.system() == "Windows" or platform.system() == "Darwin": self._controller = pynput_keyboard.Controller() else: @@ -81,20 +80,14 @@ def start_listener( on_release = self._do_nothing if platform.system() == "Windows" or platform.system() == "Darwin": - self._keyboard_listener = pynput_keyboard.Listener( + keyboard_listener = pynput_keyboard.Listener( on_press=on_press, on_release=on_release ) - self._keyboard_listener.start() + keyboard_listener.start() else: keyboard.on_press(on_press) keyboard.on_release(on_release) - def stop_listener(self) -> None: - if platform.system() == "Windows" or platform.system() == "Darwin": - self._keyboard_listener.stop() - else: - pass - def press_and_release(self, key_code: str) -> None: """Press and release a hotkey. From 6ab0429d75df80b5528d14209d0b01d81c8bd6bc Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Thu, 22 Aug 2024 03:03:32 -0600 Subject: [PATCH 17/22] Simplify test_pilgrim_autosplitter --- tests/test_pilgrim_autosplitter.py | 42 +++++++----------------------- 1 file changed, 10 insertions(+), 32 deletions(-) diff --git a/tests/test_pilgrim_autosplitter.py b/tests/test_pilgrim_autosplitter.py index 9f2204f..06b60fa 100644 --- a/tests/test_pilgrim_autosplitter.py +++ b/tests/test_pilgrim_autosplitter.py @@ -28,39 +28,17 @@ """Test pilgrim_autosplitter.py.""" - -import platform -import time -from threading import active_count, Thread - from PyQt5.QtWidgets import QApplication -from src.pilgrim_autosplitter import PilgrimAutosplitter - - -def test_run() -> None: - """Make sure app opens correctly and shuts down cleanly (kills threads).""" - def quit_app(app: QApplication) -> None: - time.sleep(2) - app.quit() - - pilgrim_autosplitter = PilgrimAutosplitter() - - exit_gui_thread = Thread(target=quit_app, args=(pilgrim_autosplitter.app,)) - exit_gui_thread.daemon = True - exit_gui_thread.start() - - pilgrim_autosplitter.run() - exit_gui_thread.join() - - # Give joined threads time to clear from active_count - time.sleep(1) +from pilgrim_autosplitter import PilgrimAutosplitter +from splitter.splitter import Splitter +from ui.ui_controller import UIController - if platform.system() == "Darwin": - # On MacOS, allow caffeinate thread (see ui_controller.py) to continue - # because it won't cause a segfault and takes a long time to exit - baseline_thread_count = 2 - else: - baseline_thread_count = 1 - assert active_count() <= baseline_thread_count +def test_PilgrimAutosplitter(): + app = PilgrimAutosplitter() + assert ( + type(app.app) == QApplication + and type(app.splitter) == Splitter + and type(app.ui_controller) == UIController + ) From a61d221e853e46fcd3061e66c021b5618f724f44 Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Thu, 22 Aug 2024 03:04:31 -0600 Subject: [PATCH 18/22] Add tests for splitter.py, settings.py --- tests/test_settings.py | 239 +++++++++++++++++++++++++++ tests/test_splitter/__init__.py | 6 + tests/test_splitter/test_splitter.py | 163 ++++++++++++++++++ tests/test_ui/__init__.py | 6 + 4 files changed, 414 insertions(+) create mode 100644 tests/test_settings.py create mode 100644 tests/test_splitter/__init__.py create mode 100644 tests/test_splitter/test_splitter.py create mode 100644 tests/test_ui/__init__.py diff --git a/tests/test_settings.py b/tests/test_settings.py new file mode 100644 index 0000000..94d38ec --- /dev/null +++ b/tests/test_settings.py @@ -0,0 +1,239 @@ +# Copyright (c) 2024 pilgrim_tabby +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +"""Test settings.py.""" + +import pytest +import requests +from pathlib import Path +from PyQt5.QtCore import QSettings + +import settings + + +class TestURLs: + """Test whether defined URLs lead to live sites.""" + + def get_url_code(self, url: str): + return requests.head(url).status_code + + def test_repo_url_live(self): + assert self.get_url_code(settings.REPO_URL) == 200 + + def test_user_manual_url_live(self): + assert self.get_url_code(settings.USER_MANUAL_URL) == 200 + + +class TestSettingsFunctionsWithDummySettings: + """Test functions in settings.py that rely on a "settings" var.""" + + @pytest.fixture(autouse=True) + def dummy_settings(self): + """Provide blank QSettings file for testing. + + Yields: + QSettings: The blank QSettings file. + """ + self.dummy_settings = QSettings("pilgrim_tabby", "pilgrim_autosplitter_test") + self.dummy_settings.clear() + yield self.dummy_settings + self.dummy_settings.clear() + + # Testing set_value + def test_set_value_str(self): + settings.set_value("test", "hello world", settings=self.dummy_settings) + assert self.dummy_settings.value("test") == "hello world" + + def test_set_value_int(self): + settings.set_value("test", 1337, settings=self.dummy_settings) + assert self.dummy_settings.value("test") == "1337" + + def test_set_value_blank(self): + settings.set_value("test", "", settings=self.dummy_settings) + assert self.dummy_settings.value("test") == "" + + # Testing get_str + def test_get_str_defined_val(self): + self.dummy_settings.setValue("test", "foo") + assert settings.get_str("test", settings=self.dummy_settings) == "foo" + + def test_get_str_unset_val(self): + assert settings.get_str("test", settings=self.dummy_settings) == "None" + + def test_get_str_blank_val(self): + self.dummy_settings.setValue("test", "") + assert settings.get_str("test", settings=self.dummy_settings) == "" + + def test_get_str_from_int(self): + self.dummy_settings.setValue("test", 123) + assert settings.get_str("test", settings=self.dummy_settings) == "123" + + # Testing get_bool + def test_get_bool_from_str_1(self): + self.dummy_settings.setValue("test", "True") + assert settings.get_bool("test", settings=self.dummy_settings) == True + + def test_get_bool_from_str_2(self): + self.dummy_settings.setValue("test", "true") + assert settings.get_bool("test", settings=self.dummy_settings) == False + + def test_get_bool_from_str_3(self): + self.dummy_settings.setValue("test", "False") + assert settings.get_bool("test", settings=self.dummy_settings) == False + + def test_get_bool_from_random_val(self): + self.dummy_settings.setValue("test", 1.23456543) + assert settings.get_bool("test", settings=self.dummy_settings) == False + + def test_get_bool_from_unset_val(self): + assert settings.get_bool("test", settings=self.dummy_settings) == False + + def test_get_bool_from_blank_val(self): + self.dummy_settings.setValue("test", "") + assert settings.get_bool("test", settings=self.dummy_settings) == False + + # Testing get_int + def test_get_int_from_non_int(self): + self.dummy_settings.setValue("test", "this will fail") + with pytest.raises(ValueError): + settings.get_int("test", settings=self.dummy_settings) + + def test_get_int_from_blank_val(self): + self.dummy_settings.setValue("test", "") + with pytest.raises(ValueError): + settings.get_int("test", settings=self.dummy_settings) + + def test_get_int_from_str_1(self): + self.dummy_settings.setValue("test", "33") + assert settings.get_int("test", settings=self.dummy_settings) == 33 + + def test_get_int_from_str_2(self): + self.dummy_settings.setValue("test", "-33") + assert settings.get_int("test", settings=self.dummy_settings) == -33 + + def test_get_int_from_int(self): + self.dummy_settings.setValue("test", -1) + assert settings.get_int("test", settings=self.dummy_settings) == -1 + + # Testing get_float + def test_get_float_from_str(self): + self.dummy_settings.setValue("test", "this will fail") + with pytest.raises(ValueError): + settings.get_float("test", settings=self.dummy_settings) + + def test_get_float_from_blank_val(self): + self.dummy_settings.setValue("test", "") + with pytest.raises(ValueError): + settings.get_float("test", settings=self.dummy_settings) + + def test_get_float_from_str(self): + self.dummy_settings.setValue("test", "-0") + assert settings.get_float("test", settings=self.dummy_settings) == 0 + + def test_get_float_from_int(self): + self.dummy_settings.setValue("test", 123) + assert settings.get_float("test", settings=self.dummy_settings) == 123 + + def test_get_float_from_float(self): + self.dummy_settings.setValue("test", 123.123123) + assert settings.get_float("test", settings=self.dummy_settings) == 123.123123 + + # Testing set_defaults + def test_set_defaults_runs_on_blank_settings(self): + settings.set_defaults(settings=self.dummy_settings) + assert settings.get_bool("SETTINGS_SET", settings=self.dummy_settings) + + def test_set_defaults_turns_off_show_min_view(self): + settings.set_defaults(settings=self.dummy_settings) + orig_start_with_video = settings.get_bool( + "START_WITH_VIDEO", settings=self.dummy_settings + ) + settings.set_value("SHOW_MIN_VIEW", True, settings=self.dummy_settings) + settings.set_defaults(settings=self.dummy_settings) + assert not orig_start_with_video and not settings.get_bool( + "SHOW_MIN_VIEW", settings=self.dummy_settings + ) + + def test_set_defaults_sets_correct_aspect_ratio_1(self): + settings.set_defaults(settings=self.dummy_settings) + settings.set_value( + "ASPECT_RATIO", "4:3 (480x360)", settings=self.dummy_settings + ) + settings.set_defaults(settings=self.dummy_settings) + assert ( + settings.get_int("FRAME_WIDTH", settings=self.dummy_settings) == 480 + and settings.get_int("FRAME_HEIGHT", settings=self.dummy_settings) == 360 + ) + + def test_set_defaults_sets_correct_aspect_ratio_2(self): + settings.set_defaults(settings=self.dummy_settings) + settings.set_value( + "ASPECT_RATIO", "4:3 (320x240)", settings=self.dummy_settings + ) + settings.set_defaults(settings=self.dummy_settings) + assert ( + settings.get_int("FRAME_WIDTH", settings=self.dummy_settings) == 320 + and settings.get_int("FRAME_HEIGHT", settings=self.dummy_settings) == 240 + ) + + def test_set_defaults_sets_correct_aspect_ratio_3(self): + settings.set_defaults(settings=self.dummy_settings) + settings.set_value( + "ASPECT_RATIO", "16:9 (512x288)", settings=self.dummy_settings + ) + settings.set_defaults(settings=self.dummy_settings) + assert ( + settings.get_int("FRAME_WIDTH", settings=self.dummy_settings) == 512 + and settings.get_int("FRAME_HEIGHT", settings=self.dummy_settings) == 288 + ) + + def test_set_defaults_sets_correct_aspect_ratio_4(self): + settings.set_defaults(settings=self.dummy_settings) + settings.set_value( + "ASPECT_RATIO", "16:9 (432x243)", settings=self.dummy_settings + ) + settings.set_defaults(settings=self.dummy_settings) + assert ( + settings.get_int("FRAME_WIDTH", settings=self.dummy_settings) == 432 + and settings.get_int("FRAME_HEIGHT", settings=self.dummy_settings) == 243 + ) + + +def test_get_latest_version(): + latest_version = settings.get_latest_version() + version_numbers = latest_version.split(".") + assert ( + len(version_numbers) == 3 + and "v" in version_numbers[0] + and version_numbers[1].isdigit() + and version_numbers[2].isdigit() + ) + + +def test_get_home_dir_returns_real_path(): + assert Path(settings.get_home_dir()).is_dir() diff --git a/tests/test_splitter/__init__.py b/tests/test_splitter/__init__.py new file mode 100644 index 0000000..726a5b6 --- /dev/null +++ b/tests/test_splitter/__init__.py @@ -0,0 +1,6 @@ +"""Add files in /src to PATH so they can be tested.""" + +import sys + + +sys.path.append("./src") diff --git a/tests/test_splitter/test_splitter.py b/tests/test_splitter/test_splitter.py new file mode 100644 index 0000000..262a571 --- /dev/null +++ b/tests/test_splitter/test_splitter.py @@ -0,0 +1,163 @@ +# Copyright (c) 2024 pilgrim_tabby +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +"""Test splitter.py.""" + +import time +import cv2 +import pytest +from PyQt5.QtWidgets import QApplication + +import settings +from splitter.splitter import Splitter +from splitter.split_dir import SplitDir + + +class TestSplitter: + """Test Splitter methods using a dummy class instance.""" + + # Required for using QWidgets + dummy_app = QApplication([]) + + @pytest.fixture(autouse=True) + def dummy_splitter(self): + """Spin up dummy Splitter instance with no split images for testing. + + Yields: + Splitter: The Splitter instance. + """ + self.splitter = Splitter() + self.splitter.splits.list = [] + + yield self.splitter + + self.splitter.safe_exit_all_threads() + + def start_capture_and_compare(self): + test_img = "resources/icon-macos.png" + self.splitter.splits.list += [SplitDir._SplitImage(test_img)] + self.splitter.start() + + def is_float_or_int(self, target: str): + arg_list = target.split(".") + for entry in arg_list: + if not entry.isdigit(): + return False + return True + + def test_start_no_splits(self): + self.splitter.start() + assert ( + self.splitter.capture_thread.is_alive() + and not self.splitter._compare_thread.is_alive() + ) + + def test_start_with_splits(self): + self.start_capture_and_compare() + assert ( + self.splitter.capture_thread.is_alive() + and self.splitter._compare_thread.is_alive() + ) + + def test_exit_all_threads(self): + self.start_capture_and_compare() + + capture_thread_orig_status = self.splitter.capture_thread.is_alive() + compare_thread_orig_status = self.splitter._compare_thread.is_alive() + self.splitter.safe_exit_all_threads() + + assert ( + capture_thread_orig_status + and compare_thread_orig_status + and not self.splitter.capture_thread.is_alive() + and not self.splitter._compare_thread.is_alive() + ) + + def test_start_compare_thread(self): + self.splitter.start_compare_thread() + assert self.splitter._compare_thread.is_alive() + + def test_safe_exit_compare_thread(self): + self.start_capture_and_compare() + compare_thread_orig_status = self.splitter._compare_thread.is_alive() + self.splitter.safe_exit_compare_thread() + assert ( + compare_thread_orig_status and not self.splitter._compare_thread.is_alive() + ) + + def test_set_next_capture_index(self): + curr_index = settings.get_int("LAST_CAPTURE_SOURCE_INDEX") + found_new_source = self.splitter.set_next_capture_index() + new_index = settings.get_int("LAST_CAPTURE_SOURCE_INDEX") + assert curr_index != new_index or not found_new_source + + def test_toggle_suspended_on(self): + self.start_capture_and_compare() + self.splitter.toggle_suspended() + assert not self.splitter._compare_thread.is_alive() + + def test_toggle_suspended_off_with_split_image(self): + test_img = "resources/icon-macos.png" + self.splitter.splits.list += [SplitDir._SplitImage(test_img)] + + self.splitter.toggle_suspended() + assert self.splitter._compare_thread.is_alive() + + def test_toggle_suspended_off_without_split_image(self): + self.splitter.toggle_suspended() + assert not self.splitter._compare_thread.is_alive() + + def test_start_capture_thread(self): + capture_thread_orig_status = self.splitter.capture_thread.is_alive() + self.splitter._start_capture_thread() + assert ( + not capture_thread_orig_status and self.splitter.capture_thread.is_alive() + ) + + def test_open_capture(self): + cap = self.splitter._open_capture() + assert type(cap) == cv2.VideoCapture + + def test_get_max_fps(self): + cap = self.splitter._open_capture() + max_fps = self.splitter._get_max_fps(cap) + assert self.is_float_or_int(str(max_fps)) and max_fps >= 0 + + def test_capture(self): + pass + + def test_frame_to_pixmap(self): + pass + + def test_wait_for_interval(self): + start_time = time.perf_counter() + new_time = self.splitter._wait_for_interval(start_time) + + assert ( + pytest.approx(new_time - start_time, abs=0.005) == self.splitter._interval + ) diff --git a/tests/test_ui/__init__.py b/tests/test_ui/__init__.py new file mode 100644 index 0000000..726a5b6 --- /dev/null +++ b/tests/test_ui/__init__.py @@ -0,0 +1,6 @@ +"""Add files in /src to PATH so they can be tested.""" + +import sys + + +sys.path.append("./src") From 5ecf66c3e928786b3b147f49aea00b1517054dab Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Thu, 22 Aug 2024 05:30:53 -0600 Subject: [PATCH 19/22] Fix logic error in toggle_suspended --- src/splitter/splitter.py | 14 +++++++++----- tests/test_splitter/test_splitter.py | 6 ++++++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/splitter/splitter.py b/src/splitter/splitter.py index 9e18edf..2f1c0f6 100644 --- a/src/splitter/splitter.py +++ b/src/splitter/splitter.py @@ -203,12 +203,16 @@ def set_next_capture_index(self) -> bool: return found_valid_source def toggle_suspended(self) -> None: - """Stop _compare_thread if running, or start it if there are splits.""" - compare_thread_was_alive = self._compare_thread.is_alive() - self.safe_exit_compare_thread() + """Stop _compare_thread, then start it if the splitter was suspended + and there are splits. - if not compare_thread_was_alive and len(self.splits.list) > 0: - self.start_compare_thread() + Use self.match_percent, since it will never be None if compare_thread + is alive AND not suspended, which is the condition we're looking for. + """ + current_match_percent = self.match_percent + self.safe_exit_compare_thread() + if current_match_percent is None and len(self.splits.list) > 0: + self.start_compare_thread() ################################### # # diff --git a/tests/test_splitter/test_splitter.py b/tests/test_splitter/test_splitter.py index 262a571..8d62c89 100644 --- a/tests/test_splitter/test_splitter.py +++ b/tests/test_splitter/test_splitter.py @@ -128,6 +128,12 @@ def test_toggle_suspended_off_with_split_image(self): self.splitter.toggle_suspended() assert self.splitter._compare_thread.is_alive() + def test_toggle_suspended_off_during_split_delay(self): + pass + + def test_toggle_suspended_off_during_split_suspend(self): + pass + def test_toggle_suspended_off_without_split_image(self): self.splitter.toggle_suspended() assert not self.splitter._compare_thread.is_alive() From af63590be37f2b6088a6ff6bd9d4c8f5109d8461 Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Thu, 22 Aug 2024 05:34:25 -0600 Subject: [PATCH 20/22] Make self.keyboard private in controller --- src/ui/ui_controller.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/ui/ui_controller.py b/src/ui/ui_controller.py index 6957a19..750e503 100644 --- a/src/ui/ui_controller.py +++ b/src/ui/ui_controller.py @@ -237,8 +237,8 @@ def __init__(self, application: QApplication, splitter: Splitter) -> None: ####################################### # Start keyboard listener - self.keyboard = UIKeyboardController() - self.keyboard.start_listener(on_press=self._handle_key_press, on_release=None) + self._keyboard = UIKeyboardController() + self._keyboard.start_listener(on_press=self._handle_key_press, on_release=None) # Start poller self._poller = QTimer() @@ -285,7 +285,7 @@ def _attempt_undo_hotkey(self) -> None: """ key_code = settings.get_str("UNDO_HOTKEY_CODE") if len(key_code) > 0: - self.keyboard.press_and_release(key_code) + self._keyboard.press_and_release(key_code) else: self._request_previous_split() @@ -309,7 +309,7 @@ def _attempt_skip_hotkey(self) -> None: """ key_code = settings.get_str("SKIP_HOTKEY_CODE") if len(key_code) > 0: - self.keyboard.press_and_release(key_code) + self._keyboard.press_and_release(key_code) else: self._request_next_split() @@ -333,7 +333,7 @@ def _attempt_reset_hotkey(self) -> None: """ key_code = settings.get_str("RESET_HOTKEY_CODE") if len(key_code) > 0: - self.keyboard.press_and_release(key_code) + self._keyboard.press_and_release(key_code) else: self._request_reset_splits() @@ -1639,7 +1639,7 @@ def _handle_key_press( """ # Get the key's name and internal value. If the key is not an # alphanumeric key, the try block throws AttributeError. - key_name, key_code = self.keyboard.parse_key_info(key) + key_name, key_code = self._keyboard.parse_key_info(key) # Some keys aren't handled very well by pynput -- they return a key # code but no name. I don't have the resources to compile an exhaustive @@ -1798,7 +1798,7 @@ def _react_to_split_flags(self) -> None: self._splitter.pause_split_action = False key_code = settings.get_str("PAUSE_HOTKEY_CODE") if len(key_code) > 0: - self.keyboard.press_and_release(key_code) + self._keyboard.press_and_release(key_code) self._request_next_split() # Dummy split (silently advance to next split) @@ -1811,7 +1811,7 @@ def _react_to_split_flags(self) -> None: self._splitter.normal_split_action = False key_code = settings.get_str("SPLIT_HOTKEY_CODE") if len(key_code) > 0: - self.keyboard.press_and_release(key_code) + self._keyboard.press_and_release(key_code) # If key didn't get pressed, OR if it did get pressed but global # hotkeys are off and the app isn't in focus, move the split image # forward, since pressing the key on its own won't do that @@ -1879,7 +1879,7 @@ def _wake_display(self): # No caffeinate, use fallback if caffeinate_path is None: - self.keyboard.release(key) + self._keyboard.release(key) # Caffeinate exists; start thread if it hasn't been started elif not self._caffeinate_thread.is_alive(): @@ -1896,7 +1896,7 @@ def _wake_display(self): # Winodws / Linux: release a key if splitter active elif splitter_active: - self.keyboard.release(key) + self._keyboard.release(key) def _caffeinate(self, caffeinate_path: str) -> None: """Use built-in caffeinate to keep the machine's display on (MacOS). From dc4b7537233c64c8ef8fe23f1b704ec171fe4d9c Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Thu, 22 Aug 2024 05:47:32 -0600 Subject: [PATCH 21/22] Add .pytest_cache to .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 32626b4..2407247 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ **/_site **/.jekyll-cache pilgrim_autosplitter.spec +**/.pytest_cache From 259e4bbeab2594fa370c85a404be642216647c27 Mon Sep 17 00:00:00 2001 From: pilgrim_tabby Date: Mon, 14 Oct 2024 18:05:00 -0600 Subject: [PATCH 22/22] Unset hotkeys when upgrading to v1.0.7 - 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) --- src/pilgrim_autosplitter.py | 2 +- src/settings.py | 188 +++++++++++++++++++----------------- src/splitter/splitter.py | 2 +- src/ui/ui_main_window.py | 2 +- tests/test_settings.py | 69 +++++++++---- 5 files changed, 155 insertions(+), 108 deletions(-) diff --git a/src/pilgrim_autosplitter.py b/src/pilgrim_autosplitter.py index f5fad21..75809e8 100644 --- a/src/pilgrim_autosplitter.py +++ b/src/pilgrim_autosplitter.py @@ -89,7 +89,7 @@ def __init__(self) -> None: QIcon(QPixmap(f"{program_directory}/../resources/icon-macos.png")) ) - settings.set_defaults() + settings.set_program_vals() self.splitter = Splitter() if settings.get_bool("START_WITH_VIDEO"): diff --git a/src/settings.py b/src/settings.py index 93577c9..755641d 100644 --- a/src/settings.py +++ b/src/settings.py @@ -133,8 +133,11 @@ def set_value(key: str, value: any, settings: QSettings = settings) -> None: settings.setValue(key, str(value)) -def set_defaults(settings: QSettings = settings) -> None: - """Ensure that settings values make sense before they are used. +def set_program_vals(settings: QSettings = settings) -> None: + """Ensure that settings values are updated and make sense before use. + + Unsets hotkeys if last used version was <=1.0.6 due to a change in the way + hotkeys are implemented. Populates settings with default values if they have not yet been set. @@ -146,138 +149,149 @@ def set_defaults(settings: QSettings = settings) -> None: Makes sure that the correct aspect ratio is shown. """ home_dir = get_home_dir() - if not get_bool("SETTINGS_SET", settings=settings): - # Indicate whether default settings have been populated - set_value("SETTINGS_SET", True, settings=settings) + + # Unset hotkeys if upgrading from <=v1.0.6 because of hotkey implementation + # updates + last_version = get_str("LAST_VERSION", settings) + if last_version == "None": + last_version = "v1.0.0" + if not version_ge(last_version, "v1.0.7"): + unset_hotkey_bindings() + + if not get_bool("SETTINGS_SET", settings): + # Indicate that default settings have been populated + set_value("SETTINGS_SET", True, settings) + + # Set hotkeys to default values + unset_hotkey_bindings() # The default minimum match percent needed to force a split action - set_value("DEFAULT_THRESHOLD", 0.90, settings=settings) + set_value("DEFAULT_THRESHOLD", 0.90, settings) # The default delay (seconds) before a split - set_value("DEFAULT_DELAY", 0.0, settings=settings) + set_value("DEFAULT_DELAY", 0.0, settings) # The default pause (seconds) after a split - set_value("DEFAULT_PAUSE", 1.0, settings=settings) + set_value("DEFAULT_PAUSE", 1.0, settings) # The FPS used by splitter and ui_controller - set_value("FPS", 30, settings=settings) + set_value("FPS", 30, settings) # The location of split images - set_value("LAST_IMAGE_DIR", home_dir, settings=settings) + set_value("LAST_IMAGE_DIR", home_dir, settings) # Determine whether screenshots should be opened using the machine's # default image viewer after capture - set_value("OPEN_SCREENSHOT_ON_CAPTURE", False, settings=settings) + set_value("OPEN_SCREENSHOT_ON_CAPTURE", False, settings) # The number of decimal places shown when displaying match percents - set_value("MATCH_PERCENT_DECIMALS", 0, settings=settings) - - # The text value of the split hotkey - set_value("SPLIT_HOTKEY_NAME", "", settings=settings) - - # The text value of the reset hotkey - set_value("RESET_HOTKEY_NAME", "", settings=settings) - - # The text value of the pause hotkey - set_value("PAUSE_HOTKEY_NAME", "", settings=settings) - - # The text value of the undo hotkey - set_value("UNDO_HOTKEY_NAME", "", settings=settings) - - # The text value of the skip split hotkey - set_value("SKIP_HOTKEY_NAME", "", settings=settings) - - # The text value of the previous split hotkey - set_value("PREV_HOTKEY_NAME", "", settings=settings) - - # The text value of the next split hotkey - set_value("NEXT_HOTKEY_NAME", "", settings=settings) - - # The text value of the screenshot hotkey - set_value("SCREENSHOT_HOTKEY_NAME", "", settings=settings) - - # The text value of the toggle global hotkeys hotkey - set_value("TOGGLE_HOTKEYS_HOTKEY_NAME", "", settings=settings) - - # The pynput.keyboard.Key.vk value of the split hotkey - set_value("SPLIT_HOTKEY_CODE", "") - - # The pynput.keyboard.Key.vk value of the reset hotkey - set_value("RESET_HOTKEY_CODE", "", settings=settings) - - # The pynput.keyboard.Key.vk value of the pause hotkey - set_value("PAUSE_HOTKEY_CODE", "", settings=settings) - - # The pynput.keyboard.Key.vk value of the undo split hotkey - set_value("UNDO_HOTKEY_CODE", "", settings=settings) - - # The pynput.keyboard.Key.vk value of the skip split hotkey - set_value("SKIP_HOTKEY_CODE", "", settings=settings) - - # The pynput.keyboard.Key.vk value of the previous split hotkey - set_value("PREV_HOTKEY_CODE", "", settings=settings) - - # The pynput.keyboard.Key.vk value of the next split hotkey - set_value("NEXT_HOTKEY_CODE", "", settings=settings) - - # The pynput.keyboard.Key.vk value of the screenshot hotkey - set_value("SCREENSHOT_HOTKEY_CODE", "", settings=settings) - - # The pynput.keyboard.Key.vk value of the toggle global hotkeys hotkey - set_value("TOGGLE_HOTKEYS_HOTKEY_CODE", "", settings=settings) + set_value("MATCH_PERCENT_DECIMALS", 0, settings) # The UI theme - set_value("THEME", "dark", settings=settings) + set_value("THEME", "dark", settings) # The aspect ratio - set_value("ASPECT_RATIO", "4:3 (480x360)", settings=settings) + set_value("ASPECT_RATIO", "4:3 (480x360)", settings) # Always on top value - set_value("ALWAYS_ON_TOP", False, settings=settings) + set_value("ALWAYS_ON_TOP", False, settings) # The last cv2 capture source index. This is an imperfect way to # remember the last video source used, since the indexes can reference # different sources at different times, but it's the best I can do in a # cross-platform environment - set_value("LAST_CAPTURE_SOURCE_INDEX", 0, settings=settings) + set_value("LAST_CAPTURE_SOURCE_INDEX", 0, settings) # Whether the program should try to open video on startup, or wait for # the user to press "reconnect video" - set_value("START_WITH_VIDEO", False, settings=settings) + set_value("START_WITH_VIDEO", False, settings) # Whether the minimal view should be showing - set_value("SHOW_MIN_VIEW", False, settings=settings) + set_value("SHOW_MIN_VIEW", False, settings) # Whether global hotkeys are enabled (default) or only local hotkeys - set_value("GLOBAL_HOTKEYS_ENABLED", True, settings=settings) + set_value("GLOBAL_HOTKEYS_ENABLED", True, settings) # Whether program checks for updates on launch - set_value("CHECK_FOR_UPDATES", True, settings=settings) + set_value("CHECK_FOR_UPDATES", True, settings) # Make sure image dir exists and is within the user's home dir # (This limits i/o to user-controlled areas) - last_image_dir = get_str("LAST_IMAGE_DIR", settings=settings) + last_image_dir = get_str("LAST_IMAGE_DIR", settings) if not last_image_dir.startswith(home_dir) or not Path(last_image_dir).is_dir(): - set_value("LAST_IMAGE_DIR", home_dir, settings=settings) + set_value("LAST_IMAGE_DIR", home_dir, settings) # Always start in full view if video doesn't come on automatically - if not get_bool("START_WITH_VIDEO", settings=settings): - set_value("SHOW_MIN_VIEW", False, settings=settings) + if not get_bool("START_WITH_VIDEO", settings): + set_value("SHOW_MIN_VIEW", False, settings) + + # Remember last used version number (to unset hotkeys if upgrading) + set_value("LAST_VERSION", VERSION_NUMBER, settings) # Set correct video, split image width and height relative to aspect ratio - aspect_ratio = get_str("ASPECT_RATIO", settings=settings) + aspect_ratio = get_str("ASPECT_RATIO", settings) if aspect_ratio == "4:3 (480x360)": - set_value("FRAME_WIDTH", 480, settings=settings) - set_value("FRAME_HEIGHT", 360, settings=settings) + set_value("FRAME_WIDTH", 480, settings) + set_value("FRAME_HEIGHT", 360, settings) elif aspect_ratio == "4:3 (320x240)": - set_value("FRAME_WIDTH", 320, settings=settings) - set_value("FRAME_HEIGHT", 240, settings=settings) + set_value("FRAME_WIDTH", 320, settings) + set_value("FRAME_HEIGHT", 240, settings) elif aspect_ratio == "16:9 (512x288)": - set_value("FRAME_WIDTH", 512, settings=settings) - set_value("FRAME_HEIGHT", 288, settings=settings) + set_value("FRAME_WIDTH", 512, settings) + set_value("FRAME_HEIGHT", 288, settings) elif aspect_ratio == "16:9 (432x243)": - set_value("FRAME_WIDTH", 432, settings=settings) - set_value("FRAME_HEIGHT", 243, settings=settings) + set_value("FRAME_WIDTH", 432, settings) + set_value("FRAME_HEIGHT", 243, settings) + + +def version_ge(version1: str, version2: str) -> bool: + """Check whether version1 equal to or more recent than version2. + + Versions are in format x.x.x or vx.x.x, where x is a nonnegative integer. + + Args: + version1 (str): The first version number. + version2 (str): The second version number. + + Returns: + bool: True if version1 is equal to or more recent than version2. + """ + + version1_numbers = version1.replace("v", "").split(".") + version2_numbers = version2.replace("v", "").split(".") + + for i in range(3): + if version1_numbers[i] > version2_numbers[i]: + return True + if version1_numbers[i] < version2_numbers[i]: + return False + + return True + + +def unset_hotkey_bindings() -> None: + """Unset all hotkey bindings.""" + # Text values + set_value("SPLIT_HOTKEY_NAME", "", settings) + set_value("RESET_HOTKEY_NAME", "", settings) + set_value("PAUSE_HOTKEY_NAME", "", settings) + set_value("UNDO_HOTKEY_NAME", "", settings) + set_value("SKIP_HOTKEY_NAME", "", settings) + set_value("PREV_HOTKEY_NAME", "", settings) + set_value("NEXT_HOTKEY_NAME", "", settings) + set_value("SCREENSHOT_HOTKEY_NAME", "", settings) + set_value("TOGGLE_HOTKEYS_HOTKEY_NAME", "", settings) + + # Key IDs + set_value("SPLIT_HOTKEY_CODE", "", settings) + set_value("RESET_HOTKEY_CODE", "", settings) + set_value("PAUSE_HOTKEY_CODE", "", settings) + set_value("UNDO_HOTKEY_CODE", "", settings) + set_value("SKIP_HOTKEY_CODE", "", settings) + set_value("PREV_HOTKEY_CODE", "", settings) + set_value("NEXT_HOTKEY_CODE", "", settings) + set_value("SCREENSHOT_HOTKEY_CODE", "", settings) + set_value("TOGGLE_HOTKEYS_HOTKEY_CODE", "", settings) def get_latest_version() -> str: diff --git a/src/splitter/splitter.py b/src/splitter/splitter.py index 2f1c0f6..798a5e9 100644 --- a/src/splitter/splitter.py +++ b/src/splitter/splitter.py @@ -212,7 +212,7 @@ def toggle_suspended(self) -> None: current_match_percent = self.match_percent self.safe_exit_compare_thread() if current_match_percent is None and len(self.splits.list) > 0: - self.start_compare_thread() + self.start_compare_thread() ################################### # # diff --git a/src/ui/ui_main_window.py b/src/ui/ui_main_window.py index 30bba4b..e1a8dd1 100644 --- a/src/ui/ui_main_window.py +++ b/src/ui/ui_main_window.py @@ -550,7 +550,7 @@ def __init__(self) -> None: if settings.get_bool("CHECK_FOR_UPDATES"): latest_version = settings.get_latest_version() - if latest_version != settings.VERSION_NUMBER: + if not settings.version_ge(settings.VERSION_NUMBER, latest_version): # Yes, I call both show and open. If you just call show, the # box doesn't always appear centered over the window (it's way # off to the side). If you just call show, then bafflingly, the diff --git a/tests/test_settings.py b/tests/test_settings.py index 94d38ec..44afe52 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -163,61 +163,61 @@ def test_get_float_from_float(self): self.dummy_settings.setValue("test", 123.123123) assert settings.get_float("test", settings=self.dummy_settings) == 123.123123 - # Testing set_defaults - def test_set_defaults_runs_on_blank_settings(self): - settings.set_defaults(settings=self.dummy_settings) + # Testing set_program_vals + def test_set_program_vals_runs_on_blank_settings(self): + settings.set_program_vals(settings=self.dummy_settings) assert settings.get_bool("SETTINGS_SET", settings=self.dummy_settings) - def test_set_defaults_turns_off_show_min_view(self): - settings.set_defaults(settings=self.dummy_settings) + def test_set_program_vals_turns_off_show_min_view(self): + settings.set_program_vals(settings=self.dummy_settings) orig_start_with_video = settings.get_bool( "START_WITH_VIDEO", settings=self.dummy_settings ) settings.set_value("SHOW_MIN_VIEW", True, settings=self.dummy_settings) - settings.set_defaults(settings=self.dummy_settings) + settings.set_program_vals(settings=self.dummy_settings) assert not orig_start_with_video and not settings.get_bool( "SHOW_MIN_VIEW", settings=self.dummy_settings ) - def test_set_defaults_sets_correct_aspect_ratio_1(self): - settings.set_defaults(settings=self.dummy_settings) + def test_set_program_vals_sets_correct_aspect_ratio_1(self): + settings.set_program_vals(settings=self.dummy_settings) settings.set_value( "ASPECT_RATIO", "4:3 (480x360)", settings=self.dummy_settings ) - settings.set_defaults(settings=self.dummy_settings) + settings.set_program_vals(settings=self.dummy_settings) assert ( settings.get_int("FRAME_WIDTH", settings=self.dummy_settings) == 480 and settings.get_int("FRAME_HEIGHT", settings=self.dummy_settings) == 360 ) - def test_set_defaults_sets_correct_aspect_ratio_2(self): - settings.set_defaults(settings=self.dummy_settings) + def test_set_program_vals_sets_correct_aspect_ratio_2(self): + settings.set_program_vals(settings=self.dummy_settings) settings.set_value( "ASPECT_RATIO", "4:3 (320x240)", settings=self.dummy_settings ) - settings.set_defaults(settings=self.dummy_settings) + settings.set_program_vals(settings=self.dummy_settings) assert ( settings.get_int("FRAME_WIDTH", settings=self.dummy_settings) == 320 and settings.get_int("FRAME_HEIGHT", settings=self.dummy_settings) == 240 ) - def test_set_defaults_sets_correct_aspect_ratio_3(self): - settings.set_defaults(settings=self.dummy_settings) + def test_set_program_vals_sets_correct_aspect_ratio_3(self): + settings.set_program_vals(settings=self.dummy_settings) settings.set_value( "ASPECT_RATIO", "16:9 (512x288)", settings=self.dummy_settings ) - settings.set_defaults(settings=self.dummy_settings) + settings.set_program_vals(settings=self.dummy_settings) assert ( settings.get_int("FRAME_WIDTH", settings=self.dummy_settings) == 512 and settings.get_int("FRAME_HEIGHT", settings=self.dummy_settings) == 288 ) - def test_set_defaults_sets_correct_aspect_ratio_4(self): - settings.set_defaults(settings=self.dummy_settings) + def test_set_program_vals_sets_correct_aspect_ratio_4(self): + settings.set_program_vals(settings=self.dummy_settings) settings.set_value( "ASPECT_RATIO", "16:9 (432x243)", settings=self.dummy_settings ) - settings.set_defaults(settings=self.dummy_settings) + settings.set_program_vals(settings=self.dummy_settings) assert ( settings.get_int("FRAME_WIDTH", settings=self.dummy_settings) == 432 and settings.get_int("FRAME_HEIGHT", settings=self.dummy_settings) == 243 @@ -237,3 +237,36 @@ def test_get_latest_version(): def test_get_home_dir_returns_real_path(): assert Path(settings.get_home_dir()).is_dir() + + +# Testing version_ge +def test_version_ge_1(): + assert settings.version_ge("1.0.1", "1.0.0") + + +def test_version_ge_2(): + assert settings.version_ge("1.0.1", "1.0.1") + + +def test_version_ge_3(): + assert settings.version_ge("1.1.0", "1.0.1") + + +def test_version_ge_4(): + assert settings.version_ge("2.1.0", "1.0.1") + + +def test_version_ge_5(): + assert settings.version_ge("1.1.0", "1.0.0") + + +def test_version_ge_6(): + assert not settings.version_ge("1.1.0", "1.1.1") + + +def test_version_ge_6(): + assert settings.version_ge("1.2.0", "1.1.10") + + +def test_version_ge_7(): + assert settings.version_ge("1.0.0", "0.20000.999")