make some changes after testing some mod loaders

- fabric
- forge
- neoforge

5 files changed, 119 insertions, 68 deletions

diff --git a/src/launcher.rs b/src/launcher.rs
index 7971c3f..5e3668b 100644
--- a/src/launcher.rs
+++ b/src/launcher.rs
@@ -23,6 +23,7 @@ use std::env::JoinPathsError;
 use std::time::{Instant, SystemTime, UNIX_EPOCH};
 use const_format::formatcp;
 use futures::{StreamExt, TryStreamExt};
+use indexmap::IndexMap;
 use log::{debug, info, trace, warn};
 use reqwest::Client;
 use sysinfo::System;
@@ -31,7 +32,7 @@ use tokio_stream::wrappers::ReadDirStream;
 use download::{MultiDownloader, VerifiedDownload};
 use rules::{CompatCheck, IncompatibleError};
 use version::{VersionList, VersionResolveError, VersionResult};
-use crate::version::{Logging, Library, OSRestriction, OperatingSystem, DownloadType, DownloadInfo, LibraryExtractRule, CompleteVersion, FeatureMatcher};
+use crate::version::{Logging, Library, OSRestriction, OperatingSystem, DownloadType, DownloadInfo, LibraryExtractRule, CompleteVersion, FeatureMatcher, ClientLogging};
 
 use assets::{AssetError, AssetRepository};
 use crate::util::{self, AsJavaPath};
@@ -42,6 +43,7 @@ pub use crate::util::{EnsureFileError, FileVerifyError, IntegrityError};
 use crate::assets::AssetIndex;
 use runner::ArgumentType;
 use strsub::SubFunc;
+use crate::launcher::download::FileDownload;
 use crate::launcher::jre::{JavaRuntimeError, JavaRuntimeRepository};
 use crate::version::manifest::VersionType;
 
@@ -272,14 +274,14 @@ impl Launcher {
         lib.natives.as_ref().map_or(None, |n| n.get(&self.system_info.os)).map(|s| s.as_str())
     }
 
-    async fn log_config_ensure(&self, config: &Logging) -> Result<String, LaunchError> {
+    async fn log_config_ensure(&self, config: &ClientLogging) -> Result<String, LaunchError> {
         info!("Ensuring log configuration exists and is valid.");
-        
-        if config.client.log_type != "log4j2-xml" {
-            return Err(LaunchError::UnknownLogType(config.client.log_type.clone()));
+
+        if config.log_type != "log4j2-xml" {
+            return Err(LaunchError::UnknownLogType(config.log_type.clone()));
         }
 
-        let dlinfo = &config.client.file;
+        let dlinfo = &config.file;
         let Some(id) = dlinfo.id.as_ref() else {
             return Err(LaunchError::InvalidLogId(None));
         };
@@ -309,7 +311,7 @@ impl Launcher {
             }
         }
 
-        Ok(strsub::replace_string(config.client.argument.as_str(), &PathSub(path.as_ref())).to_string())
+        Ok(strsub::replace_string(config.argument.as_str(), &PathSub(path.as_ref())).to_string())
     }
 
     pub async fn prepare_launch(&self, profile: &Profile, instance: &Instance) -> Result<Launch, LaunchError> {
@@ -377,7 +379,7 @@ impl Launcher {
 
         let mut libs = Vec::new();
         let mut extract_jobs = Vec::new();
-        let mut downloads = Vec::new();
+        let mut downloads = IndexMap::new();
 
         for lib in ver.libraries.values() {
             if lib.rules_apply(&self.system_info, &feature_matcher).is_err() {
@@ -386,6 +388,9 @@ impl Launcher {
 
             libs.push(lib);
             if let Some(dl) = self.libraries.create_download(lib, self.choose_lib_classifier(lib)) {
+                let canon_name = lib.get_canonical_name();
+                if downloads.contains_key(&canon_name) { continue; }
+
                 dl.make_dirs().await.map_err(|e| LaunchError::LibraryDirError(dl.get_path().to_path_buf(), e))?;
 
                 if lib.natives.is_some() {
@@ -395,21 +400,21 @@ impl Launcher {
                     });
                 }
 
-                downloads.push(dl);
+                downloads.insert(canon_name, dl);
             }
         }
 
         if self.online {
             info!("Downloading {} libraries...", downloads.len());
             let client = Client::new();
-            MultiDownloader::new(downloads.iter_mut()).perform(&client).await
+            MultiDownloader::new(downloads.values_mut()).perform(&client).await
                 .inspect_err(|e| warn!("library download failed: {e}"))
                 .try_fold((), |_, _| async {Ok(())})
                 .await
                 .map_err(|_| LaunchError::LibraryDownloadError)?;
         } else {
             info!("Verifying {} libraries...", downloads.len());
-            download::verify_files(downloads.iter_mut()).await.map_err(|e| {
+            download::verify_files(downloads.values_mut()).await.map_err(|e| {
                 warn!("A library could not be verified: {}", e);
                 warn!("Since the launcher is in offline mode, libraries cannot be downloaded. Please try again in online mode.");
                 LaunchError::LibraryVerifyError(e)
@@ -417,7 +422,7 @@ impl Launcher {
         }
 
         let log_arg;
-        if let Some(logging) = ver.logging.as_ref() {
+        if let Some(logging) = ver.logging.as_ref().map_or(None, |l| l.client.as_ref()) {
             log_arg = Some(self.log_config_ensure(logging).await?);
         } else {
             log_arg = None;
@@ -476,7 +481,7 @@ impl Launcher {
             };
 
         info!("Building classpath");
-        let classpath = env::join_paths(downloads.iter()
+        let classpath = env::join_paths(downloads.values()
                 .map(|job| job.get_path().as_java_path())
                 .chain(client_jar_path.iter().map(|p| p.as_path().as_java_path())))
             .map_err(|e| LaunchError::LibraryClasspathError(e))?
@@ -627,17 +632,21 @@ impl LibraryRepository {
     }
     
     fn create_download(&self, lib: &Library, classifier: Option<&str>) -> Option<VerifiedDownload> {
-        if lib.url.is_some() || lib.downloads.is_none() {
-            // TODO: derive download URL in this situation?
-            warn!("BUG: Deprecated case for library {}: url present or downloads missing. The launcher does not support out-of-line checksums at this time. Not downloading this library.", lib.name);
-            return None;
+        if let Some(ref url) = lib.url {
+            let path = Self::get_artifact_path(lib.name.as_str(), classifier)?;
+            let url = [url.as_str(), path.to_string_lossy().as_ref()].into_iter().collect::<String>();
+            Some(VerifiedDownload::new(url.as_ref(), self.home.join(path).as_path(), lib.size, lib.sha1)) // TODO: could download sha1
+        } else if let Some(ref downloads) = lib.downloads {
+            let dlinfo = downloads.get_download_info(classifier)?;
+            // drinking game: take a shot once per heap allocation
+            let path = self.home.join(dlinfo.path.as_ref().map(PathBuf::from).or_else(|| Self::get_artifact_path(lib.name.as_str(), classifier))?);
+
+            Some(VerifiedDownload::new(dlinfo.url.as_ref()?, path.as_path(), dlinfo.size, dlinfo.sha1))
+        } else {
+            let path = Self::get_artifact_path(lib.name.as_str(), classifier)?;
+            let url = ["https://libraries.minecraft.net/", path.to_string_lossy().as_ref()].into_iter().collect::<String>();
+            Some(VerifiedDownload::new(url.as_ref(), self.home.join(path).as_path(), lib.size, lib.sha1)) // TODO: could download sha1
         }
-
-        let dlinfo = lib.downloads.as_ref()?.get_download_info(classifier)?;
-        // drinking game: take a shot once per heap allocation
-        let path = self.home.join(dlinfo.path.as_ref().map(PathBuf::from).or_else(|| Self::get_artifact_path(lib.name.as_str(), classifier))?);
-
-        Some(VerifiedDownload::new(dlinfo.url.as_ref()?, path.as_path(), dlinfo.size, dlinfo.sha1))
     }
 
     async fn clean_old_natives(&self) -> Result<usize, LaunchError> {
diff --git a/src/launcher/download.rs b/src/launcher/download.rs
index 846bed1..ec4a59c 100644
--- a/src/launcher/download.rs
+++ b/src/launcher/download.rs
@@ -1,10 +1,9 @@
 use std::error::Error;
 use std::fmt::{Debug, Display, Formatter};
-use std::io::ErrorKind;
 use std::path::{Path, PathBuf};
 use futures::{stream, StreamExt, TryStream, TryStreamExt};
-use log::{debug, warn};
-use reqwest::{Client, IntoUrl, Method, RequestBuilder};
+use log::debug;
+use reqwest::{Client, Method, RequestBuilder};
 use sha1_smol::{Digest, Sha1};
 use tokio::fs;
 use tokio::fs::File;
@@ -15,13 +14,15 @@ use crate::util::{FileVerifyError, IntegrityError};
 
 pub trait Download: Debug + Display {
     // return Ok(None) to skip downloading this file
-    fn get_url(&self) -> impl IntoUrl;
-
-    async fn prepare(&mut self, req: RequestBuilder) -> Result<Option<RequestBuilder>, Box<dyn Error>>;
+    async fn prepare(&mut self, client: &Client) -> Result<Option<RequestBuilder>, Box<dyn Error>>;
     async fn handle_chunk(&mut self, chunk: &[u8]) -> Result<(), Box<dyn Error>>;
     async fn finish(&mut self) -> Result<(), Box<dyn Error>>;
 }
 
+pub trait FileDownload: Download {
+    fn get_path(&self) -> &Path;
+}
+
 pub struct MultiDownloader<'j, T: Download + 'j, I: Iterator<Item = &'j mut T>> {
     jobs: I,
     nconcurrent: usize
@@ -110,12 +111,12 @@ impl<'j, T: Download + 'j, I: Iterator<Item = &'j mut T>> MultiDownloader<'j, T,
                 }
             }
 
-            let Some(rq) = map_err!(
-            job.prepare(client.request(Method::GET, job.get_url())
-                .header(reqwest::header::USER_AGENT, USER_AGENT)).await, Phase::Prepare, job) else {
+            let Some(rq) = map_err!(job.prepare(client).await, Phase::Prepare, job) else {
                 return Ok(())
             };
 
+            let rq = rq.header(reqwest::header::USER_AGENT, USER_AGENT);
+
             let mut data = map_err!(map_err!(rq.send().await, Phase::Send, job).error_for_status(), Phase::Send, job).bytes_stream();
 
             while let Some(bytes) = data.next().await {
@@ -187,10 +188,6 @@ impl VerifiedDownload {
         &self.url
     }
     
-    pub fn get_path(&self) -> &Path {
-        &self.path
-    }
-    
     pub fn get_expect_size(&self) -> Option<usize> {
         self.expect_size
     }
@@ -210,11 +207,7 @@ impl VerifiedDownload {
 }
 
 impl Download for VerifiedDownload {
-    fn get_url(&self) -> impl IntoUrl {
-        &self.url
-    }
-
-    async fn prepare(&mut self, req: RequestBuilder) -> Result<Option<RequestBuilder>, Box<dyn Error>> {
+    async fn prepare(&mut self, client: &Client) -> Result<Option<RequestBuilder>, Box<dyn Error>> {
         if !util::should_download(&self.path, self.expect_size, self.expect_sha1).await? {
             return Ok(None)
         }
@@ -222,7 +215,7 @@ impl Download for VerifiedDownload {
         // potentially racy to close the file and reopen it... :/
         self.open_output().await?;
 
-        Ok(Some(req))
+        Ok(Some(client.request(Method::GET, &self.url)))
     }
 
     async fn handle_chunk(&mut self, chunk: &[u8]) -> Result<(), Box<dyn Error>> {
@@ -257,6 +250,12 @@ impl Download for VerifiedDownload {
     }
 }
 
+impl FileDownload for VerifiedDownload {
+    fn get_path(&self) -> &Path {
+        &self.path
+    }
+}
+
 pub async fn verify_files(files: impl Iterator<Item = &mut VerifiedDownload>) -> Result<(), FileVerifyError> {
     stream::iter(files)
         .map(|dl| Ok(async move {
diff --git a/src/launcher/jre/download.rs b/src/launcher/jre/download.rs
index d8631aa..c24b82f 100644
--- a/src/launcher/jre/download.rs
+++ b/src/launcher/jre/download.rs
@@ -6,7 +6,7 @@ use std::ops::AddAssign;
 use std::path::{Path, PathBuf};
 use log::debug;
 use lzma_rs::decompress;
-use reqwest::{IntoUrl, RequestBuilder};
+use reqwest::{Client, IntoUrl, RequestBuilder};
 use sha1_smol::{Digest, Sha1};
 use tokio::fs;
 use tokio::io::AsyncWriteExt;
@@ -31,9 +31,6 @@ pub struct LzmaDownloadJob {
     raw_size: Option<usize>,
     raw_sha1: Option<Digest>,
 
-    com_size: Option<usize>,
-    com_sha1: Option<Digest>,
-
     raw_sha1_st: Sha1,
     raw_tally: usize,
 
@@ -52,9 +49,6 @@ impl LzmaDownloadJob {
             raw_size: raw.size,
             raw_sha1: raw.sha1,
 
-            com_size: lzma.size,
-            com_sha1: lzma.sha1,
-
             raw_sha1_st: Sha1::new(),
             raw_tally: 0,
 
@@ -73,9 +67,6 @@ impl LzmaDownloadJob {
             raw_size: raw.size,
             raw_sha1: raw.sha1,
 
-            com_size: None,
-            com_sha1: None,
-
             raw_sha1_st: Sha1::new(),
             raw_tally: 0,
 
@@ -125,11 +116,7 @@ impl Display for LzmaDownloadJob {
 }
 
 impl Download for LzmaDownloadJob {
-    fn get_url(&self) -> impl IntoUrl {
-        self.url.as_str()
-    }
-
-    async fn prepare(&mut self, req: RequestBuilder) -> Result<Option<RequestBuilder>, Box<dyn Error>> {
+    async fn prepare(&mut self, client: &Client) -> Result<Option<RequestBuilder>, Box<dyn Error>> {
         if !util::should_download(&self.path, self.raw_size, self.raw_sha1).await? {
             return Ok(None)
         }
@@ -147,7 +134,7 @@ impl Download for LzmaDownloadJob {
         let file = options.create(true).write(true).truncate(true).open(&self.path).await?;
         self.out_file = Some(file);
 
-        Ok(Some(req))
+        Ok(Some(client.get(&self.url)))
     }
 
     async fn handle_chunk(&mut self, chunk: &[u8]) -> Result<(), Box<dyn Error>> {
diff --git a/src/launcher/runner.rs b/src/launcher/runner.rs
index 50a9ff8..a58602e 100644
--- a/src/launcher/runner.rs
+++ b/src/launcher/runner.rs
@@ -14,6 +14,13 @@ use super::{Launch, LaunchInfo};
 #[derive(Clone, Copy)]
 struct LaunchArgSub<'a, 'l, F: FeatureMatcher>(&'a LaunchInfo<'l, F>);
 
+// FIXME: this is not correct
+#[cfg(windows)]
+const PATH_SEP: &str = ";";
+
+#[cfg(not(windows))]
+const PATH_SEP: &str = ":";
+
 impl<'rep, 'l, F: FeatureMatcher> SubFunc<'rep> for LaunchArgSub<'rep, 'l, F> {
     fn substitute(&self, key: &str) -> Option<Cow<'rep, str>> {
         match key {
@@ -24,8 +31,8 @@ impl<'rep, 'l, F: FeatureMatcher> SubFunc<'rep> for LaunchArgSub<'rep, 'l, F> {
             "auth_session" => Some(Cow::Borrowed("-")), // TODO
             "auth_uuid" => Some(Cow::Borrowed("00000000-0000-0000-0000-000000000000")), // TODO
             "auth_xuid" => Some(Cow::Borrowed("00000000-0000-0000-0000-000000000000")), // TODO
-            "classpath" => Some(Cow::Borrowed(self.0.classpath.as_str())), // TODO
-            "classpath_separator" => None, // FIXME
+            "classpath" => Some(Cow::Borrowed(self.0.classpath.as_str())),
+            "classpath_separator" => Some(Cow::Borrowed(PATH_SEP)),
             "game_assets" => self.0.virtual_assets_path.as_ref()
                 .map(|s| s.as_path().as_java_path().to_string_lossy()),
             "game_directory" => Some(self.0.instance_home.as_java_path().to_string_lossy()),
diff --git a/src/version.rs b/src/version.rs
index 6354143..462ba3b 100644
--- a/src/version.rs
+++ b/src/version.rs
@@ -1,10 +1,12 @@
 use core::fmt;
 use std::{collections::BTreeMap, convert::Infallible, marker::PhantomData, ops::Deref, str::FromStr};
 use std::collections::HashMap;
-use chrono::{DateTime, Utc};
+use chrono::{DateTime, NaiveDateTime, Utc};
+use chrono::format::ParseErrorKind;
+use indexmap::IndexMap;
 use regex::Regex;
 use serde::{de::{self, Visitor}, Deserialize, Deserializer};
-use serde::de::SeqAccess;
+use serde::de::{Error, SeqAccess};
 use sha1_smol::Digest;
 
 pub mod manifest;
@@ -203,7 +205,17 @@ pub struct Library {
     pub extract: Option<LibraryExtractRule>,
     pub natives: Option<BTreeMap<OperatingSystem, String>>,
     pub rules: Option<Vec<CompatibilityRule>>,
-    pub url: Option<String> // old format
+
+    // old format
+    pub url: Option<String>,
+    pub size: Option<usize>,
+    pub sha1: Option<Digest>
+}
+
+impl Library {
+    pub fn get_canonical_name(&self) -> String {
+        canonicalize_library_name(self.name.as_str())
+    }
 }
 
 impl LibraryDownloads {
@@ -227,7 +239,7 @@ pub struct ClientLogging {
 
 #[derive(Deserialize, Debug, Clone)]
 pub struct Logging {
-    pub client: ClientLogging // other fields unknown
+    pub client: Option<ClientLogging> // other fields unknown
 }
 
 #[derive(Deserialize, Debug, Clone)]
@@ -247,7 +259,7 @@ pub struct CompleteVersion {
     pub downloads: BTreeMap<DownloadType, DownloadInfo>,
 
     #[serde(default, deserialize_with = "deserialize_libraries")]
-    pub libraries: HashMap<String, Library>,
+    pub libraries: IndexMap<String, Library>,
 
     pub id: String,
     pub jar: Option<String>, // used as the jar filename if specified? (no longer used officially)
@@ -256,7 +268,10 @@ pub struct CompleteVersion {
 
     pub main_class: Option<String>,
     pub minimum_launcher_version: Option<u32>,
+
+    #[serde(deserialize_with = "deserialize_datetime_lenient")]
     pub release_time: Option<DateTime<Utc>>,
+    #[serde(deserialize_with = "deserialize_datetime_lenient")]
     pub time: Option<DateTime<Utc>>,
 
     #[serde(rename = "type")]
@@ -338,14 +353,48 @@ fn canonicalize_library_name(name: &str) -> String {
         .join(":")
 }
 
-fn deserialize_libraries<'de, D>(deserializer: D) -> Result<HashMap<String, Library>, D::Error>
+fn deserialize_datetime_lenient<'de, D>(deserializer: D) -> Result<Option<DateTime<Utc>>, D::Error>
+where
+    D: Deserializer<'de>
+{
+    struct DateTimeVisitor;
+
+    impl<'de> Visitor<'de> for DateTimeVisitor {
+        type Value = Option<DateTime<Utc>>;
+
+        fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+            formatter.write_str("a valid datetime")
+        }
+
+        fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
+        where
+            E: Error
+        {
+            match value.parse::<DateTime<Utc>>() {
+                Ok(dt) => Ok(Some(dt)),
+                Err(e) if e.kind() == ParseErrorKind::TooShort => {
+                    // this probably just doesn't have an offset for some reason
+                    match value.parse::<NaiveDateTime>() {
+                        Ok(ndt) => Ok(Some(ndt.and_utc())),
+                        Err(e) => Err(Error::custom(e))
+                    }
+                },
+                Err(e) => Err(Error::custom(e))
+            }
+        }
+    }
+
+    deserializer.deserialize_str(DateTimeVisitor)
+}
+
+fn deserialize_libraries<'de, D>(deserializer: D) -> Result<IndexMap<String, Library>, D::Error>
 where
     D: Deserializer<'de>
 {
     struct LibrariesVisitor;
 
     impl<'de> Visitor<'de> for LibrariesVisitor {
-        type Value = HashMap<String, Library>;
+        type Value = IndexMap<String, Library>;
 
         fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
             formatter.write_str("an array of libraries")
@@ -355,7 +404,7 @@ where
         where
             A: SeqAccess<'de>,
         {
-            let mut map = HashMap::new();
+            let mut map = IndexMap::new();
 
             while let Some(lib) = seq.next_element::<Library>()? {
                 //map.insert(canonicalize_library_name(lib.name.as_str()), lib);