fix bundle newer mods being overwritten by local mods

This commit is contained in:
Julian Chang 2026-10-08 03:11:19 -04:00
parent 66b4ab4e64
commit 1b633adcf2
No known key found for this signature in database
11 changed files with 372 additions and 59 deletions

View file

@ -129,6 +129,7 @@ fn entry_from_item(
hidden: false,
opted_in: true,
shadowed: false,
outranked: false,
advanced: false,
update_available: false,
// Offered, not installed, so there is no recency badge to show

View file

@ -66,6 +66,8 @@ pub struct PackageEntry {
/// Only set for browser-installed content bundle packages use the bundle update flow
pub update_available: bool,
pub shadowed: bool,
/// The user's own copy, older than the bundle's the loader picks over it
pub outranked: bool,
/// Recency badge state cleared once the user views the list
pub seen_status: SeenStatus,
pub essential: Option<&'static EssentialPackage>,
@ -470,7 +472,8 @@ pub(crate) fn grid_card(
.into_element()
});
let badged = item.is_outdated() || item.shadowed || item.recency_badge().is_some();
let badged =
item.is_outdated() || item.shadowed || item.outranked || item.recency_badge().is_some();
let floating = badged.then(|| {
rect()
.horizontal()
@ -485,6 +488,7 @@ pub(crate) fn grid_card(
.spacing(4.)
.maybe_child(item.is_outdated().then(outdated_badge))
.maybe_child(item.shadowed.then(shadowed_badge))
.maybe_child(item.outranked.then(outranked_badge))
.maybe_child(item.recency_badge())
.into_element()
});
@ -771,6 +775,7 @@ fn package_info(
})
.maybe_child(item.is_outdated().then(outdated_badge))
.maybe_child(item.shadowed.then(shadowed_badge))
.maybe_child(item.outranked.then(outranked_badge))
.maybe_child(item.recency_badge()),
)
.maybe(!item.author.is_empty(), |el| {
@ -876,6 +881,10 @@ fn shadowed_badge() -> Element {
status_tag("Using your copy", colors::brand())
}
fn outranked_badge() -> Element {
status_tag("Using bundle's newer copy", colors::brand())
}
fn new_badge() -> Element {
status_tag("New", colors::success())
}

View file

@ -7,6 +7,7 @@ use std::sync::Arc;
use oneclient_content::packages::release_migration::has_packages_to_migrate;
use oneclient_content::packages::{ContentType, PackageStore};
use oneclient_core::game::DuplicateMods;
use oneclient_core::{LauncherError, LinkedArtifactInfo};
use oneclient_db::models::ClusterId;
@ -96,7 +97,7 @@ pub fn use_migratable_routes(
pub struct ShadowedModsQuery;
impl QueryCapability for ShadowedModsQuery {
type Ok = Arc<HashSet<String>>;
type Ok = Arc<DuplicateMods>;
type Err = LauncherError;
type Keys = ClusterId;
@ -110,7 +111,7 @@ impl QueryCapability for ShadowedModsQuery {
}
}
pub fn use_shadowed_mods(cluster_id: ClusterId) -> Arc<HashSet<String>> {
pub fn use_shadowed_mods(cluster_id: ClusterId) -> Arc<DuplicateMods> {
let query = use_query(Query::new(cluster_id, ShadowedModsQuery));
super::state::settled_or_loading(&query).unwrap_or_default()
}

View file

@ -47,10 +47,11 @@ impl Component for ClusterMods {
ContentType::Mod,
);
for item in &mut items {
item.shadowed = item
.hash
.as_ref()
.is_some_and(|hash| shadowed.contains(hash));
let Some(hash) = &item.hash else {
continue;
};
item.shadowed = shadowed.shadowed.contains(hash);
item.outranked = shadowed.outranked.contains(hash);
}
cluster_content()

View file

@ -317,6 +317,7 @@ fn make_row(
hidden,
opted_in,
shadowed: false,
outranked: false,
advanced: false,
seen_status: installed_info.map(|i| i.seen_status).unwrap_or_default(),
}

View file

@ -0,0 +1,167 @@
//! Version ordering as Fabric Loader does it, ported from its `VersionParser`,
use std::cmp::Ordering;
struct Semantic<'a> {
components: Vec<i32>,
prerelease: Option<&'a str>,
build: Option<&'a str>,
}
impl Semantic<'_> {
fn friendly(&self) -> String {
let mut out = self
.components
.iter()
.map(ToString::to_string)
.collect::<Vec<_>>()
.join(".");
if let Some(prerelease) = self.prerelease {
out.push('-');
out.push_str(prerelease);
}
if let Some(build) = self.build {
out.push('+');
out.push_str(build);
}
out
}
}
fn parse(version: &str) -> Option<Semantic<'_>> {
let (version, build) = match version.split_once('+') {
Some((version, build)) => (version, Some(build)),
None => (version, None),
};
let (version, prerelease) = match version.split_once('-') {
Some((version, prerelease)) => (version, Some(prerelease)),
None => (version, None),
};
if prerelease.is_some_and(|prerelease| !is_dot_separated_id(prerelease)) {
return None;
}
let components = version
.split('.')
.map(|component| component.parse::<i32>().ok())
.collect::<Option<Vec<_>>>()?;
Some(Semantic {
components,
prerelease,
build,
})
}
fn is_dot_separated_id(text: &str) -> bool {
text.is_empty()
|| text.split('.').all(|part| {
!part.is_empty() && part.bytes().all(|b| b == b'-' || b.is_ascii_alphanumeric())
})
}
fn is_unsigned_integer(text: &str) -> bool {
text == "0"
|| (!text.is_empty() && !text.starts_with('0') && text.bytes().all(|b| b.is_ascii_digit()))
}
fn compare_strings(left: &str, right: &str) -> Ordering {
left.encode_utf16().cmp(right.encode_utf16())
}
fn compare_prereleases(left: &str, right: &str) -> Ordering {
let mut left = left.split('.').filter(|part| !part.is_empty());
let mut right = right.split('.').filter(|part| !part.is_empty());
loop {
let (a, b) = match (left.next(), right.next()) {
(Some(a), Some(b)) => (a, b),
(Some(_), None) => return Ordering::Greater,
(None, Some(_)) => return Ordering::Less,
(None, None) => return Ordering::Equal,
};
let by_kind = match (is_unsigned_integer(a), is_unsigned_integer(b)) {
(true, true) => a.len().cmp(&b.len()),
(true, false) => return Ordering::Less,
(false, true) => return Ordering::Greater,
(false, false) => Ordering::Equal,
};
let ordering = by_kind.then_with(|| compare_strings(a, b));
if ordering != Ordering::Equal {
return ordering;
}
}
}
#[must_use]
pub fn compare(left: &str, right: &str) -> Ordering {
let (left, right) = match (parse(left), parse(right)) {
(Some(left), Some(right)) => (left, right),
(parsed_left, parsed_right) => {
let friendly = |parsed: Option<Semantic<'_>>, raw: &str| {
parsed.map_or_else(|| raw.to_owned(), |semantic| semantic.friendly())
};
return compare_strings(&friendly(parsed_left, left), &friendly(parsed_right, right));
}
};
for index in 0..left.components.len().max(right.components.len()) {
let component =
|semantic: &Semantic<'_>| semantic.components.get(index).copied().unwrap_or(0);
let ordering = component(&left).cmp(&component(&right));
if ordering != Ordering::Equal {
return ordering;
}
}
match (left.prerelease, right.prerelease) {
(Some(left), Some(right)) => compare_prereleases(left, right),
(Some(_), None) => Ordering::Less,
(None, Some(_)) => Ordering::Greater,
(None, None) => Ordering::Equal,
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn versions_order_as_fabric_loader_orders_them() {
use Ordering::{Equal, Greater, Less};
for (left, right, expected) in [
("0.141.4+1.21.11", "0.141.6+1.21.11", Less),
("1.13.12+kotlin.2.4.0", "1.14.1+kotlin.2.4.20", Less),
("0.141.10", "0.141.9", Greater),
("1.0", "1.0.0", Equal),
("1.0.0+a", "1.0.0+b", Equal),
("1.0.0-beta.1", "1.0.0", Less),
("1.0.0-beta.2", "1.0.0-beta.10", Less),
("1.0.0-beta", "1.0.0-beta.1", Less),
("1.0.0-1", "1.0.0-alpha", Less),
(
"0.4.0-alpha.0.27+1.21.11",
"0.4.0-alpha.0.3+1.21.11",
Greater,
),
("1.01", "1.1", Equal),
("1.0.0-", "1.0.0", Less),
("1.0.0-", "1.0.0-0", Less),
("1.0.0-rc", "1.0.0-rc-1", Less),
("1.99999999999", "1.200", Greater),
("1..5", "1.0.5", Less),
("v1.10", "v1.9", Less),
("1.2", "1.2a", Less),
] {
assert_eq!(compare(left, right), expected, "{left} vs {right}");
assert_eq!(
compare(right, left),
expected.reverse(),
"{right} vs {left}"
);
}
}
}

View file

@ -88,8 +88,14 @@ pub async fn read_jar_manifest(jar: &Path) -> JarManifest {
out
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct JarModId {
pub id: String,
pub version: Option<String>,
}
#[tracing::instrument(level = "debug", fields(jar = %jar.display()))]
pub async fn read_jar_mod_id(jar: &Path) -> Option<String> {
pub async fn read_jar_mod_id(jar: &Path) -> Option<JarModId> {
let entries = polyio::read_zip_file_entries(jar, |name| MANIFESTS.contains(&name))
.await
.inspect_err(|err| tracing::debug!("could not read {}: {err}", jar.display()))
@ -97,19 +103,22 @@ pub async fn read_jar_mod_id(jar: &Path) -> Option<String> {
MANIFESTS.into_iter().find_map(|wanted| {
let (_, bytes) = entries.iter().find(|(name, _)| name == wanted)?;
parse_mod_id(wanted, std::str::from_utf8(bytes).ok()?)
parse_mod(wanted, std::str::from_utf8(bytes).ok()?)
})
}
fn parse_mod_id(manifest: &str, text: &str) -> Option<String> {
let id = match manifest {
NEOFORGE | FORGE => toml::from_str::<toml::Value>(text)
.ok()?
.get("mods")?
.get(0)?
.get("modId")?
.as_str()?
.to_owned(),
fn parse_mod(manifest: &str, text: &str) -> Option<JarModId> {
let (id, version) = match manifest {
NEOFORGE | FORGE => {
let id = toml::from_str::<toml::Value>(text)
.ok()?
.get("mods")?
.get(0)?
.get("modId")?
.as_str()?
.to_owned();
(id, None)
}
_ => {
let json: serde_json::Value = serde_json::from_str(text).ok()?;
let pointers: &[&str] = match manifest {
@ -117,14 +126,21 @@ fn parse_mod_id(manifest: &str, text: &str) -> Option<String> {
QUILT => &["/quilt_loader/id"],
_ => &["/0/modid", "/modList/0/modid"],
};
pointers
let id = pointers
.iter()
.find_map(|pointer| json.pointer(pointer)?.as_str())?
.to_owned()
.to_owned();
let version = (manifest == FABRIC)
.then(|| json.pointer("/version")?.as_str().map(str::to_owned))
.flatten();
(id, version)
}
};
clean(Some(id)).filter(|id| !is_placeholder_id(id))
Some(JarModId {
id: clean(Some(id)).filter(|id| !is_placeholder_id(id))?,
version: clean(version).filter(|version| !version.contains('$')),
})
}
fn is_placeholder_id(id: &str) -> bool {
@ -412,6 +428,14 @@ fn entry_path(raw: String) -> Option<String> {
mod tests {
use super::*;
fn parse_mod_id(manifest: &str, text: &str) -> Option<String> {
parse_mod(manifest, text).map(|parsed| parsed.id)
}
fn parse_mod_version(manifest: &str, text: &str) -> Option<String> {
parse_mod(manifest, text)?.version
}
#[test]
fn every_loader_names_its_mod_id() {
assert_eq!(
@ -435,6 +459,24 @@ mod tests {
Some("oneconfig")
);
assert_eq!(parse_mod_id(FABRIC, r#"{"id":" "}"#), None);
assert_eq!(
parse_mod_version(FABRIC, r#"{"id":"fabric-api","version":"0.141.6+1.21.11"}"#)
.as_deref(),
Some("0.141.6+1.21.11")
);
assert_eq!(
parse_mod_version(QUILT, r#"{"quilt_loader":{"id":"qsl","version":"1.2.3"}}"#),
None
);
assert_eq!(
parse_mod_version(FABRIC, r#"{"id":"a","version":"${version}"}"#),
None
);
assert_eq!(
parse_mod_version(FORGE, "[[mods]]\nmodId = \"jei\"\nversion = \"1\"\n"),
None
);
assert_eq!(
parse_mod_id(LEGACY_FORGE, r#"[{"modid":"examplemod"}]"#),
None

View file

@ -2,6 +2,7 @@ pub mod activity;
pub mod bad_mods;
pub mod dependencies;
pub mod error;
pub mod fabric_version;
pub mod local_manifest;
pub mod metadata_cache;
pub mod provider;

View file

@ -39,6 +39,7 @@ pub use process::{
};
pub use reattach::recover_sessions;
pub use shared_dir::{
dematerialize_content, import_manual_content, link_cluster_logs, materialize_content,
shadowed_bundle_mods, sync_cluster_mods, unlink_cluster_logs, write_allowed_symlinks,
DuplicateMods, dematerialize_content, import_manual_content, link_cluster_logs,
materialize_content, shadowed_bundle_mods, sync_cluster_mods, unlink_cluster_logs,
write_allowed_symlinks,
};

View file

@ -11,9 +11,11 @@ use crate::LauncherResult;
use crate::clusters::Cluster;
use crate::state::{LauncherServices, LauncherState};
use oneclient_cluster::remove_mods_link;
use oneclient_common::domain::{ContentType, ProviderId};
use oneclient_common::domain::{ContentType, GameLoader, ProviderId};
use oneclient_common::paths;
use oneclient_content::packages::PackageStore;
use oneclient_content::packages::fabric_version;
use oneclient_content::packages::local_manifest::JarModId;
use oneclient_content::packages::store::manifest::{self, ManifestEntry, MaterializedManifest};
use oneclient_content::packages::store::{
artifact_absolute_path, link_or_copy, remove_entry, sweep_staging_files,
@ -1204,7 +1206,14 @@ async fn skip_shadowed_bundle_mods(
.filter(|item| item.content_type == ContentType::Mod)
.map(|item| (item.hash.clone(), item.src.clone()))
.collect();
let shadowed = shadowed_bundle_hashes(services, cluster.id, &mods).await;
let shadowed = shadowed_bundle_hashes(
services,
cluster.id,
cluster.mc_loader == GameLoader::Fabric,
&mods,
)
.await
.shadowed;
if shadowed.is_empty() {
return desired;
}
@ -1254,11 +1263,11 @@ async fn skip_shadowed_bundle_mods(
desired
}
pub async fn shadowed_bundle_mods(state: &LauncherState, cluster_id: i64) -> HashSet<String> {
pub async fn shadowed_bundle_mods(state: &LauncherState, cluster_id: i64) -> DuplicateMods {
let services = &state.services;
let Ok(linked) = PackageStore::list_linked_artifacts(cluster_id, &services.content()).await
else {
return HashSet::new();
return DuplicateMods::default();
};
let mut mods = Vec::new();
@ -1274,25 +1283,32 @@ pub async fn shadowed_bundle_mods(state: &LauncherState, cluster_id: i64) -> Has
}
}
shadowed_bundle_hashes(services, cluster_id, &mods).await
let loader_picks_newer = cluster_dao::get_by_id(&services.db, cluster_id)
.await
.ok()
.flatten()
.is_some_and(|row| row.mc_loader == GameLoader::Fabric as i64);
shadowed_bundle_hashes(services, cluster_id, loader_picks_newer, &mods).await
}
async fn shadowed_bundle_hashes(
services: &LauncherServices,
cluster_id: i64,
loader_picks_newer: bool,
mods: &[(String, PathBuf)],
) -> HashSet<String> {
) -> DuplicateMods {
let bundled: HashSet<String> = match bundle_dao::list_bundle_tracked(&services.db, cluster_id)
.await
{
Ok(rows) => rows.into_iter().map(|row| row.hash).collect(),
Err(err) => {
tracing::warn!(error = %err, "cannot list bundle mods; not checking for duplicates");
return HashSet::new();
return DuplicateMods::default();
}
};
if bundled.is_empty() {
return HashSet::new();
return DuplicateMods::default();
}
let mut copies = Vec::new();
@ -1310,15 +1326,15 @@ async fn shadowed_bundle_hashes(
);
if !bundle_pass && copies.iter().all(|copy| copy.mod_id.is_none()) {
return HashSet::new();
return DuplicateMods::default();
}
}
shadowed_bundle_copies(&copies)
shadowed_bundle_copies(&copies, loader_picks_newer)
}
async fn cached_mod_id(hash: &str, jar: &Path) -> Option<String> {
static IDS: std::sync::Mutex<std::collections::BTreeMap<String, Option<String>>> =
async fn cached_mod_id(hash: &str, jar: &Path) -> Option<JarModId> {
static IDS: std::sync::Mutex<std::collections::BTreeMap<String, Option<JarModId>>> =
std::sync::Mutex::new(std::collections::BTreeMap::new());
if let Some(id) = IDS.lock().unwrap().get(hash) {
@ -1335,21 +1351,49 @@ async fn cached_mod_id(hash: &str, jar: &Path) -> Option<String> {
struct ModCopy {
hash: String,
bundled: bool,
mod_id: Option<String>,
mod_id: Option<JarModId>,
}
fn shadowed_bundle_copies(mods: &[ModCopy]) -> HashSet<String> {
let users: HashSet<&str> = mods
.iter()
.filter(|copy| !copy.bundled)
.filter_map(|copy| copy.mod_id.as_deref())
.collect();
#[derive(Debug, Default, Clone, PartialEq, Eq)]
pub struct DuplicateMods {
pub shadowed: HashSet<String>,
pub outranked: HashSet<String>,
}
mods.iter()
.filter(|copy| copy.bundled)
.filter(|copy| copy.mod_id.as_deref().is_some_and(|id| users.contains(id)))
.map(|copy| copy.hash.clone())
.collect()
fn shadowed_bundle_copies(mods: &[ModCopy], loader_picks_newer: bool) -> DuplicateMods {
let mut duplicates = DuplicateMods::default();
for copy in mods.iter().filter(|copy| copy.bundled) {
let Some(bundle) = &copy.mod_id else {
continue;
};
let users: Vec<(&str, &JarModId)> = mods
.iter()
.filter(|copy| !copy.bundled)
.filter_map(|copy| Some((copy.hash.as_str(), copy.mod_id.as_ref()?)))
.filter(|(_, user)| user.id == bundle.id)
.collect();
if users.is_empty() {
continue;
}
if loader_picks_newer && users.iter().all(|(_, user)| is_older(user, bundle)) {
duplicates
.outranked
.extend(users.iter().map(|(hash, _)| (*hash).to_owned()));
} else {
duplicates.shadowed.insert(copy.hash.clone());
}
}
duplicates
}
fn is_older(user: &JarModId, bundle: &JarModId) -> bool {
match (&user.version, &bundle.version) {
(Some(user), Some(bundle)) => fabric_version::compare(user, bundle).is_lt(),
_ => false,
}
}
async fn cached_file(
@ -2869,21 +2913,67 @@ mod tests {
#[test]
fn a_bundle_mod_the_user_added_their_own_copy_of_is_skipped() {
let copy = |hash: &str, bundled: bool, mod_id: Option<&str>| ModCopy {
let versioned = |hash: &str, bundled: bool, id: &str, version: Option<&str>| ModCopy {
hash: hash.into(),
bundled,
mod_id: mod_id.map(Into::into),
mod_id: Some(JarModId {
id: id.into(),
version: version.map(Into::into),
}),
};
let copy = |hash: &str, bundled: bool, mod_id: Option<&str>| match mod_id {
Some(id) => versioned(hash, bundled, id, None),
None => ModCopy {
hash: hash.into(),
bundled,
mod_id: None,
},
};
let shadowed = shadowed_bundle_copies(&[
copy("bundle-argentum", true, Some("argentum")),
copy("user-argentum", false, Some("argentum")),
copy("bundle-sodium", true, Some("sodium")),
copy("bundle-unreadable", true, None),
copy("user-unreadable", false, None),
]);
let duplicates = shadowed_bundle_copies(
&[
copy("bundle-argentum", true, Some("argentum")),
copy("user-argentum", false, Some("argentum")),
copy("bundle-sodium", true, Some("sodium")),
copy("bundle-unreadable", true, None),
copy("user-unreadable", false, None),
],
true,
);
assert_eq!(shadowed, HashSet::from(["bundle-argentum".to_owned()]));
assert_eq!(
duplicates.shadowed,
HashSet::from(["bundle-argentum".to_owned()])
);
assert!(duplicates.outranked.is_empty());
let versioned_copies = [
versioned("bundle-api", true, "fabric-api", Some("0.141.6+1.21.11")),
versioned("user-api", false, "fabric-api", Some("0.141.4+1.21.11")),
versioned("bundle-sodium", true, "sodium", Some("0.8.3")),
versioned("user-sodium", false, "sodium", Some("0.8.5")),
versioned("bundle-iris", true, "iris", Some("1.9.0")),
versioned("user-iris", false, "iris", Some("1.9.0")),
];
let duplicates = shadowed_bundle_copies(&versioned_copies, true);
assert_eq!(
duplicates.shadowed,
HashSet::from(["bundle-sodium".to_owned(), "bundle-iris".to_owned()])
);
assert_eq!(duplicates.outranked, HashSet::from(["user-api".to_owned()]));
let duplicates = shadowed_bundle_copies(&versioned_copies, false);
assert_eq!(
duplicates.shadowed,
HashSet::from([
"bundle-api".to_owned(),
"bundle-sodium".to_owned(),
"bundle-iris".to_owned()
])
);
assert!(duplicates.outranked.is_empty());
}
#[test]

View file

@ -82,6 +82,5 @@ pub use tos::{TermsDocument, fetch_terms};
pub use verify::{ClusterVerifyReport, verify_cluster_files};
pub use versions::{
ReleaseTarget, RemoteMigration, VersionArts, VersionMetadata, VersionsManager,
VersionsManifest,
resolve_migration_chain,
VersionsManifest, resolve_migration_chain,
};