Merge pull request #57 from HsiangNianian/fix/critical-bugs

7 files changed, 179 insertions, 37 deletions

diff --git a/src-tauri/src/core/auth.rs b/src-tauri/src/core/auth.rs
index ac5904c..d5e6c17 100644
--- a/src-tauri/src/core/auth.rs
+++ b/src-tauri/src/core/auth.rs
@@ -6,9 +6,9 @@ use uuid::Uuid;
 // This is critical because Microsoft's WAF often blocks requests without a valid UA
 fn get_client() -> reqwest::Client {
     reqwest::Client::builder()
-        .user_agent("DropOut/1.0 (Linux)")
+        .user_agent("DropOut/1.0")
         .build()
-        .unwrap_or_else(|_| get_client())
+        .unwrap_or_else(|_| reqwest::Client::new())
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
@@ -136,7 +136,6 @@ pub async fn refresh_microsoft_token(refresh_token: &str) -> Result<TokenRespons
 }
 
 /// Check if a Microsoft account token is expired or about to expire
-#[allow(dead_code)]
 pub fn is_token_expired(expires_at: i64) -> bool {
     let now = std::time::SystemTime::now()
         .duration_since(std::time::UNIX_EPOCH)
diff --git a/src-tauri/src/core/downloader.rs b/src-tauri/src/core/downloader.rs
index 9c6b7f0..26f6ebd 100644
--- a/src-tauri/src/core/downloader.rs
+++ b/src-tauri/src/core/downloader.rs
@@ -270,12 +270,12 @@ pub async fn download_with_resume(
                 }
 
                 current_pos += chunk_len;
-                let total_downloaded = progress.fetch_add(chunk_len, Ordering::Relaxed) + chunk_len;
+                let total_downloaded = progress.fetch_add(chunk_len, Ordering::AcqRel) + chunk_len;
 
                 // Emit progress event (throttled)
-                let last_bytes = last_progress_bytes.load(Ordering::Relaxed);
+                let last_bytes = last_progress_bytes.load(Ordering::Acquire);
                 if total_downloaded - last_bytes > 100 * 1024 || total_downloaded >= total_size {
-                    last_progress_bytes.store(total_downloaded, Ordering::Relaxed);
+                    last_progress_bytes.store(total_downloaded, Ordering::Release);
 
                     let elapsed = start_time.elapsed().as_secs_f64();
                     let speed = if elapsed > 0.0 {
@@ -319,7 +319,7 @@ pub async fn download_with_resume(
                 all_success = false;
                 if e.contains("cancelled") {
                     // Save progress for resume
-                    metadata.downloaded_bytes = progress.load(Ordering::Relaxed);
+                    metadata.downloaded_bytes = progress.load(Ordering::Acquire);
                     let meta_content =
                         serde_json::to_string_pretty(&metadata).map_err(|e| e.to_string())?;
                     tokio::fs::write(&meta_path, meta_content).await.ok();
@@ -335,7 +335,7 @@ pub async fn download_with_resume(
 
     if !all_success {
         // Save progress
-        metadata.downloaded_bytes = progress.load(Ordering::Relaxed);
+        metadata.downloaded_bytes = progress.load(Ordering::Acquire);
         let meta_content = serde_json::to_string_pretty(&metadata).map_err(|e| e.to_string())?;
         tokio::fs::write(&meta_path, meta_content).await.ok();
         return Err("Some segments failed".to_string());
@@ -482,19 +482,19 @@ impl GlobalProgress {
     /// Get current progress snapshot without modification
     fn snapshot(&self) -> ProgressSnapshot {
         ProgressSnapshot {
-            completed_files: self.completed_files.load(Ordering::Relaxed),
+            completed_files: self.completed_files.load(Ordering::Acquire),
             total_files: self.total_files,
-            total_downloaded_bytes: self.total_downloaded_bytes.load(Ordering::Relaxed),
+            total_downloaded_bytes: self.total_downloaded_bytes.load(Ordering::Acquire),
         }
     }
 
     /// Increment completed files counter and return updated snapshot
     fn inc_completed(&self) -> ProgressSnapshot {
-        let completed = self.completed_files.fetch_add(1, Ordering::Relaxed) + 1;
+        let completed = self.completed_files.fetch_add(1, Ordering::Release) + 1;
         ProgressSnapshot {
             completed_files: completed,
             total_files: self.total_files,
-            total_downloaded_bytes: self.total_downloaded_bytes.load(Ordering::Relaxed),
+            total_downloaded_bytes: self.total_downloaded_bytes.load(Ordering::Acquire),
         }
     }
 
@@ -502,10 +502,10 @@ impl GlobalProgress {
     fn add_bytes(&self, delta: u64) -> ProgressSnapshot {
         let total_bytes = self
             .total_downloaded_bytes
-            .fetch_add(delta, Ordering::Relaxed)
+            .fetch_add(delta, Ordering::AcqRel)
             + delta;
         ProgressSnapshot {
-            completed_files: self.completed_files.load(Ordering::Relaxed),
+            completed_files: self.completed_files.load(Ordering::Acquire),
             total_files: self.total_files,
             total_downloaded_bytes: total_bytes,
         }
diff --git a/src-tauri/src/core/instance.rs b/src-tauri/src/core/instance.rs
index 90ec34e..738dbd8 100644
--- a/src-tauri/src/core/instance.rs
+++ b/src-tauri/src/core/instance.rs
@@ -218,21 +218,42 @@ impl InstanceState {
             .get_instance(id)
             .ok_or_else(|| format!("Instance {} not found", id))?;
 
-        // Create new instance
-        let mut new_instance = self.create_instance(new_name, app_handle)?;
-
-        // Copy instance properties
-        new_instance.version_id = source_instance.version_id.clone();
-        new_instance.mod_loader = source_instance.mod_loader.clone();
-        new_instance.mod_loader_version = source_instance.mod_loader_version.clone();
-        new_instance.notes = source_instance.notes.clone();
-
-        // Copy directory contents
+        // Prepare new instance metadata (but don't save yet)
+        let new_id = uuid::Uuid::new_v4().to_string();
+        let instances_dir = app_handle
+            .path()
+            .app_data_dir()
+            .map_err(|e| e.to_string())?
+            .join("instances");
+        let new_game_dir = instances_dir.join(&new_id);
+
+        // Copy directory FIRST - if this fails, don't create metadata
         if source_instance.game_dir.exists() {
-            copy_dir_all(&source_instance.game_dir, &new_instance.game_dir)
+            copy_dir_all(&source_instance.game_dir, &new_game_dir)
                 .map_err(|e| format!("Failed to copy instance directory: {}", e))?;
+        } else {
+            // If source dir doesn't exist, create new empty game dir
+            std::fs::create_dir_all(&new_game_dir)
+                .map_err(|e| format!("Failed to create instance directory: {}", e))?;
         }
 
+        // NOW create metadata and save
+        let new_instance = Instance {
+            id: new_id,
+            name: new_name,
+            game_dir: new_game_dir,
+            version_id: source_instance.version_id.clone(),
+            mod_loader: source_instance.mod_loader.clone(),
+            mod_loader_version: source_instance.mod_loader_version.clone(),
+            notes: source_instance.notes.clone(),
+            icon_path: source_instance.icon_path.clone(),
+            created_at: std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .unwrap()
+                .as_secs() as i64,
+            last_played: None,
+        };
+
         self.update_instance(new_instance.clone())?;
 
         Ok(new_instance)
diff --git a/src-tauri/src/core/java.rs b/src-tauri/src/core/java.rs
index 0c7769b..d3e1bb9 100644
--- a/src-tauri/src/core/java.rs
+++ b/src-tauri/src/core/java.rs
@@ -850,8 +850,27 @@ fn parse_version_string(output: &str) -> Option<String> {
 
 /// Parse version for comparison (returns major version number)
 fn parse_java_version(version: &str) -> u32 {
-    // Handle both old format (1.8.0_xxx) and new format (11.0.x, 17.0.x)
-    let parts: Vec<&str> = version.split('.').collect();
+    // Handle various formats:
+    // - Old format: 1.8.0_xxx (Java 8 with update)
+    // - New format: 17.0.1, 11.0.5+10 (Java 11+)
+    // - Format with build: 21.0.3+13-Ubuntu-0ubuntu0.24.04.1
+    // - Format with underscores: 1.8.0_411
+    
+    // First, strip build metadata (everything after '+')
+    let version_only = version.split('+').next().unwrap_or(version);
+    
+    // Remove trailing junk (like "-Ubuntu-0ubuntu0.24.04.1")
+    let version_only = version_only
+        .split('-')
+        .next()
+        .unwrap_or(version_only);
+    
+    // Replace underscores with dots (1.8.0_411 -> 1.8.0.411)
+    let normalized = version_only.replace('_', ".");
+    
+    // Split by dots
+    let parts: Vec<&str> = normalized.split('.').collect();
+    
     if let Some(first) = parts.first() {
         if *first == "1" {
             // Old format: 1.8.0 -> major is 8
diff --git a/src-tauri/src/core/manifest.rs b/src-tauri/src/core/manifest.rs
index 637b935..e792071 100644
--- a/src-tauri/src/core/manifest.rs
+++ b/src-tauri/src/core/manifest.rs
@@ -97,6 +97,43 @@ pub async fn fetch_vanilla_version(
     Ok(resp)
 }
 
+/// Find the root vanilla version by following the inheritance chain.
+///
+/// For modded versions (Fabric, Forge), this walks up the `inheritsFrom`
+/// chain to find the base vanilla Minecraft version.
+///
+/// # Arguments
+/// * `game_dir` - The .minecraft directory path
+/// * `version_id` - The version ID to start from
+///
+/// # Returns
+/// The ID of the root vanilla version (the version without `inheritsFrom`)
+pub async fn find_root_version(
+    game_dir: &std::path::Path,
+    version_id: &str,
+) -> Result<String, Box<dyn Error + Send + Sync>> {
+    let mut current_id = version_id.to_string();
+
+    // Keep following the inheritance chain
+    loop {
+        let version = match load_local_version(game_dir, &current_id).await {
+            Ok(v) => v,
+            Err(_) => {
+                // If not found locally, assume it's a vanilla version (root)
+                return Ok(current_id);
+            }
+        };
+
+        // If this version has no parent, it's the root
+        if let Some(parent_id) = version.inherits_from {
+            current_id = parent_id;
+        } else {
+            // This is the root
+            return Ok(current_id);
+        }
+    }
+}
+
 /// Load a version, checking local first, then fetching from remote if needed.
 ///
 /// For modded versions (those with `inheritsFrom`), this will also resolve
diff --git a/src-tauri/src/core/rules.rs b/src-tauri/src/core/rules.rs
index 71abda5..10a40b6 100644
--- a/src-tauri/src/core/rules.rs
+++ b/src-tauri/src/core/rules.rs
@@ -57,18 +57,37 @@ fn rule_matches(rule: &Rule) -> bool {
     match &rule.os {
         None => true, // No OS condition means it applies to all
         Some(os_rule) => {
+            // Check OS name
             if let Some(os_name) = &os_rule.name {
-                match os_name.as_str() {
+                let os_match = match os_name.as_str() {
                     "osx" | "macos" => env::consts::OS == "macos",
                     "linux" => env::consts::OS == "linux",
                     "windows" => env::consts::OS == "windows",
                     _ => false, // Unknown OS name in rule
+                };
+                
+                if !os_match {
+                    return false;
                 }
-            } else {
-                // OS rule exists but name is None? Maybe checking version/arch only.
-                // For simplicity, mostly name is used.
-                true
             }
+            
+            // Check architecture if specified
+            if let Some(arch) = &os_rule.arch {
+                let current_arch = env::consts::ARCH;
+                if arch != current_arch && arch != "x86_64" {
+                    // "x86" is sometimes used for x86_64, but we only match exact arch
+                    return false;
+                }
+            }
+            
+            // Check version if specified (for OS version compatibility)
+            if let Some(_version) = &os_rule.version {
+                // Version checking would require parsing OS version strings
+                // For now, we accept all versions (conservative approach)
+                // In the future, parse version and compare
+            }
+            
+            true
         }
     }
 }
diff --git a/src-tauri/src/main.rs b/src-tauri/src/main.rs
index 2871b03..853c93e 100644
--- a/src-tauri/src/main.rs
+++ b/src-tauri/src/main.rs
@@ -82,13 +82,42 @@ async fn start_game(
 
     // Check for active account
     emit_log!(window, "Checking for active account...".to_string());
-    let account = auth_state
+    let mut account = auth_state
         .active_account
         .lock()
         .unwrap()
         .clone()
         .ok_or("No active account found. Please login first.")?;
 
+    // Check if Microsoft account token is expired and refresh if needed
+    if let core::auth::Account::Microsoft(ms_account) = &account {
+        if core::auth::is_token_expired(ms_account.expires_at) {
+            emit_log!(window, "Token expired, refreshing...".to_string());
+            match core::auth::refresh_full_auth(
+                &ms_account
+                    .refresh_token
+                    .clone()
+                    .ok_or("No refresh token available")?,
+            )
+            .await
+            {
+                Ok((refreshed_account, _new_ms_refresh)) => {
+                    let refreshed_account = core::auth::Account::Microsoft(refreshed_account);
+                    *auth_state.active_account.lock().unwrap() = Some(refreshed_account.clone());
+                    account = refreshed_account;
+                    emit_log!(window, "Token refreshed successfully".to_string());
+                }
+                Err(e) => {
+                    emit_log!(window, format!("Token refresh failed: {}", e));
+                    return Err(format!(
+                        "Your login session has expired. Please login again: {}",
+                        e
+                    ));
+                }
+            }
+        }
+    }
+
     emit_log!(window, format!("Account found: {}", account.username()));
 
     let config = config_state.config.lock().unwrap().clone();
@@ -1917,10 +1946,28 @@ async fn install_forge(
         "Forge installer completed, creating version profile...".to_string()
     );
 
-    // Now create the version JSON
-    let result = core::forge::install_forge(&game_dir, &game_version, &forge_version)
-        .await
-        .map_err(|e| e.to_string())?;
+    // Check if the version JSON already exists
+    let version_id = core::forge::generate_version_id(&game_version, &forge_version);
+    let json_path = game_dir.join("versions").join(&version_id).join(format!("{}.json", version_id));
+
+    let result = if json_path.exists() {
+        // Version JSON was created by the installer, load it
+        emit_log!(
+            window,
+            "Using version profile created by Forge installer".to_string()
+        );
+        core::forge::InstalledForgeVersion {
+            id: version_id,
+            minecraft_version: game_version.clone(),
+            forge_version: forge_version.clone(),
+            path: json_path,
+        }
+    } else {
+        // Installer didn't create JSON, create it manually
+        core::forge::install_forge(&game_dir, &game_version, &forge_version)
+            .await
+            .map_err(|e| e.to_string())?
+    };
 
     emit_log!(
         window,