refactor: eliminate code duplication in cursor operations and cleanup
- Extract cursor application logic into apply_cursor_by_index() helper - Extract cleanup logic into perform_app_cleanup() helper - Update select_cursor(), next_cursor(), and system tray handlers to use helpers - Reduce ~55 lines of duplicated code while maintaining functionality - Mark step 1 of code cleanup TODO as completed
This commit is contained in:
@@ -0,0 +1,38 @@
|
|||||||
|
# Code Cleanup TODO
|
||||||
|
|
||||||
|
## 🔥 Critical Issues
|
||||||
|
|
||||||
|
### 1. Code Duplication
|
||||||
|
- [x] Extract cursor application logic from `select_cursor()` and `next_cursor()`
|
||||||
|
- [x] Extract cleanup logic from `quit_app()` and system tray quit handler
|
||||||
|
|
||||||
|
### 2. Steam Integration
|
||||||
|
- [ ] Replace `unwrap()` with proper error handling in `Steam::new()`- Leave this on the backburner for now
|
||||||
|
- [ ] App should work fine even if Steam is offline (Steam remembers inventory, we pull from Steam even in offline mode) - Leave this on the backburner for now
|
||||||
|
|
||||||
|
### 3. Unsafe Thread-Safety
|
||||||
|
- [ ] Document or fix `SyncHCURSOR` unsafe impls in `types.rs`
|
||||||
|
|
||||||
|
## 🚨 High Priority
|
||||||
|
|
||||||
|
### 4. Debug Logging Spam
|
||||||
|
- [ ] Remove 73 debug print statements from production code
|
||||||
|
- [ ] Add proper logging framework
|
||||||
|
|
||||||
|
### 5. Mutex Unwraps
|
||||||
|
- [ ] Replace 9 `lock().unwrap()` calls with proper error handling
|
||||||
|
|
||||||
|
### 6. Large Functions
|
||||||
|
- [ ] Break down `load_cursor_files()` (90+ lines)
|
||||||
|
- [ ] Break down `create_cursor_from_image()` (170+ lines)
|
||||||
|
|
||||||
|
## 📋 Medium Priority
|
||||||
|
|
||||||
|
### 7. Mock Data Fallbacks
|
||||||
|
- [ ] Remove hardcoded fallback cursors pointing to non-existent files
|
||||||
|
|
||||||
|
### 8. Inline Event Handlers
|
||||||
|
- [ ] Extract system tray/window event handlers from Tauri builder
|
||||||
|
|
||||||
|
### 9. Error Handling
|
||||||
|
- [ ] Standardize error types and handling patterns
|
||||||
@@ -44,6 +44,59 @@ fn get_current_cursor_index(state: tauri::State<AppStateWrapper>) -> usize {
|
|||||||
app_state.current_index
|
app_state.current_index
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Helper function to apply a cursor by index with proper error handling and event emission
|
||||||
|
fn apply_cursor_by_index(
|
||||||
|
index: usize,
|
||||||
|
app_state: &AppState,
|
||||||
|
app_handle: &AppHandle,
|
||||||
|
) -> Result<(), String> {
|
||||||
|
if index >= app_state.cursors.len() {
|
||||||
|
return Err("Index out of bounds".to_string());
|
||||||
|
}
|
||||||
|
|
||||||
|
let cursor = &app_state.cursors[index];
|
||||||
|
println!("Applying cursor: {} ({})", cursor.name, cursor.path);
|
||||||
|
|
||||||
|
// Verify the file exists
|
||||||
|
if !Path::new(&cursor.path).exists() {
|
||||||
|
println!("File doesn't exist: {}", cursor.path);
|
||||||
|
return Err(format!("Cursor file does not exist: {}", cursor.path));
|
||||||
|
}
|
||||||
|
|
||||||
|
println!("File exists, applying cursor...");
|
||||||
|
|
||||||
|
// Apply the cursor
|
||||||
|
if let Err(e) = platform::set_system_cursor(&cursor.path) {
|
||||||
|
println!("Error applying cursor: {}", e);
|
||||||
|
return Err(format!("Failed to apply cursor '{}': {}", cursor.name, e));
|
||||||
|
}
|
||||||
|
|
||||||
|
// Emit an event to notify the frontend
|
||||||
|
let _ = app_handle.emit_all("cursor_changed", index); // Ignore error
|
||||||
|
|
||||||
|
println!("Successfully applied cursor using actual file path");
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
// Helper function to perform application cleanup (Steam shutdown and cursor restoration)
|
||||||
|
fn perform_app_cleanup() {
|
||||||
|
println!("Performing application cleanup...");
|
||||||
|
|
||||||
|
// Clean up Steam integration
|
||||||
|
{
|
||||||
|
let steam = STEAM.lock();
|
||||||
|
steam.shutdown();
|
||||||
|
}
|
||||||
|
|
||||||
|
// Clean up and restore default cursor before exit
|
||||||
|
match platform::restore_system_cursor() {
|
||||||
|
Ok(_) => println!("Default cursor restored"),
|
||||||
|
Err(e) => eprintln!("Failed to restore default cursor: {}", e),
|
||||||
|
}
|
||||||
|
|
||||||
|
println!("Cleanup complete");
|
||||||
|
}
|
||||||
|
|
||||||
// Command to select a cursor
|
// Command to select a cursor
|
||||||
#[tauri::command]
|
#[tauri::command]
|
||||||
fn select_cursor(
|
fn select_cursor(
|
||||||
@@ -96,24 +149,8 @@ fn select_cursor(
|
|||||||
// Try using the actual file path directly
|
// Try using the actual file path directly
|
||||||
println!("Using direct file path: {}", cursor.path);
|
println!("Using direct file path: {}", cursor.path);
|
||||||
|
|
||||||
// Verify the file exists
|
// Use the helper function to apply the cursor
|
||||||
if Path::new(&cursor.path).exists() {
|
apply_cursor_by_index(index, &app_state, &app_handle)
|
||||||
println!("File exists, applying cursor...");
|
|
||||||
|
|
||||||
if let Err(e) = platform::set_system_cursor(&cursor.path) {
|
|
||||||
println!("Error applying cursor: {}", e);
|
|
||||||
return Err(format!("Failed to apply cursor '{}': {}", cursor.name, e));
|
|
||||||
}
|
|
||||||
|
|
||||||
// Emit an event to notify the frontend
|
|
||||||
let _ = app_handle.emit_all("cursor_changed", index); // Ignore error
|
|
||||||
|
|
||||||
println!("Successfully applied cursor using actual file path");
|
|
||||||
return Ok(());
|
|
||||||
} else {
|
|
||||||
println!("File doesn't exist: {}", cursor.path);
|
|
||||||
return Err(format!("Cursor file does not exist: {}", cursor.path));
|
|
||||||
}
|
|
||||||
} else {
|
} else {
|
||||||
Err("Index out of bounds".to_string())
|
Err("Index out of bounds".to_string())
|
||||||
}
|
}
|
||||||
@@ -145,24 +182,9 @@ fn next_cursor(
|
|||||||
// Try using the actual file path directly
|
// Try using the actual file path directly
|
||||||
println!("Using direct file path: {}", cursor.path);
|
println!("Using direct file path: {}", cursor.path);
|
||||||
|
|
||||||
// Verify the file exists
|
// Use the helper function to apply the cursor
|
||||||
if Path::new(&cursor.path).exists() {
|
apply_cursor_by_index(app_state.current_index, &app_state, &app_handle)?;
|
||||||
println!("File exists, applying cursor...");
|
Ok(app_state.current_index)
|
||||||
|
|
||||||
if let Err(e) = platform::set_system_cursor(&cursor.path) {
|
|
||||||
println!("Error applying cursor: {}", e);
|
|
||||||
return Err(format!("Failed to apply cursor '{}': {}", cursor.name, e));
|
|
||||||
}
|
|
||||||
|
|
||||||
// Emit an event to notify the frontend
|
|
||||||
let _ = app_handle.emit_all("cursor_changed", app_state.current_index); // Ignore error
|
|
||||||
|
|
||||||
println!("Successfully applied cursor using actual file path");
|
|
||||||
return Ok(app_state.current_index);
|
|
||||||
} else {
|
|
||||||
println!("File doesn't exist: {}", cursor.path);
|
|
||||||
return Err(format!("Cursor file does not exist: {}", cursor.path));
|
|
||||||
}
|
|
||||||
} else {
|
} else {
|
||||||
Err("No cursors available".to_string())
|
Err("No cursors available".to_string())
|
||||||
}
|
}
|
||||||
@@ -171,21 +193,12 @@ fn next_cursor(
|
|||||||
// Command to quit the application
|
// Command to quit the application
|
||||||
#[tauri::command]
|
#[tauri::command]
|
||||||
fn quit_app(app_handle: AppHandle) {
|
fn quit_app(app_handle: AppHandle) {
|
||||||
println!("Quit app command received, performing cleanup");
|
println!("Quit app command received");
|
||||||
|
|
||||||
// Clean up Steam integration
|
// Use the helper function to perform cleanup
|
||||||
{
|
perform_app_cleanup();
|
||||||
let steam = STEAM.lock();
|
|
||||||
steam.shutdown();
|
|
||||||
}
|
|
||||||
|
|
||||||
// Clean up and restore default cursor before exit
|
println!("Exiting application");
|
||||||
match platform::restore_system_cursor() {
|
|
||||||
Ok(_) => println!("Default cursor restored"),
|
|
||||||
Err(e) => eprintln!("Failed to restore default cursor: {}", e),
|
|
||||||
}
|
|
||||||
|
|
||||||
println!("Cleanup complete, exiting application");
|
|
||||||
|
|
||||||
// Exit the application
|
// Exit the application
|
||||||
std::thread::spawn(move || {
|
std::thread::spawn(move || {
|
||||||
@@ -432,15 +445,10 @@ pub fn run() {
|
|||||||
if !app_state.cursors.is_empty() {
|
if !app_state.cursors.is_empty() {
|
||||||
app_state.current_index =
|
app_state.current_index =
|
||||||
(app_state.current_index + 1) % app_state.cursors.len();
|
(app_state.current_index + 1) % app_state.cursors.len();
|
||||||
let cursor = &app_state.cursors[app_state.current_index];
|
|
||||||
|
|
||||||
if Path::new(&cursor.path).exists() {
|
// Use the helper function to apply the cursor
|
||||||
if let Err(e) = platform::set_system_cursor(&cursor.path) {
|
if let Err(e) = apply_cursor_by_index(app_state.current_index, &app_state, app) {
|
||||||
eprintln!("Error applying cursor from tray: {}", e);
|
eprintln!("Error applying cursor from tray: {}", e);
|
||||||
} else {
|
|
||||||
// Emit an event to notify the frontend
|
|
||||||
let _ = app.emit_all("cursor_changed", app_state.current_index);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -460,19 +468,10 @@ pub fn run() {
|
|||||||
"quit" => {
|
"quit" => {
|
||||||
println!("Quit requested from tray");
|
println!("Quit requested from tray");
|
||||||
|
|
||||||
// Clean up Steam integration
|
// Use the helper function to perform cleanup
|
||||||
{
|
perform_app_cleanup();
|
||||||
let steam = STEAM.lock();
|
|
||||||
steam.shutdown();
|
|
||||||
}
|
|
||||||
|
|
||||||
// Clean up and restore default cursor before exit
|
println!("Exiting from tray");
|
||||||
match platform::restore_system_cursor() {
|
|
||||||
Ok(_) => println!("Default cursor restored"),
|
|
||||||
Err(e) => eprintln!("Failed to restore default cursor: {}", e),
|
|
||||||
}
|
|
||||||
|
|
||||||
println!("Cleanup complete, exiting from tray");
|
|
||||||
app.exit(0);
|
app.exit(0);
|
||||||
}
|
}
|
||||||
_ => {}
|
_ => {}
|
||||||
|
|||||||
Reference in New Issue
Block a user