From c1b8160f4a48e914aa8116e5be189125301bd7bb Mon Sep 17 00:00:00 2001 From: JobyGitGud Date: Fri, 30 May 2025 22:24:33 -0500 Subject: [PATCH] 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 --- CODE_CLEANUP_TODO.md | 38 ++++++++++ frontend/src-tauri/src/lib.rs | 137 +++++++++++++++++----------------- 2 files changed, 106 insertions(+), 69 deletions(-) create mode 100644 CODE_CLEANUP_TODO.md diff --git a/CODE_CLEANUP_TODO.md b/CODE_CLEANUP_TODO.md new file mode 100644 index 0000000..d08bb37 --- /dev/null +++ b/CODE_CLEANUP_TODO.md @@ -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 diff --git a/frontend/src-tauri/src/lib.rs b/frontend/src-tauri/src/lib.rs index d560e9e..4509f0f 100644 --- a/frontend/src-tauri/src/lib.rs +++ b/frontend/src-tauri/src/lib.rs @@ -44,6 +44,59 @@ fn get_current_cursor_index(state: tauri::State) -> usize { 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 #[tauri::command] fn select_cursor( @@ -96,24 +149,8 @@ fn select_cursor( // Try using the actual file path directly println!("Using direct file path: {}", cursor.path); - // Verify the file exists - if Path::new(&cursor.path).exists() { - 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)); - } + // Use the helper function to apply the cursor + apply_cursor_by_index(index, &app_state, &app_handle) } else { Err("Index out of bounds".to_string()) } @@ -145,24 +182,9 @@ fn next_cursor( // Try using the actual file path directly println!("Using direct file path: {}", cursor.path); - // Verify the file exists - if Path::new(&cursor.path).exists() { - 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", 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)); - } + // Use the helper function to apply the cursor + apply_cursor_by_index(app_state.current_index, &app_state, &app_handle)?; + Ok(app_state.current_index) } else { Err("No cursors available".to_string()) } @@ -171,21 +193,12 @@ fn next_cursor( // Command to quit the application #[tauri::command] fn quit_app(app_handle: AppHandle) { - println!("Quit app command received, performing cleanup"); + println!("Quit app command received"); - // Clean up Steam integration - { - let steam = STEAM.lock(); - steam.shutdown(); - } + // Use the helper function to perform cleanup + perform_app_cleanup(); - // 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, exiting application"); + println!("Exiting application"); // Exit the application std::thread::spawn(move || { @@ -432,15 +445,10 @@ pub fn run() { if !app_state.cursors.is_empty() { app_state.current_index = (app_state.current_index + 1) % app_state.cursors.len(); - let cursor = &app_state.cursors[app_state.current_index]; - if Path::new(&cursor.path).exists() { - if let Err(e) = platform::set_system_cursor(&cursor.path) { - 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); - } + // Use the helper function to apply the cursor + if let Err(e) = apply_cursor_by_index(app_state.current_index, &app_state, app) { + eprintln!("Error applying cursor from tray: {}", e); } } } @@ -460,19 +468,10 @@ pub fn run() { "quit" => { println!("Quit requested from tray"); - // Clean up Steam integration - { - let steam = STEAM.lock(); - steam.shutdown(); - } + // Use the helper function to perform cleanup + perform_app_cleanup(); - // 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, exiting from tray"); + println!("Exiting from tray"); app.exit(0); } _ => {}