refactor out is_saved flag for Things

Boolean flag was used to indicate if a Thing was currently located in the journal or temporarily stored in recent entries. This wasn't ideal because the data saved to journal/recent included the flag, so there was a lot of bespoke logic to ensure (maybe unsuccessfully) that the flag was never accidentally saved to the wrong place. Instead, we wrap the Thing in an ephemeral wrapper called Record, making the saved-ness of the Thing accessible through the return value if needed but without bothering to include it in either the stored data or code that interacts with the data directly without regard for where it came from.
initiative-sh · Sep 9, 2024 · 253af1a · 253af1a
1 parent 2be8082
commit 253af1a
Show file tree

Hide file tree

Showing 13 changed files with 431 additions and 329 deletions.
diff --git a/core/src/app/command/tutorial.rs b/core/src/app/command/tutorial.rs
@@ -4,7 +4,7 @@ use crate::app::{
     CommandMatches, ContextAwareParse, Runnable,
 };
 use crate::reference::{ItemCategory, ReferenceCommand, Spell};
-use crate::storage::{Change, StorageCommand};
+use crate::storage::{Change, Record, StorageCommand};
 use crate::time::TimeCommand;
 use crate::utils::CaseInsensitiveStr;
 use crate::world::npc::{Age, Ethnicity, Gender, NpcData, Species};
@@ -42,7 +42,7 @@ pub enum TutorialCommand {
     },
     ViewingAlternatives {
         inn: ThingRef,
-        npc: ThingRef,
+        npc_name: String,
     },
     EditingCharacters {
         inn: ThingRef,
@@ -167,7 +167,7 @@ impl TutorialCommand {
             Self::GeneratingAlternatives { .. } => output.push_str(include_str!(
                 "../../../../data/tutorial/03-generating-characters.md"
             )),
-            Self::ViewingAlternatives { npc, .. } => {
+            Self::ViewingAlternatives { npc_name, .. } => {
                 let thing_data: ThingData = NpcData {
                     species: Species::Human.into(),
                     ethnicity: Ethnicity::Human.into(),
@@ -185,7 +185,7 @@ impl TutorialCommand {
 
                 output.push_str(&format!(
                     include_str!("../../../../data/tutorial/04-generating-alternatives.md"),
-                    npc_name = npc,
+                    npc_name = npc_name,
                 ));
             }
             Self::EditingCharacters { npc, .. } => {
@@ -507,7 +507,8 @@ impl Runnable for TutorialCommand {
                                 .unwrap()
                                 .trim_start_matches(&[' ', '#'][..]);
 
-                            let inn = app_meta.repository.get_by_name(inn_name).await.unwrap();
+                            let Record { thing: inn, .. } =
+                                app_meta.repository.get_by_name(inn_name).await.unwrap();
 
                             Ok((
                                 ThingRef {
@@ -543,51 +544,56 @@ impl Runnable for TutorialCommand {
                                 .lines()
                                 .find(|s| s.starts_with('#'))
                                 .unwrap()
-                                .trim_start_matches(&[' ', '#'][..]);
+                                .trim_start_matches(&[' ', '#'][..])
+                                .to_string();
 
-                            let npc = app_meta.repository.get_by_name(npc_name).await.unwrap();
-
-                            Ok((
-                                ThingRef {
-                                    name: npc.name().to_string(),
-                                    uuid: npc.uuid,
-                                },
-                                output,
-                            ))
+                            Ok((npc_name, output))
                         }
                         Err(e) => Err(e),
                     };
 
                     match command_output {
-                        Ok((npc, output)) => {
-                            let next = Self::ViewingAlternatives { inn, npc };
+                        Ok((npc_name, output)) => {
+                            let next = Self::ViewingAlternatives { inn, npc_name };
                             (next.output(Some(Ok(output)), app_meta), Some(next))
                         }
                         Err(e) => (Err(e), Some(Self::GeneratingAlternatives { inn })),
                     }
                 }
-                Self::ViewingAlternatives { inn, npc } => {
+                Self::ViewingAlternatives { inn, npc_name } => {
                     let command_output = input_command.run(input, app_meta).await;
 
                     if let Ok(output) = command_output {
-                        if let Some(npc_name) = output
+                        if let Some(new_npc_name) = output
                             .lines()
                             .find(|s| s.starts_with("~2~"))
                             .and_then(|s| s.find('(').map(|i| (i, s)))
                             .map(|(i, s)| s[10..i - 2].to_string())
                         {
+                            let Record { thing: new_npc, .. } = app_meta
+                                .repository
+                                .get_by_name(&new_npc_name)
+                                .await
+                                .unwrap();
+
                             let new_npc = ThingRef {
-                                name: npc_name,
-                                uuid: npc.uuid,
+                                name: new_npc_name,
+                                uuid: new_npc.uuid,
                             };
                             let next = Self::EditingCharacters { npc: new_npc, inn };
 
                             (next.output(Some(Ok(output)), app_meta), Some(next))
                         } else {
-                            (Ok(output), Some(Self::ViewingAlternatives { inn, npc }))
+                            (
+                                Ok(output),
+                                Some(Self::ViewingAlternatives { inn, npc_name }),
+                            )
                         }
                     } else {
-                        (command_output, Some(Self::ViewingAlternatives { inn, npc }))
+                        (
+                            command_output,
+                            Some(Self::ViewingAlternatives { inn, npc_name }),
+                        )
                     }
                 }
                 Self::EditingCharacters { inn, npc } => {

diff --git a/core/src/storage/command.rs b/core/src/storage/command.rs
@@ -1,5 +1,5 @@
 use super::backup::export;
-use super::{Change, RepositoryError};
+use super::{Change, Record, RecordStatus, RepositoryError};
 use crate::app::{
     AppMeta, Autocomplete, AutocompleteSuggestion, CommandAlias, CommandMatches, ContextAwareParse,
     Event, Runnable,
@@ -81,7 +81,7 @@ impl Runnable for StorageCommand {
             }
             Self::Delete { name } => {
                 let result = match app_meta.repository.get_by_name(&name).await {
-                    Ok(thing) => {
+                    Ok(Record { thing, .. }) => {
                         app_meta
                                 .repository
                                 .modify(Change::Delete { uuid: thing.uuid, name: thing.name().to_string() })
@@ -92,7 +92,7 @@ impl Runnable for StorageCommand {
                 };
 
                 match result {
-                    Ok(Some(thing)) => Ok(format!("{} was successfully deleted. Use `undo` to reverse this.", thing.name())),
+                    Ok(Some(Record { thing, .. })) => Ok(format!("{} was successfully deleted. Use `undo` to reverse this.", thing.name())),
                     Ok(None) | Err(RepositoryError::NotFound) => Err(format!("There is no entity named \"{}\".", name)),
                     Err(_) => Err(format!("Couldn't delete `{}`.", name)),
                 }
@@ -120,10 +120,10 @@ impl Runnable for StorageCommand {
                 Ok("The file upload popup should appear momentarily. Please select a compatible JSON file, such as that produced by the `export` command.".to_string())
             }
             Self::Load { name } => {
-                let thing = app_meta.repository.get_by_name(&name).await;
+                let record = app_meta.repository.get_by_name(&name).await;
                 let mut save_command = None;
-                let output = if let Ok(thing) = thing {
-                    if !thing.is_saved {
+                let output = if let Ok(Record { thing, status }) = record {
+                    if status == RecordStatus::Unsaved {
                         save_command = Some(CommandAlias::literal(
                             "save",
                             format!("save {}", name),
@@ -150,35 +150,34 @@ impl Runnable for StorageCommand {
                 output
             }
             Self::Redo => match app_meta.repository.redo().await {
-                Some(Ok(thing)) => {
+                Some(Ok(option_record)) => {
                     let action = app_meta
                         .repository
                         .undo_history()
                         .next()
                         .unwrap()
                         .display_undo();
 
-                    if let Some(thing) = thing {
-                        Ok(format!(
+                    match option_record {
+                        Some(Record { thing, status }) if status != RecordStatus::Deleted => Ok(format!(
                             "{}\n\n_Successfully redid {}. Use `undo` to reverse this._",
                             thing.display_details(app_meta.repository.load_relations(&thing).await.unwrap_or_default()),
                             action,
-                        ))
-                    } else {
-                        Ok(format!(
+                        )),
+                        _ => Ok(format!(
                             "Successfully redid {}. Use `undo` to reverse this.",
                             action,
-                        ))
+                        )),
                     }
                 }
                 Some(Err(_)) => Err("Failed to redo.".to_string()),
                 None => Err("Nothing to redo.".to_string()),
             },
             Self::Undo => match app_meta.repository.undo().await {
-                Some(Ok(thing)) => {
+                Some(Ok(option_record)) => {
                     let action = app_meta.repository.get_redo().unwrap().display_redo();
 
-                    if let Some(thing) = thing {
+                    if let Some(Record { thing, .. }) = option_record {
                         Ok(format!(
                             "{}\n\n_Successfully undid {}. Use `redo` to reverse this._",
                             thing.display_details(app_meta.repository.load_relations(&thing).await.unwrap_or_default()),
@@ -305,7 +304,7 @@ impl Autocomplete for StorageCommand {
             )
         };
 
-        for (thing, prefix) in full_matches
+        for (record, prefix) in full_matches
             .unwrap_or_default()
             .iter()
             .zip(repeat(""))
@@ -316,10 +315,14 @@ impl Autocomplete for StorageCommand {
                     .zip(repeat(prefix)),
             )
         {
-            if (prefix == "save " && thing.is_saved) || (prefix == "delete " && !thing.is_saved) {
+            if (prefix == "save " && record.is_saved())
+                || (prefix == "delete " && record.is_unsaved())
+            {
                 continue;
             }
 
+            let thing = &record.thing;
+
             let suggestion_term = format!("{}{}", prefix, thing.name());
             let matches = Self::parse_input(&suggestion_term, app_meta).await;
 
@@ -330,7 +333,7 @@ impl Autocomplete for StorageCommand {
                         Self::Delete { .. } => format!("remove {} from journal", thing.as_str()),
                         Self::Save { .. } => format!("save {} to journal", thing.as_str()),
                         Self::Load { .. } => {
-                            if thing.is_saved {
+                            if record.is_saved() {
                                 format!("{}", thing.display_description())
                             } else {
                                 format!("{} (unsaved)", thing.display_description())

diff --git a/core/src/storage/data_store.rs b/core/src/storage/data_store.rs
@@ -227,7 +227,6 @@ mod test {
 
         let gandalf_the_grey = Npc {
             uuid: TEST_UUID,
-            is_saved: true,
             data: NpcData {
                 name: "Gandalf the Grey".into(),
                 ..Default::default()
@@ -252,7 +251,6 @@ mod test {
                 ds.save_thing(
                     &Npc {
                         uuid: Uuid::new_v4(),
-                        is_saved: true,
                         data: NpcData {
                             name: name.into(),
                             ..Default::default()
@@ -291,7 +289,6 @@ mod test {
 
         let gandalf_the_grey = Npc {
             uuid: TEST_UUID,
-            is_saved: true,
             data: NpcData {
                 name: "Gandalf the Grey".into(),
                 ..Default::default()
@@ -300,7 +297,6 @@ mod test {
 
         let gandalf_the_white = Npc {
             uuid: TEST_UUID,
-            is_saved: true,
             data: NpcData {
                 name: "Gandalf the White".into(),
                 ..Default::default()
@@ -387,7 +383,6 @@ mod test {
     fn person(uuid: Uuid) -> Thing {
         Npc {
             uuid,
-            is_saved: true,
             data: NpcData::default(),
         }
         .into()
@@ -396,7 +391,6 @@ mod test {
     fn place(uuid: Uuid) -> Thing {
         Place {
             uuid,
-            is_saved: true,
             data: PlaceData::default(),
         }
         .into()

diff --git a/core/src/storage/mod.rs b/core/src/storage/mod.rs
@@ -2,7 +2,9 @@ pub mod backup;
 
 pub use command::StorageCommand;
 pub use data_store::{DataStore, MemoryDataStore, NullDataStore};
-pub use repository::{Change, Error as RepositoryError, KeyValue, Repository};
+pub use repository::{
+    Change, Error as RepositoryError, KeyValue, Record, RecordStatus, Repository,
+};
 
 mod command;
 mod data_store;