feat: Implement punch combination detection #5
Conversation
This feature introduces the ability to detect predefined punch combinations
(e.g., Jab-Cross, Jab-Cross-Hook) based on the sequence and timing of
punches.
Key changes include:
- **`utils/combo_detector.py` (New)**:
- Defines `ComboDetector` class to manage and detect combos.
- Supports predefined combos with specific punch sequences and
inter-punch timing thresholds.
- Sorts combos by length to prioritize longer, more specific ones.
- Includes epsilon for robust floating-point timing comparisons.
- **`utils/punch_counter.py`**:
- Added `punch_event_history` (a deque) to store recent
`(punch_type, timestamp)` tuples for combo analysis.
- **`main.py`**:
- Integrates `ComboDetector` into the main processing loop.
- Manages combo statistics (successes, specific combo counts).
- Clears punch history after a combo is detected to prevent
overlapping detections.
- Passes combo information to UI and data management.
- **`utils/ui_manager.py`**:
- Displays the name of a detected combo prominently on screen for a
short duration.
- Adds combo success counts to the on-screen statistics panel.
- Increased stats panel height to accommodate new info.
- **`utils/data_manager.py`**:
- Updated database schema (`sessions` table) to include
`combo_successes`, `combo_attempts`, and `combo_details` (JSON
string for counts of each detected combo type).
- Modified save, load, and summary methods to handle these new
combo statistics.
- **`tests/test_combo_detector.py` (New)**:
- Added a suite of 14 unit tests for `ComboDetector`, covering
various scenarios including timing, sequence, boundary conditions,
and prioritization. These tests helped identify and fix a float
precision issue in timing.
This feature enhances user engagement by adding a gamified element and
allowing you to get feedback on specific boxing combinations.
Reviewer's GuideThis PR implements punch combination detection by introducing a dedicated ComboDetector, recording punch events for combo analysis, integrating detection into the main loop with UI feedback and persistence, and verifying behavior via new unit tests. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughCombo detection has been integrated into the PunchTracker application. This includes a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PunchTracker
participant PunchCounter
participant ComboDetector
participant UIManager
participant DataManager
User->>PunchTracker: Start session
PunchTracker->>PunchCounter: Reset counters
PunchTracker->>ComboDetector: Reset combo state
loop For each frame
PunchTracker->>PunchCounter: detect_punches(frame)
PunchCounter->>PunchTracker: punch_counts, punch_event_history
PunchTracker->>ComboDetector: detect_combo(punch_event_history)
ComboDetector-->>PunchTracker: Detected combo (if any)
PunchTracker->>UIManager: update_display(..., current_detected_combo, combo_stats)
end
User->>PunchTracker: End session
PunchTracker->>DataManager: save_session_data(..., combo_stats)
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements punch combination detection to enrich the user experience by recognizing specific boxing combinations based on punch sequence and timing. Key changes include:
- Introduction of the ComboDetector class in a new file (utils/combo_detector.py) for detecting predefined combos.
- Updates to UI, punch counting, data management, and main processing logic to integrate combo detection and statistics.
- A new suite of tests in tests/test_combo_detector.py ensuring various timing and sequence scenarios are correctly handled.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/ui_manager.py | Adjusted panel dimensions and display logic to incorporate combo stats. |
| utils/punch_counter.py | Added event history tracking for combo detection. |
| utils/data_manager.py | Updated the database schema and saving process to include combo stats. |
| utils/combo_detector.py | New module for detecting combos based on punch sequences and timing. |
| tests/test_combo_detector.py | Added tests for validating combo detection logic. |
| main.py | Integrated combo detection into the main processing loop and session management. |
| # Instruction height: (len(instructions) * 25 + 10) -> approx 8*25+10 = 210 | ||
| # If panel is at top (h=230), text_y can be panel_height + 50 | ||
| text_y = self.panel_height + 50 # Below a top panel | ||
| if text_y > h - 220 : # Avoid overlapping with bottom instructions |
There was a problem hiding this comment.
Consider replacing the hardcoded offset value (220) with a named constant to improve maintainability and clarity.
| if text_y > h - 220 : # Avoid overlapping with bottom instructions | |
| if text_y > h - self.BOTTOM_INSTRUCTIONS_OFFSET : # Avoid overlapping with bottom instructions |
| # Jab-Cross default timing is 0.7s | ||
| # Construct timestamps to ensure exact difference | ||
| punch1_ts = time.time() | ||
| punch2_ts = punch1_ts + 0.7 # Exact difference of 0.7 |
There was a problem hiding this comment.
[nitpick] Consider using fixed or mocked timestamps in tests to avoid potential flakiness due to reliance on system time.
| punches_detected = self.punch_counter.detect_punches(poses) | ||
|
|
||
| # Combo Detection Logic | ||
| if len(self.punch_counter.punch_event_history) > 0: |
There was a problem hiding this comment.
[nitpick] Consider using a direct truthiness check (e.g., 'if self.punch_counter.punch_event_history:') for improved readability.
| if len(self.punch_counter.punch_event_history) > 0: | |
| if self.punch_counter.punch_event_history: |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @tblakex01 - I've reviewed your changes - here's some feedback:
Blocking issues:
returnshould never appear inside a class init function. This will cause a runtime error. (link)returnshould never appear inside a class init function. This will cause a runtime error. (link)
General comments:
- combo_stats['attempts'] is never incremented in process_frame, so combo_attempts remains zero—consider updating attempts when detect_combo is invoked or when a punch sequence starts.
- Clearing the entire punch_event_history after a combo is detected prevents overlapping combos; consider only removing the consumed events or sliding the window to allow back-to-back detections.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🔴 Security: 2 blocking issues
- 🟡 Testing: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # Instruction height: (len(instructions) * 25 + 10) -> approx 8*25+10 = 210 | ||
| # If panel is at top (h=230), text_y can be panel_height + 50 | ||
| text_y = self.panel_height + 50 # Below a top panel | ||
| if text_y > h - 220 : # Avoid overlapping with bottom instructions | ||
| text_y = (h // 2) + 50 # Fallback to just below center |
There was a problem hiding this comment.
suggestion: Avoid hard-coded pixel offsets for combo text placement
Use dynamic calculations for text placement based on actual instruction and panel sizes to prevent overlap as UI elements change.
| # Instruction height: (len(instructions) * 25 + 10) -> approx 8*25+10 = 210 | |
| # If panel is at top (h=230), text_y can be panel_height + 50 | |
| text_y = self.panel_height + 50 # Below a top panel | |
| if text_y > h - 220 : # Avoid overlapping with bottom instructions | |
| text_y = (h // 2) + 50 # Fallback to just below center | |
| # Dynamically calculate instruction block height | |
| instruction_line_height = text_h + 8 # 8px padding between lines | |
| instruction_block_height = len(instructions) * instruction_line_height + 10 # 10px extra padding | |
| # Place combo text below the panel, but above the instructions | |
| text_y = self.panel_height + self.panel_padding + 10 # 10px gap below panel | |
| # If this would overlap with the instructions at the bottom, move it up | |
| instructions_top_y = h - instruction_block_height - 20 # 20px margin from bottom | |
| if text_y + text_h > instructions_top_y: | |
| # Place combo text just above the instructions | |
| text_y = instructions_top_y - text_h - 10 # 10px gap above instructions | |
| # Ensure combo text is not off the screen | |
| if text_y < self.panel_height + self.panel_padding: | |
| text_y = self.panel_height + self.panel_padding |
| self.current_detected_combo = detected_combo_name | ||
| self.last_combo_detected_time = time.time() | ||
|
|
||
| self.combo_stats['successes'] += 1 |
There was a problem hiding this comment.
issue (bug_risk): combo_stats['attempts'] never gets incremented
Increment 'attempts' when detect_combo is called or when a combo window starts to accurately track attempts.
|
|
||
| # Record punch event for combo detection | ||
| # Use the timestamp associated with the punch from timestamp_history | ||
| self.punch_event_history.append((punch_type, self.timestamp_history[wrist_key][-1])) |
There was a problem hiding this comment.
issue (bug_risk): Guard against empty timestamp_history to avoid IndexError
Accessing the last element of an empty list will raise an IndexError. Consider defaulting to time.time() or skipping the record if no timestamp is available.
|
|
||
| return None | ||
|
|
||
| if __name__ == '__main__': |
There was a problem hiding this comment.
suggestion (testing): Demo test code in module; consider moving to separate tests
Consider relocating the __main__ test block to a separate test file to keep production code clean and maintainable.
|
|
||
| def __init__(self, predefined_combos=None): | ||
| if predefined_combos is None: | ||
| self.predefined_combos = sorted(self.DEFAULT_COMBOS, key=lambda c: len(c['sequence']), reverse=True) |
There was a problem hiding this comment.
security (return-in-init): return should never appear inside a class init function. This will cause a runtime error.
Source: opengrep
| if predefined_combos is None: | ||
| self.predefined_combos = sorted(self.DEFAULT_COMBOS, key=lambda c: len(c['sequence']), reverse=True) | ||
| else: | ||
| self.predefined_combos = sorted(predefined_combos, key=lambda c: len(c['sequence']), reverse=True) |
There was a problem hiding this comment.
security (return-in-init): return should never appear inside a class init function. This will cause a runtime error.
Source: opengrep
| if detected_combo_name: | ||
| if self.current_detected_combo != detected_combo_name or \ | ||
| (time.time() - self.last_combo_detected_time > self.combo_display_duration): | ||
|
|
||
| print(f"COMBO DETECTED: {detected_combo_name}") | ||
| self.current_detected_combo = detected_combo_name | ||
| self.last_combo_detected_time = time.time() | ||
|
|
||
| self.combo_stats['successes'] += 1 | ||
| self.combo_stats['detected_combos'][detected_combo_name] = \ | ||
| self.combo_stats['detected_combos'].get(detected_combo_name, 0) + 1 | ||
|
|
||
| self.punch_counter.punch_event_history.clear() |
There was a problem hiding this comment.
suggestion (code-quality): Merge nested if conditions (merge-nested-ifs)
| if detected_combo_name: | |
| if self.current_detected_combo != detected_combo_name or \ | |
| (time.time() - self.last_combo_detected_time > self.combo_display_duration): | |
| print(f"COMBO DETECTED: {detected_combo_name}") | |
| self.current_detected_combo = detected_combo_name | |
| self.last_combo_detected_time = time.time() | |
| self.combo_stats['successes'] += 1 | |
| self.combo_stats['detected_combos'][detected_combo_name] = \ | |
| self.combo_stats['detected_combos'].get(detected_combo_name, 0) + 1 | |
| self.punch_counter.punch_event_history.clear() | |
| if detected_combo_name and (self.current_detected_combo != detected_combo_name or \ | |
| (time.time() - self.last_combo_detected_time > self.combo_display_duration)): | |
| print(f"COMBO DETECTED: {detected_combo_name}") | |
| self.current_detected_combo = detected_combo_name | |
| self.last_combo_detected_time = time.time() | |
| self.combo_stats['successes'] += 1 | |
| self.combo_stats['detected_combos'][detected_combo_name] = \ | |
| self.combo_stats['detected_combos'].get(detected_combo_name, 0) + 1 | |
| self.punch_counter.punch_event_history.clear() | |
Explanation
Too much nesting can make code difficult to understand, and this is especiallytrue in Python, where there are no brackets to help out with the delineation of
different nesting levels.
Reading deeply nested code is confusing, since you have to keep track of which
conditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two if conditions can be combined using
and is an easy win.
| def test_sub_combo_detection_priority(self): | ||
| # DEFAULT_COMBOS are sorted by length descending in __init__ | ||
| # Jab-Jab-Cross vs Jab-Cross | ||
| history = deque(maxlen=7) |
There was a problem hiding this comment.
issue (code-quality): Extract duplicate code into method (extract-duplicate-method)
| # Check punch sequence | ||
| match = True | ||
| for i, expected_punch in enumerate(combo_sequence): | ||
| if relevant_history[i][0] != expected_punch: | ||
| match = False | ||
| break | ||
|
|
There was a problem hiding this comment.
suggestion (code-quality): We've found these issues:
- Use any() instead of for loop (
use-any) - Invert any/all to simplify comparisons (
invert-any-all)
| # Check punch sequence | |
| match = True | |
| for i, expected_punch in enumerate(combo_sequence): | |
| if relevant_history[i][0] != expected_punch: | |
| match = False | |
| break | |
| match = all( | |
| relevant_history[i][0] == expected_punch | |
| for i, expected_punch in enumerate(combo_sequence) | |
| ) |
|
|
||
| # Display current combo | ||
| if current_detected_combo and (time.time() - last_combo_detected_time < combo_display_duration): | ||
| h, w = display_frame.shape[:2] |
There was a problem hiding this comment.
issue (code-quality): Extract code out into method (extract-method)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
utils/ui_manager.py (1)
33-48: 🛠️ Refactor suggestionAddress the high parameter count in update_display method.
The method signature has grown to 11 parameters, exceeding typical best practices. Consider using a data class or dictionary to group related parameters.
from dataclasses import dataclass from typing import Optional, Dict @dataclass class ComboDisplayData: current_detected_combo: Optional[str] = None last_combo_detected_time: float = 0 combo_display_duration: float = 3 combo_stats: Optional[Dict] = None def update_display(self, frame, total_count, punch_counts, session_start_time, sensitivity, paused=False, combo_data: Optional[ComboDisplayData] = None):This would improve maintainability and make the method signature more manageable.
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 33-33: Too many arguments (11/5)
(R0913)
[refactor] 33-33: Too many positional arguments (11/5)
(R0917)
[refactor] 33-33: Too many local variables (20/15)
(R0914)
🧹 Nitpick comments (2)
main.py (1)
28-37: Consider initializing combo stats in a separate method.The combo-related attributes are well-defined, but the initialization block is getting quite large. Consider extracting combo initialization to a separate method for better maintainability.
- # Combo Detection related attributes - self.combo_detector = ComboDetector() # Uses default combos - self.current_detected_combo = None - self.last_combo_detected_time = 0 - self.combo_display_duration = 3.0 # seconds - self.combo_stats = { - 'attempts': 0, - 'successes': 0, - 'detected_combos': {} - } + # Combo Detection related attributes + self._initialize_combo_detection()And add a new method:
def _initialize_combo_detection(self): """Initialize combo detection components and state""" self.combo_detector = ComboDetector() # Uses default combos self.current_detected_combo = None self.last_combo_detected_time = 0 self.combo_display_duration = 3.0 # seconds self.combo_stats = { 'attempts': 0, 'successes': 0, 'detected_combos': {} }utils/ui_manager.py (1)
58-78: Improve combo text positioning and consider edge cases.The combo display logic is functional but has room for improvement in positioning calculations and edge case handling.
- # Display current combo - if current_detected_combo and (time.time() - last_combo_detected_time < combo_display_duration): - h, w = display_frame.shape[:2] - combo_font_scale = 1.2 - combo_thickness = 2 - (text_w, text_h), _ = cv2.getTextSize(current_detected_combo, self.font, combo_font_scale, combo_thickness) - - text_x = (w - text_w) // 2 - # Position above the stats panel (assuming panel is at top right) - # or in a noticeable central area if panel is not at top. - # Let's try to place it below the center, ensuring it's above instructions. - # Instruction height: (len(instructions) * 25 + 10) -> approx 8*25+10 = 210 - # If panel is at top (h=230), text_y can be panel_height + 50 - text_y = self.panel_height + 50 # Below a top panel - if text_y > h - 220 : # Avoid overlapping with bottom instructions - text_y = (h // 2) + 50 # Fallback to just below center - - cv2.putText(display_frame, current_detected_combo, (text_x, text_y), - self.font, combo_font_scale, (0,0,0), combo_thickness + 3, cv2.LINE_AA) # Black outline - cv2.putText(display_frame, current_detected_combo, (text_x, text_y), - self.font, combo_font_scale, (50, 255, 50), combo_thickness, cv2.LINE_AA) # Bright green color + # Display current combo + if current_detected_combo and (time.time() - last_combo_detected_time < combo_display_duration): + self._draw_combo_text(display_frame, current_detected_combo)Add a new method:
def _draw_combo_text(self, frame, combo_text): """Draw combo text with proper positioning and styling""" h, w = frame.shape[:2] combo_font_scale = 1.2 combo_thickness = 2 (text_w, text_h), _ = cv2.getTextSize(combo_text, self.font, combo_font_scale, combo_thickness) text_x = max(0, (w - text_w) // 2) # Ensure text doesn't go off-screen # Calculate optimal Y position instructions_height = 8 * 25 + 10 # Approximate instruction area height min_y = self.panel_height + 50 max_y = h - instructions_height - text_h - 10 text_y = min_y if min_y < max_y else h // 2 # Draw text with outline for better visibility cv2.putText(frame, combo_text, (text_x, text_y), self.font, combo_font_scale, (0, 0, 0), combo_thickness + 3, cv2.LINE_AA) cv2.putText(frame, combo_text, (text_x, text_y), self.font, combo_font_scale, (50, 255, 50), combo_thickness, cv2.LINE_AA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
main.py(7 hunks)tests/test_combo_detector.py(1 hunks)utils/combo_detector.py(1 hunks)utils/data_manager.py(8 hunks)utils/punch_counter.py(2 hunks)utils/ui_manager.py(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/test_combo_detector.py (1)
utils/combo_detector.py (2)
ComboDetector(4-90)detect_combo(39-90)
main.py (1)
utils/combo_detector.py (2)
ComboDetector(4-90)detect_combo(39-90)
🪛 Pylint (3.3.7)
utils/combo_detector.py
[refactor] 4-4: Too few public methods (1/2)
(R0903)
utils/ui_manager.py
[refactor] 33-33: Too many arguments (11/5)
(R0913)
[refactor] 33-33: Too many positional arguments (11/5)
(R0917)
[refactor] 33-33: Too many local variables (20/15)
(R0914)
[refactor] 88-88: Too many arguments (7/5)
(R0913)
[refactor] 88-88: Too many positional arguments (7/5)
(R0917)
[refactor] 88-88: Too many local variables (27/15)
(R0914)
🪛 Ruff (0.11.9)
main.py
126-128: Use a single if statement instead of nested if statements
(SIM102)
🔇 Additional comments (22)
utils/punch_counter.py (2)
49-50: LGTM! Clean integration for combo detection.The addition of
punch_event_historywith a reasonable maximum length of 7 is well-designed to support combo detection while preventing unbounded memory growth.
241-243: LGTM! Proper event recording for combo analysis.The punch event recording correctly uses the timestamp from
timestamp_historyand appends after the punch counting logic, ensuring data consistency for combo detection.tests/test_combo_detector.py (3)
1-17: LGTM! Well-structured test setup.The test setup properly initializes both default and custom detectors, providing good coverage for testing different combo configurations.
112-133: Excellent boundary testing for timing precision.The exact timing boundary tests (lines 116-122 and 127-132) are particularly valuable for validating the epsilon tolerance in the combo detection logic. This helps ensure the system handles floating-point precision issues correctly.
80-98: Good test coverage for combo prioritization logic.The tests properly verify that longer combos are detected in preference to shorter sub-combos when timing permits, which aligns with the sorting logic in ComboDetector.
utils/combo_detector.py (4)
10-31: Well-designed combo definitions.The predefined combos have realistic timing constraints and logical punch sequences that reflect common boxing combinations.
33-37: Smart prioritization strategy.Sorting combos by sequence length in descending order ensures that longer, more specific combos are detected before shorter sub-combos, which is the correct behavior.
74-84: Robust timing validation with appropriate epsilon.The timing validation logic correctly handles floating-point precision issues with a sensible epsilon value, and the logic properly checks consecutive punch intervals against the combo's timing constraints.
4-4: Static analysis hint can be safely ignored.The pylint hint about "too few public methods" is a false positive for this utility class, which has a single, well-defined responsibility of combo detection.
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 4-4: Too few public methods (1/2)
(R0903)
utils/data_manager.py (4)
44-47: Well-designed database schema extension.The new combo-related columns have appropriate data types and sensible default values that maintain backward compatibility with existing data.
92-95: Proper handling of nested combo statistics.The code correctly extracts combo statistics from the nested dictionary structure and uses JSON serialization for the combo details, with appropriate fallbacks to default values.
163-168: Safe JSON deserialization with fallback.The combo_details deserialization properly handles the case where the stored value might be empty or invalid, falling back to an empty dictionary.
194-196: Complete aggregation including combo statistics.The stats summary properly includes combo successes in the aggregation, maintaining consistency with the new schema additions.
main.py (6)
15-15: LGTM on the import statement.The import is correctly added and follows the existing import structure.
64-68: Good implementation of combo stats reset.The combo stats are properly reset for new sessions, and the print statement provides helpful feedback.
104-109: LGTM on paused mode UI parameter updates.The combo-related parameters are correctly passed to the UI manager during paused mode.
154-159: LGTM on normal mode UI parameter updates.The combo-related parameters are correctly passed to the UI manager during normal processing mode.
80-82:Details
✅ Verification successful
Verify combo_stats structure matches DataManager expectations.
The combo_stats addition to session_data looks correct. Ensure the DataManager can properly handle this structure.
🏁 Script executed:
#!/bin/bash # Description: Verify DataManager handles combo_stats in session_data # Expected: Find save_session_data method and check if it handles combo_stats ast-grep --pattern $'def save_session_data($ARG) { $$$ }'Length of output: 60
I need to inspect how
DataManagerhandles the newcombo_statsfield. Let’s search for its class definition, thesave_session_datamethod, and any references tocombo_statsin the repo.
🏁 Script executed:
#!/bin/bash # Find DataManager declaration, save_session_data signature, and any combo_stats usage rg -n "class DataManager" rg -n "def save_session_data" rg -n "combo_stats"Length of output: 1932
Gathering
save_session_dataimplementation for review.
🏁 Script executed:
#!/bin/bash # Display save_session_data implementation and related combo_stats handling sed -n '1,200p' utils/data_manager.py | sed -n '1,120p'Length of output: 4214
DataManager correctly persists combo_stats
The
save_session_datamethod inutils/data_manager.pyalready extracts and stores all threecombo_statsfields—successes,attempts, anddetected_combos—mapping them to thecombo_successes,combo_attempts, andcombo_detailscolumns (withdetected_combosJSON-encoded). No changes are needed.
122-138:Details
✅ Verification successful
Address the nested if statement suggestion and verify combo detection logic.
The combo detection logic is sound, but there's a static analysis hint about simplifying nested if statements. The logic correctly:
- Checks for punch event history
- Detects combos using the ComboDetector
- Handles new combo detection with timing constraints
- Updates statistics and clears history
Apply this diff to address the nested if statement:
- # Combo Detection Logic - if len(self.punch_counter.punch_event_history) > 0: - detected_combo_name = self.combo_detector.detect_combo(self.punch_counter.punch_event_history) - - if detected_combo_name: - if self.current_detected_combo != detected_combo_name or \ - (time.time() - self.last_combo_detected_time > self.combo_display_duration): - - print(f"COMBO DETECTED: {detected_combo_name}") - self.current_detected_combo = detected_combo_name - self.last_combo_detected_time = time.time() - - self.combo_stats['successes'] += 1 - self.combo_stats['detected_combos'][detected_combo_name] = \ - self.combo_stats['detected_combos'].get(detected_combo_name, 0) + 1 - - self.punch_counter.punch_event_history.clear() + # Combo Detection Logic + if (len(self.punch_counter.punch_event_history) > 0 and + (detected_combo_name := self.combo_detector.detect_combo(self.punch_counter.punch_event_history)) and + (self.current_detected_combo != detected_combo_name or + time.time() - self.last_combo_detected_time > self.combo_display_duration)): + + print(f"COMBO DETECTED: {detected_combo_name}") + self.current_detected_combo = detected_combo_name + self.last_combo_detected_time = time.time() + + self.combo_stats['successes'] += 1 + self.combo_stats['detected_combos'][detected_combo_name] = \ + self.combo_stats['detected_combos'].get(detected_combo_name, 0) + 1 + + self.punch_counter.punch_event_history.clear()Verify that punch_event_history exists in PunchCounter:
🏁 Script executed:
#!/bin/bash # Description: Verify punch_event_history attribute exists in PunchCounter # Expected: Find punch_event_history in PunchCounter class ast-grep --pattern $'class PunchCounter { $$$ punch_event_history $$$ }'Length of output: 79
I’m not finding the
PunchCounterclass or itspunch_event_historyattribute yet. Let’s locate them with a broader search:
🏁 Script executed:
#!/bin/bash # Find where PunchCounter is defined rg -n "class PunchCounter" --glob "*.py" # Find any occurrences of punch_event_history rg -n "punch_event_history" --glob "*.py"Length of output: 617
Simplify nested if in combo detection logic
- Verified
punch_event_historyis defined in utils/punch_counter.py:50.- The proposed diff correctly merges the three nested if checks into a single condition using the assignment expression, preserving the original timing constraints and stats updates.
- # Combo Detection Logic - if len(self.punch_counter.punch_event_history) > 0: - detected_combo_name = self.combo_detector.detect_combo(self.punch_counter.punch_event_history) - - if detected_combo_name: - if self.current_detected_combo != detected_combo_name or \ - (time.time() - self.last_combo_detected_time > self.combo_display_duration): - - print(f"COMBO DETECTED: {detected_combo_name}") - self.current_detected_combo = detected_combo_name - self.last_combo_detected_time = time.time() - - self.combo_stats['successes'] += 1 - self.combo_stats['detected_combos'][detected_combo_name] = \ - self.combo_stats['detected_combos'].get(detected_combo_name, 0) + 1 - - self.punch_counter.punch_event_history.clear() + # Combo Detection Logic + if (len(self.punch_counter.punch_event_history) > 0 and + (detected_combo_name := self.combo_detector.detect_combo(self.punch_counter.punch_event_history)) and + (self.current_detected_combo != detected_combo_name or + time.time() - self.last_combo_detected_time > self.combo_display_duration)): + + print(f"COMBO DETECTED: {detected_combo_name}") + self.current_detected_combo = detected_combo_name + self.last_combo_detected_time = time.time() + + self.combo_stats['successes'] += 1 + self.combo_stats['detected_combos'][detected_combo_name] = \ + self.combo_stats['detected_combos'].get(detected_combo_name, 0) + 1 + + self.punch_counter.punch_event_history.clear()🧰 Tools
🪛 Ruff (0.11.9)
126-128: Use a single
ifstatement instead of nestedifstatements(SIM102)
utils/ui_manager.py (3)
22-22: LGTM on panel height increase.The 30-pixel increase appropriately accommodates the new combo statistics display.
119-151: Excellent implementation of sequential positioning.The refactoring to use
current_stat_yfor sequential positioning is a good improvement that makes the layout more maintainable and ensures proper spacing.
154-157: Good use of updated panel height for sensitivity positioning.The sensitivity display correctly uses the new panel height to maintain its position at the bottom of the expanded panel.
| return display_frame | ||
|
|
||
| def _add_stats_panel(self, frame, total_count, punch_counts, session_start_time, sensitivity): | ||
| def _add_stats_panel(self, frame, total_count, punch_counts, session_start_time, sensitivity, combo_stats=None): |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Parameter count concern applies here too.
The _add_stats_panel method also has too many parameters (7/5). This reinforces the need for parameter grouping suggested earlier.
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 88-88: Too many arguments (7/5)
(R0913)
[refactor] 88-88: Too many positional arguments (7/5)
(R0917)
[refactor] 88-88: Too many local variables (27/15)
(R0914)
🤖 Prompt for AI Agents
In utils/ui_manager.py at line 88, the _add_stats_panel method has too many
parameters (7), exceeding the recommended maximum of 5. Refactor this method by
grouping related parameters into one or more data structures or objects, such as
a dictionary or a custom class, to reduce the parameter count and improve
readability.
Docstrings generation was requested by @tblakex01. * #5 (comment) The following files were modified: * `main.py` * `tests/test_combo_detector.py` * `utils/combo_detector.py` * `utils/data_manager.py` * `utils/punch_counter.py` * `utils/ui_manager.py`
|
Note Generated docstrings for this pull request at #6 |
This feature introduces the ability to detect predefined punch combinations (e.g., Jab-Cross, Jab-Cross-Hook) based on the sequence and timing of punches.
Key changes include:
utils/combo_detector.py(New):ComboDetectorclass to manage and detect combos.inter-punch timing thresholds.
utils/punch_counter.py:punch_event_history(a deque) to store recent(punch_type, timestamp)tuples for combo analysis.main.py:ComboDetectorinto the main processing loop.overlapping detections.
utils/ui_manager.py:short duration.
utils/data_manager.py:sessionstable) to includecombo_successes,combo_attempts, andcombo_details(JSONstring for counts of each detected combo type).
combo statistics.
tests/test_combo_detector.py(New):ComboDetector, coveringvarious scenarios including timing, sequence, boundary conditions,
and prioritization. These tests helped identify and fix a float
precision issue in timing.
This feature enhances user engagement by adding a gamified element and allowing you to get feedback on specific boxing combinations.
Summary by Sourcery
Implement punch combination detection by introducing a ComboDetector class and integrating it into the main application loop, UI, and database to track, display, and persist predefined punch combos.
New Features:
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit
New Features
UI Improvements
Bug Fixes
Tests