Add synchronized rainbow effects for title bar and border #65
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Introduce a new 'synchronized_rainbow' class to manage a single, module-level color cycle thread. This allows the 'rainbow_title_bar' and 'rainbow_border' effects to run in a synchronized mode (using `start_sync()`), ensuring all rainbow elements change color at the exact same time. The original single-element rainbow effects remain unchanged for backward compatibility.
Contributor
Reviewer's GuideThis PR introduces a module-level synchronized_rainbow manager to coordinate synchronized rainbow effects for both title bar and border, while preserving the original single-element modes for backward compatibility. Sequence diagram for starting synchronized rainbow effect on title barsequenceDiagram
participant User
participant "rainbow_title_bar"
participant "synchronized_rainbow"
participant "title_bar_color"
User->>rainbow_title_bar: start_sync(window, interval, color_stops)
rainbow_title_bar->>synchronized_rainbow: start(interval, color_stops)
loop Periodically
rainbow_title_bar->>synchronized_rainbow: get_current_color()
rainbow_title_bar->>title_bar_color: set(hwnd, rgb)
end
Sequence diagram for stopping synchronized rainbow effect on bordersequenceDiagram
participant User
participant "rainbow_border"
participant "border_color"
participant "synchronized_rainbow"
User->>rainbow_border: stop_sync(window)
rainbow_border->>border_color: reset(window)
rainbow_border->>synchronized_rainbow: stop() (if last sync window)
Class diagram for synchronized rainbow effectsclassDiagram
class synchronized_rainbow {
+_color_changer_task(interval: int, color_stops: int)
+get_current_color() Tuple[int, int, int]
+start(interval: int, color_stops: int)
+stop()
}
class rainbow_title_bar {
+current_color
+_sync_timer_threads: Dict[int, threading.Thread]
+start(window, interval=5, color_stops=5)
+stop(window)
+start_sync(window, interval=5, color_stops=5)
+stop_sync(window)
}
class rainbow_border {
+current_color
+_sync_timer_threads: Dict[int, threading.Thread]
+start(window, interval=5, color_stops=5)
+stop(window)
+start_sync(window, interval=5, color_stops=5)
+stop_sync(window)
}
synchronized_rainbow <.. rainbow_title_bar : uses (sync)
synchronized_rainbow <.. rainbow_border : uses (sync)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- There’s a lot of duplicated logic in start_sync/stop_sync between rainbow_title_bar and rainbow_border—consider extracting the common timer thread setup/teardown into a shared helper.
- Using a global boolean for sync_is_running can introduce race conditions—consider switching to a threading.Event or similar primitive for more robust thread signaling.
- The int-to-RGB conversion appears inconsistent (storing with r<<16|g<<8|b but decoding as BGR)—double-check your bit shifts to avoid swapped color channels.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s a lot of duplicated logic in start_sync/stop_sync between rainbow_title_bar and rainbow_border—consider extracting the common timer thread setup/teardown into a shared helper.
- Using a global boolean for sync_is_running can introduce race conditions—consider switching to a threading.Event or similar primitive for more robust thread signaling.
- The int-to-RGB conversion appears inconsistent (storing with r<<16|g<<8|b but decoding as BGR)—double-check your bit shifts to avoid swapped color channels.
## Individual Comments
### Comment 1
<location> `hPyT/hPyT.py:191-196` </location>
<code_context>
+ if sync_is_running:
+ sync_is_running = False
+ # Wait for the thread to finish cleanly
+ if threading.current_thread() is not sync_color_thread:
+ sync_color_thread.join(timeout=0.1)
+ sync_color_thread = None
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Joining the thread with a short timeout may not guarantee clean shutdown.
A short timeout may not be sufficient if the thread is sleeping. Consider increasing the timeout or adding logic to handle cases where the thread does not terminate, to prevent resource leaks.
```suggestion
if sync_is_running:
sync_is_running = False
# Wait for the thread to finish cleanly
if threading.current_thread() is not sync_color_thread:
sync_color_thread.join(timeout=2.0)
if sync_color_thread.is_alive():
import logging
logging.warning("sync_color_thread did not terminate after timeout. Possible resource leak.")
sync_color_thread = None
```
</issue_to_address>
### Comment 2
<location> `hPyT/hPyT.py:1001-1006` </location>
<code_context>
+ synchronized_rainbow.start(interval, color_stops)
+
+ # Start a timer thread for this window to periodically apply the master color
+ def sync_task(h: int, i: int):
+ while h in cls._sync_timer_threads:
+ rgb = synchronized_rainbow.get_current_color()
+ # Apply color using standard setter
+ title_bar_color.set(h, rgb)
+ ctypes.windll.kernel32.Sleep(i)
+
+ cls._sync_timer_threads[hwnd] = threading.Thread(target=sync_task, args=(hwnd, interval), daemon=True)
</code_context>
<issue_to_address>
**suggestion:** Using kernel32.Sleep in Python threads may block signals and affect responsiveness.
Using ctypes.windll.kernel32.Sleep can interfere with Python's threading and signal handling. Prefer time.sleep(i / 1000) for compatibility and responsiveness.
Suggested implementation:
```python
time.sleep(i / 1000)
```
```python
import time
hwnd: int = module_find(window)
```
</issue_to_address>
### Comment 3
<location> `hPyT/hPyT.py:882` </location>
<code_context>
"""Add a rainbow effect to the title bar of a window."""
current_color = None
+ _sync_timer_threads: Dict[int, threading.Thread] = {} # Tracks sync mode threads
@classmethod
</code_context>
<issue_to_address>
**issue (bug_risk):** Dictionary of threads is not thread-safe and may cause race conditions.
Consider protecting access to _sync_timer_threads with a threading.Lock or similar synchronization to prevent race conditions during concurrent modifications.
</issue_to_address>
### Comment 4
<location> `hPyT/hPyT.py:117` </location>
<code_context>
+ if not is_title_bar_sync_running and not is_border_sync_running:
+ synchronized_rainbow.stop()
+
+# New Synchronization Class
+class synchronized_rainbow:
+ """Manages the single, module-level thread for synchronized color cycling."""
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring synchronization to use a single master thread with subscriber callbacks instead of per-window threads and global color bit-packing.
```suggestion
Rather than spawning a thread+Sleep loop for each window and bit-packing colors into an int, you can:
1. Store `sync_current_color` as an `(r,g,b)` tuple.
2. Keep a single master thread in `synchronized_rainbow` that
– updates the color,
– iterates over a `{hwnd: setter}` subscriber dict to push the new color,
– sleeps once per interval.
3. Expose `subscribe(hwnd, setter)`/`unsubscribe(hwnd)` so `title_bar` and `border` just register/reset callbacks.
Example refactoring of `synchronized_rainbow`:
```python
class synchronized_rainbow:
_thread: Optional[threading.Thread] = None
_running = False
_lock = threading.Lock()
_current_color: Tuple[int,int,int] = (0,0,0)
_subscribers: Dict[int, Callable[[int, Tuple[int,int,int]], None]] = {}
@classmethod
def _task(cls, interval: int, color_stops: int):
r, g, b = 200, 0, 0
while cls._running:
# ... same RGB‐cycle logic, but store tuple
with cls._lock:
cls._current_color = (r, g, b)
for hwnd, setter in cls._subscribers.items():
setter(hwnd, cls._current_color)
time.sleep(interval / 1000.0)
with cls._lock:
cls._current_color = (0,0,0)
@classmethod
def start(cls, interval: int, color_stops: int):
if not cls._running:
cls._running = True
cls._thread = threading.Thread(
target=cls._task, args=(interval, color_stops), daemon=True
)
cls._thread.start()
@classmethod
def stop(cls):
cls._running = False
if cls._thread:
cls._thread.join(0.1)
cls._thread = None
@classmethod
def subscribe(cls, hwnd: int, setter: Callable[[int, Tuple[int,int,int]], None]):
cls._subscribers[hwnd] = setter
@classmethod
def unsubscribe(cls, hwnd: int):
cls._subscribers.pop(hwnd, None)
if not cls._subscribers:
cls.stop()
```
Then in `rainbow_title_bar` and `rainbow_border`:
```python
@classmethod
def start_sync(cls, window, interval=5, color_stops=5):
hwnd = module_find(window)
synchronized_rainbow.start(interval, color_stops)
synchronized_rainbow.subscribe(hwnd, title_bar_color.set)
@classmethod
def stop_sync(cls, window):
hwnd = module_find(window)
synchronized_rainbow.unsubscribe(hwnd)
title_bar_color.reset(window)
```
This removes the per-window thread loops, global bit‐packing, and duplicated sync logic.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
191
to
196
| if sync_is_running: | ||
| sync_is_running = False | ||
| # Wait for the thread to finish cleanly | ||
| if threading.current_thread() is not sync_color_thread: | ||
| sync_color_thread.join(timeout=0.1) | ||
| sync_color_thread = None |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Joining the thread with a short timeout may not guarantee clean shutdown.
A short timeout may not be sufficient if the thread is sleeping. Consider increasing the timeout or adding logic to handle cases where the thread does not terminate, to prevent resource leaks.
Suggested change
| if sync_is_running: | |
| sync_is_running = False | |
| # Wait for the thread to finish cleanly | |
| if threading.current_thread() is not sync_color_thread: | |
| sync_color_thread.join(timeout=0.1) | |
| sync_color_thread = None | |
| if sync_is_running: | |
| sync_is_running = False | |
| # Wait for the thread to finish cleanly | |
| if threading.current_thread() is not sync_color_thread: | |
| sync_color_thread.join(timeout=2.0) | |
| if sync_color_thread.is_alive(): | |
| import logging | |
| logging.warning("sync_color_thread did not terminate after timeout. Possible resource leak.") | |
| sync_color_thread = None |
Comment on lines
+1001
to
+1006
| def sync_task(h: int, i: int): | ||
| while h in cls._sync_timer_threads: | ||
| rgb = synchronized_rainbow.get_current_color() | ||
| # Apply color using standard setter | ||
| title_bar_color.set(h, rgb) | ||
| ctypes.windll.kernel32.Sleep(i) |
Contributor
There was a problem hiding this comment.
suggestion: Using kernel32.Sleep in Python threads may block signals and affect responsiveness.
Using ctypes.windll.kernel32.Sleep can interfere with Python's threading and signal handling. Prefer time.sleep(i / 1000) for compatibility and responsiveness.
Suggested implementation:
time.sleep(i / 1000)import time
hwnd: int = module_find(window)
hPyT/hPyT.py
Outdated
| """Add a rainbow effect to the title bar of a window.""" | ||
|
|
||
| current_color = None | ||
| _sync_timer_threads: Dict[int, threading.Thread] = {} # Tracks sync mode threads |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Dictionary of threads is not thread-safe and may cause race conditions.
Consider protecting access to _sync_timer_threads with a threading.Lock or similar synchronization to prevent race conditions during concurrent modifications.
| if not is_title_bar_sync_running and not is_border_sync_running: | ||
| synchronized_rainbow.stop() | ||
|
|
||
| # New Synchronization Class |
Contributor
There was a problem hiding this comment.
issue (complexity): Consider refactoring synchronization to use a single master thread with subscriber callbacks instead of per-window threads and global color bit-packing.
Suggested change
| # New Synchronization Class | |
| Rather than spawning a thread+Sleep loop for each window and bit-packing colors into an int, you can: | |
| 1. Store `sync_current_color` as an `(r,g,b)` tuple. | |
| 2. Keep a single master thread in `synchronized_rainbow` that | |
| – updates the color, | |
| – iterates over a `{hwnd: setter}` subscriber dict to push the new color, | |
| – sleeps once per interval. | |
| 3. Expose `subscribe(hwnd, setter)`/`unsubscribe(hwnd)` so `title_bar` and `border` just register/reset callbacks. | |
| Example refactoring of `synchronized_rainbow`: | |
| ```python | |
| class synchronized_rainbow: | |
| _thread: Optional[threading.Thread] = None | |
| _running = False | |
| _lock = threading.Lock() | |
| _current_color: Tuple[int,int,int] = (0,0,0) | |
| _subscribers: Dict[int, Callable[[int, Tuple[int,int,int]], None]] = {} | |
| @classmethod | |
| def _task(cls, interval: int, color_stops: int): | |
| r, g, b = 200, 0, 0 | |
| while cls._running: | |
| # ... same RGB‐cycle logic, but store tuple | |
| with cls._lock: | |
| cls._current_color = (r, g, b) | |
| for hwnd, setter in cls._subscribers.items(): | |
| setter(hwnd, cls._current_color) | |
| time.sleep(interval / 1000.0) | |
| with cls._lock: | |
| cls._current_color = (0,0,0) | |
| @classmethod | |
| def start(cls, interval: int, color_stops: int): | |
| if not cls._running: | |
| cls._running = True | |
| cls._thread = threading.Thread( | |
| target=cls._task, args=(interval, color_stops), daemon=True | |
| ) | |
| cls._thread.start() | |
| @classmethod | |
| def stop(cls): | |
| cls._running = False | |
| if cls._thread: | |
| cls._thread.join(0.1) | |
| cls._thread = None | |
| @classmethod | |
| def subscribe(cls, hwnd: int, setter: Callable[[int, Tuple[int,int,int]], None]): | |
| cls._subscribers[hwnd] = setter | |
| @classmethod | |
| def unsubscribe(cls, hwnd: int): | |
| cls._subscribers.pop(hwnd, None) | |
| if not cls._subscribers: | |
| cls.stop() |
Then in rainbow_title_bar and rainbow_border:
@classmethod
def start_sync(cls, window, interval=5, color_stops=5):
hwnd = module_find(window)
synchronized_rainbow.start(interval, color_stops)
synchronized_rainbow.subscribe(hwnd, title_bar_color.set)
@classmethod
def stop_sync(cls, window):
hwnd = module_find(window)
synchronized_rainbow.unsubscribe(hwnd)
title_bar_color.reset(window)This removes the per-window thread loops, global bit‐packing, and duplicated sync logic.
Owner
|
@Sae3sy amazing PR! Please allow me some time to review and test it! |
Owner
|
Also could you please run |
The sync_color_thread is an Optional[Thread]. This change adds an explicit None check to satisfy mypy's union-attr validation before calling .join().
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduce a new 'synchronized_rainbow' class to manage a single, module-level color cycle thread.
This allows the 'rainbow_title_bar' and 'rainbow_border' effects to run in a synchronized mode (using
start_sync()), ensuring all rainbow elements change color at the exact same time.The original single-element rainbow effects remain unchanged for backward compatibility.
2025-10-18.01-02-09.mp4
Summary by Sourcery
Add a module-level synchronization mechanism that drives a single rainbow color cycle thread and extend the title bar and border effects to run in lockstep using this shared manager.
New Features:
Enhancements: