From 51608f7d94bd06685979ef5744f527e81f57f2c8 Mon Sep 17 00:00:00 2001 From: Julian Chang Date: Thu, 1 Oct 2026 00:47:15 -0400 Subject: [PATCH] feat: don't re-provision clusters the user deleted --- .../src/view/app/cluster/cluster_settings.rs | 27 ++++-- .../src/view/app/clusters/delete_instance.rs | 11 ++- packages/oneclient_cluster/src/manager.rs | 15 ++++ .../oneclient_core/src/clusters/migrate.rs | 82 +++++++++++++++++++ .../20260930120000_dismissed_provisions.sql | 5 ++ packages/oneclient_db/src/dao/cluster.rs | 41 ++++++++++ 6 files changed, 171 insertions(+), 10 deletions(-) create mode 100644 packages/oneclient_db/migrations/20260930120000_dismissed_provisions.sql diff --git a/packages/oneclient_app/src/view/app/cluster/cluster_settings.rs b/packages/oneclient_app/src/view/app/cluster/cluster_settings.rs index 75b7d6ca..aeec0da7 100644 --- a/packages/oneclient_app/src/view/app/cluster/cluster_settings.rs +++ b/packages/oneclient_app/src/view/app/cluster/cluster_settings.rs @@ -68,8 +68,9 @@ impl Component for ClusterSettings { let versions = loader_versions(&versions_query); let runtimes = java_runtimes(&runtimes_query); - let instance_row = cluster.user_created.then(|| { + let instance_row = { InstanceRow { + editable: cluster.user_created, facts: InstanceFacts { cluster_id, name: cluster.name.clone(), @@ -83,7 +84,7 @@ impl Component for ClusterSettings { }, } .into_element() - }); + }; let mod_loader = !cluster.lacks_mod_loader(); let loader_section: Vec = if mod_loader { @@ -124,7 +125,7 @@ impl Component for ClusterSettings { .scrollbar_gutter(true) .spacing(4.) .child(section_header("INSTANCE")) - .append_children(instance_row) + .child(instance_row) .child( DedicatedDirRow { cluster_id, @@ -311,6 +312,7 @@ impl Component for ToggleRow { #[derive(PartialEq)] struct InstanceRow { + editable: bool, facts: InstanceFacts, } @@ -322,16 +324,24 @@ impl Component for InstanceRow { let name = self.facts.name.clone(); let facts = self.facts.clone(); + let editable = self.editable; + let description = if editable { + "Change this instance's name, description, tags and cover image, or remove it from your list." + } else { + "Remove this version from your list." + }; + let buttons = rect() .horizontal() .spacing(8.) - .child( + .maybe_child(editable.then(|| { Button::new() .small() .secondary() .on_press(move |_| editing.set(true)) - .text("Edit"), - ) + .text("Edit") + .into_element() + })) .child( Button::new() .small() @@ -346,12 +356,11 @@ impl Component for InstanceRow { .child(settings_row( IconType::Pencil01, "Instance Details", - "Change this instance's name, description, tags and cover image, or remove it from your list.", + description, buttons, )) .maybe_child(editing.read().then(|| { - EditInstanceModal::new(facts.clone(), move |()| editing.set(false)) - .into_element() + EditInstanceModal::new(facts.clone(), move |()| editing.set(false)).into_element() })) .maybe_child(deleting.read().then(|| { DeleteInstanceModal::new(cluster_id, name.clone(), move |()| deleting.set(false)) diff --git a/packages/oneclient_app/src/view/app/clusters/delete_instance.rs b/packages/oneclient_app/src/view/app/clusters/delete_instance.rs index ecbecef3..855efde9 100644 --- a/packages/oneclient_app/src/view/app/clusters/delete_instance.rs +++ b/packages/oneclient_app/src/view/app/clusters/delete_instance.rs @@ -35,7 +35,9 @@ impl Component for DeleteInstanceModal { let game = use_game_snapshot(); let cluster_id = self.cluster_id; let running = game.is_running(cluster_id); - let dedicated = use_cluster(cluster_id).is_none_or(|c| c.uses_dedicated_dir()); + let cluster = use_cluster(cluster_id); + let dedicated = cluster.as_ref().is_none_or(|c| c.uses_dedicated_dir()); + let provisioned = cluster.as_ref().is_some_and(|c| !c.user_created); let warning = if dedicated { format!( "{} and its files will be deleted, including its worlds, settings and installed content. This cannot be undone.", @@ -82,6 +84,13 @@ impl Component for DeleteInstanceModal { .font_size(13.) .color(colors::fg_secondary()), ) + .maybe_child(provisioned.then(|| { + label() + .text("It won't be added back automatically. To play this version again, create a OneClient instance for it.") + .font_size(12.) + .color(colors::fg_secondary()) + .into_element() + })) .maybe_child(running.then(|| { label() .text("Close the game before deleting this instance.") diff --git a/packages/oneclient_cluster/src/manager.rs b/packages/oneclient_cluster/src/manager.rs index e63f1874..0b43f6ff 100644 --- a/packages/oneclient_cluster/src/manager.rs +++ b/packages/oneclient_cluster/src/manager.rs @@ -115,6 +115,12 @@ impl ClusterManager { ) .await? .is_some() + || cluster_dao::is_provision_dismissed( + &self.db, + &options.mc_version, + options.mc_loader as i64, + ) + .await? { return Ok(None); } @@ -388,6 +394,15 @@ impl ClusterManager { pub async fn delete(&self, cluster_id: ClusterId, remove_files: bool) -> ClusterResult<()> { let cluster = self.get(cluster_id).await?; + if !cluster.user_created { + cluster_dao::dismiss_provision( + &self.db, + &cluster.mc_version, + cluster.mc_loader as i64, + ) + .await?; + } + let trashed = if remove_files && cluster.is_isolated() { move_to_trash(&cluster).await } else { diff --git a/packages/oneclient_core/src/clusters/migrate.rs b/packages/oneclient_core/src/clusters/migrate.rs index e3829941..b0c7a4f0 100644 --- a/packages/oneclient_core/src/clusters/migrate.rs +++ b/packages/oneclient_core/src/clusters/migrate.rs @@ -95,6 +95,16 @@ async fn apply_rule(state: &LauncherState, rule: &RemoteMigration) -> LauncherRe return Ok(false); }; + if cluster_dao::is_provision_dismissed(db, &to.mc_version, to.loader as i64).await? { + tracing::info!( + migration_id = %rule.id, + to = %to.mc_version, + to_loader = %to.loader, + "target version was dismissed; skipping migration" + ); + return Ok(false); + } + if cluster_dao::find_by_version_loader(db, &to.mc_version, to.loader as i64) .await? .is_some() @@ -432,6 +442,78 @@ mod tests { use super::*; use oneclient_common::domain::GameLoader; + #[cfg(debug_assertions)] + #[tokio::test] + async fn remote_migration_does_not_restore_a_deleted_target() { + use crate::clusters::CreateClusterOptions; + use crate::versions::{MigrationSource, MigrationTarget}; + + let state = crate::dev::ephemeral_state().await.unwrap(); + let global = state.settings.read().global_game_settings.clone(); + let mut clusters = Vec::new(); + for version in ["26.1", "26.1.2"] { + clusters.push( + state + .clusters + .create_provisioned( + &global, + CreateClusterOptions::new( + format!("{version} Fabric"), + version, + GameLoader::Fabric, + ), + ) + .await + .unwrap() + .unwrap(), + ); + } + let source = &clusters[0]; + let target = &clusters[1]; + let marker = source.dir().unwrap().join("keep.txt"); + polyio::write(&marker, b"keep").await.unwrap(); + state.clusters.delete(target.id, true).await.unwrap(); + + let mut rule = RemoteMigration { + id: "dismissed-target".to_string(), + from: MigrationSource { + mc_version: source.mc_version.clone(), + loader: "fabric".to_string(), + }, + to: MigrationTarget { + mc_version: target.mc_version.clone(), + loader: None, + }, + allow_without_bundles: false, + }; + + for loader in [None, Some("fabric".to_string())] { + rule.to.loader = loader; + assert!(!apply_rule(&state, &rule).await.unwrap()); + let unchanged = state.clusters.get(source.id).await.unwrap(); + assert_eq!(unchanged.mc_version, source.mc_version); + assert_eq!(unchanged.folder_name, source.folder_name); + assert_eq!(polyio::read(&marker).await.unwrap(), b"keep"); + assert!( + cluster_dao::find_by_version_loader( + &state.services.db, + &target.mc_version, + target.mc_loader as i64, + ) + .await + .unwrap() + .is_none() + ); + } + + rule.to.mc_version = "26.1.3".to_string(); + assert!(apply_rule(&state, &rule).await.unwrap()); + assert_eq!( + state.clusters.get(source.id).await.unwrap().mc_version, + "26.1.3" + ); + } + #[test] fn retargets_generated_folder() { assert_eq!( diff --git a/packages/oneclient_db/migrations/20260930120000_dismissed_provisions.sql b/packages/oneclient_db/migrations/20260930120000_dismissed_provisions.sql new file mode 100644 index 00000000..7ff89feb --- /dev/null +++ b/packages/oneclient_db/migrations/20260930120000_dismissed_provisions.sql @@ -0,0 +1,5 @@ +CREATE TABLE dismissed_provisions ( + mc_version TEXT NOT NULL, + mc_loader INTEGER NOT NULL, + PRIMARY KEY (mc_version, mc_loader) +); diff --git a/packages/oneclient_db/src/dao/cluster.rs b/packages/oneclient_db/src/dao/cluster.rs index cae774d6..5ef3f74c 100644 --- a/packages/oneclient_db/src/dao/cluster.rs +++ b/packages/oneclient_db/src/dao/cluster.rs @@ -86,6 +86,34 @@ pub async fn find_by_version_loader( .await } +pub async fn dismiss_provision( + pool: &SqlitePool, + mc_version: &str, + mc_loader: i64, +) -> Result<(), sqlx::Error> { + sqlx::query("INSERT OR IGNORE INTO dismissed_provisions (mc_version, mc_loader) VALUES (?, ?)") + .bind(mc_version) + .bind(mc_loader) + .execute(pool) + .await?; + Ok(()) +} + +pub async fn is_provision_dismissed( + pool: &SqlitePool, + mc_version: &str, + mc_loader: i64, +) -> Result { + sqlx::query_scalar::<_, i64>( + "SELECT 1 FROM dismissed_provisions WHERE mc_version = ? AND mc_loader = ?", + ) + .bind(mc_version) + .bind(mc_loader) + .fetch_optional(pool) + .await + .map(|row| row.is_some()) +} + pub async fn insert(pool: &SqlitePool, new: &NewCluster<'_>) -> Result { let created_at = Utc::now().to_rfc3339(); @@ -317,6 +345,19 @@ mod tests { .expect("insert cluster") } + #[tokio::test] + async fn dismissed_provision_is_remembered_per_version_and_loader() { + let pool = pool().await; + assert!(!is_provision_dismissed(&pool, "26.1", 1).await.unwrap()); + + dismiss_provision(&pool, "26.1", 1).await.unwrap(); + dismiss_provision(&pool, "26.1", 1).await.unwrap(); + + assert!(is_provision_dismissed(&pool, "26.1", 1).await.unwrap()); + assert!(!is_provision_dismissed(&pool, "26.1", 2).await.unwrap()); + assert!(!is_provision_dismissed(&pool, "26.2", 1).await.unwrap()); + } + #[tokio::test] async fn migrate_version_rewrites_identity_and_keeps_history() { let pool = pool().await;