Skip to content

Refactor and enhance TrainingScreen components #171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 24, 2025
Merged

Refactor and enhance TrainingScreen components #171

merged 6 commits into from
Jul 24, 2025

Conversation

pethers
Copy link
Member

@pethers pethers commented Jul 24, 2025

Refactor the TrainingScreen component to improve structure and functionality by introducing new modular components. Enhance training session management, feedback mechanisms, and visual indicators for better user experience. Add a new VitalPointTrainingPanel for targeted practice with Korean vital points. Update documentation and export structure for improved maintainability.

pethers added 5 commits July 24, 2025 08:43
…nctionality

- Simplified props destructuring for better readability.
- Enhanced training session management with clearer start/stop logic.
- Improved feedback mechanism for training actions.
- Updated player and dummy health management with visual indicators.
- Refined layout using responsive design principles for better mobile support.
- Added detailed control instructions and player status display.
- Streamlined stance change handling and incorporated keyboard shortcuts.
- Enhanced dummy reset functionality and feedback display.
- Introduced new components: TrainingControlsPanel, TrainingDummy, TrainingFeedback, TrainingModeSelector, and TrainingStatsPanel to modularize the training screen functionality.
- Removed inline logic from TrainingScreen and delegated responsibilities to the new components for better readability and maintainability.
- Updated the TrainingScreen to utilize the new components, enhancing the structure and organization of the code.
- Improved hit detection logic in TrainingDummy and feedback display in TrainingFeedback.
- Added type exports for TrainingModeSelectorProps and TrainingStatsPanelProps for better type management.
…interaction

- Integrated Korean anatomy vital points with detailed properties.
- Added dynamic visibility of vital points based on training mode.
- Implemented hit detection and recent hit tracking for vital points.
- Enhanced visual representation of the training dummy with Korean-inspired design elements.
- Improved interaction handling for vital points, including hover and click events.
- Added distance indicators and training state visuals.
- Introduced hit effects with traditional Korean styling.
@github-actions github-actions bot added ui User interface improvements needs-tests Needs test coverage labels Jul 24, 2025
@pethers pethers requested a review from Copilot July 24, 2025 10:23
Copilot

This comment was marked as outdated.

@pethers pethers requested a review from Copilot July 24, 2025 11:16
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Copilot

This comment was marked as outdated.

@pethers pethers requested a review from Copilot July 24, 2025 11:50
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors and enhances the TrainingScreen component by breaking it down into modular, reusable components and adding new training features. The main goals are to improve code maintainability, enhance the training experience with better visual feedback, and introduce vital point training capabilities with authentic Korean martial arts elements.

Key changes include:

  • Modularization of the training interface into 6 specialized components
  • Enhanced training session management with improved scoring and combo systems
  • New VitalPointTrainingPanel for targeted practice with Korean vital points
  • Comprehensive layout system with responsive design for mobile and desktop

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
tsconfig.tsbuildinfo Build cache update reflecting new component structure
src/components/training/index.ts Enhanced exports with new training components and type definitions
src/components/training/components/index.ts New index file for training subcomponents with proper exports
src/components/training/components/VitalPointTrainingPanel.tsx New component for Korean vital point selection and training interface
src/components/training/components/TrainingStatsPanel.tsx Enhanced statistics display with Korean martial arts theming
src/components/training/components/TrainingModeSelector.tsx Mode selection component with Korean language support
src/components/training/components/TrainingFeedback.tsx Visual feedback system with animations and Korean text
src/components/training/components/TrainingDummy.tsx Comprehensive training dummy with vital points and Korean anatomy
src/components/training/components/TrainingControlsPanel.tsx Control panel for training session management
src/components/training/TrainingScreen.tsx Major refactor using new modular components with enhanced layout

@@ -1,22 +1,21 @@
import { PlayerState } from "@/systems";
import { usePlayerMovement } from "@/utils/inputSystem";
Copy link
Preview

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import should be organized with external imports first, then internal imports. The @/ import should come after the other project imports like audio and types, not at the top.

Copilot generated this review using guidance from copilot-instructions.md.

@@ -0,0 +1,201 @@
import { PlayerState } from "@/systems";
Copy link
Preview

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import organization violates the guidelines. External imports should come first, followed by internal imports. The @/systems import should come after React import but before relative imports.

Copilot generated this review using guidance from copilot-instructions.md.

@@ -0,0 +1,800 @@
import "@pixi/layout";
Copy link
Preview

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import organization is incorrect. External imports should be grouped together, but @pixi/layout should come after the pixi.js imports, not at the very top. Consider grouping all PixiJS-related imports together.

Copilot generated this review using guidance from copilot-instructions.md.

@@ -0,0 +1,138 @@
import React, { useEffect, useState } from "react";
import { KOREAN_COLORS } from "../../../types/constants";
Copy link
Preview

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra whitespace in destructuring import. Should be 'import { KOREAN_COLORS }' instead of 'import { KOREAN_COLORS }'.

Suggested change
import { KOREAN_COLORS } from "../../../types/constants";
import { KOREAN_COLORS } from "../../../types/constants";

Copilot uses AI. Check for mistakes.

Comment on lines +52 to +53
const selectedPoint = availableVitalPoints.find(
(point) => point.id === selectedVitalPoint
Copy link
Preview

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The find operation is performed on every render. Consider memoizing this calculation with useMemo to avoid unnecessary computations when availableVitalPoints or selectedVitalPoint haven't changed.

Suggested change
const selectedPoint = availableVitalPoints.find(
(point) => point.id === selectedVitalPoint
const selectedPoint = useMemo(
() => availableVitalPoints.find((point) => point.id === selectedVitalPoint),
[availableVitalPoints, selectedVitalPoint]

Copilot uses AI. Check for mistakes.


// Get vital point color based on severity and state
const getVitalPointColor = useCallback(
(vitalPoint: VitalPoint): number => {
Copy link
Preview

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The getVitalPointColor function is recreated on every render due to dependencies in useCallback. Consider memoizing some of the expensive calculations like the recentHits lookup or breaking this into smaller, more stable functions.

Copilot uses AI. Check for mistakes.

);
};

export const TrainingScreen: React.FC<TrainingScreenProps> = ({
Copy link
Preview

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function should have explicit return type. Add ': JSX.Element' to the function declaration for better TypeScript compliance.

Copilot generated this review using guidance from copilot-instructions.md.

},
];

export const TrainingDummy: React.FC<TrainingDummyProps> = ({
Copy link
Preview

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function should have explicit return type. Add ': JSX.Element' to the function declaration for better TypeScript compliance.

Copilot generated this review using guidance from copilot-instructions.md.

y={y + offsetY}
alpha={alpha}
data-testid="training-feedback"
layout={{
Copy link
Preview

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The layout prop with flexDirection and positioning properties should use PixiJS Layout system properly. Consider using @pixi/layout components or ensure the layout properties are properly supported by the pixiContainer component.

Copilot uses AI. Check for mistakes.

@pethers pethers merged commit 99c94da into main Jul 24, 2025
12 of 13 checks passed
@pethers pethers deleted the training branch July 24, 2025 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-tests Needs test coverage ui User interface improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant