-
Notifications
You must be signed in to change notification settings - Fork 1
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
Conversation
…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.
…th Korean vital points
…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.
…system, and improved exports
|
There was a problem hiding this 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"; |
There was a problem hiding this comment.
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"; |
There was a problem hiding this comment.
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"; |
There was a problem hiding this comment.
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"; |
There was a problem hiding this comment.
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 }'.
import { KOREAN_COLORS } from "../../../types/constants"; | |
import { KOREAN_COLORS } from "../../../types/constants"; |
Copilot uses AI. Check for mistakes.
const selectedPoint = availableVitalPoints.find( | ||
(point) => point.id === selectedVitalPoint |
There was a problem hiding this comment.
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.
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 => { |
There was a problem hiding this comment.
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> = ({ |
There was a problem hiding this comment.
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> = ({ |
There was a problem hiding this comment.
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={{ |
There was a problem hiding this comment.
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.
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.