Merge branch 'Mckol/552-fix-persistence-crash' into 'master'

Fix #552

Closes #552

See merge request veloren/veloren!984

server/src/events/player.rs

@@ -42,6 +42,7 @@ pub fn handle_exit_ingame(server: &mut Server, entity: EcsEntity) {
 }
 
 pub fn handle_client_disconnect(server: &mut Server, entity: EcsEntity) -> Event {
+    let db_dir = &server.server_settings.persistence_db_dir.clone();
     let state = server.state_mut();
 
     // Tell other clients to remove from player list
@@ -77,7 +78,7 @@ pub fn handle_client_disconnect(server: &mut Server, entity: EcsEntity) -> Event
         state.read_storage::<comp::Stats>().get(entity),
     ) {
         if let Some(character_id) = player.character_id {
-            persistence::stats::update(character_id, stats, None);
+            persistence::stats::update(character_id, stats, None, db_dir);
         }
     }
 

server/src/lib.rs

@@ -117,6 +117,9 @@ impl Server {
         // Server-only components
         state.ecs_mut().register::<RegionSubscription>();
         state.ecs_mut().register::<Client>();
+        state.ecs_mut().insert(crate::settings::PersistenceDBDir(
+            settings.persistence_db_dir.clone(),
+        ));
 
         #[cfg(feature = "worldgen")]
         let world = World::generate(settings.world_seed, WorldOpts {
@@ -236,7 +239,9 @@ impl Server {
         // Run pending DB migrations (if any)
         debug!("Running DB migrations...");
 
-        if let Some(error) = persistence::run_migrations().err() {
+        if let Some(error) =
+            persistence::run_migrations(&this.server_settings.persistence_db_dir).err()
+        {
             log::info!("Migration error: {}", format!("{:#?}", error));
         }
 

server/src/persistence/character.rs

@@ -16,12 +16,12 @@ type CharacterListResult = Result<Vec<CharacterItem>, Error>;
 ///
 /// After first logging in, and after a character is selected, we fetch this
 /// data for the purpose of inserting their persisted data for the entity.
-pub fn load_character_data(character_id: i32) -> Result<comp::Stats, Error> {
+pub fn load_character_data(character_id: i32, db_dir: &str) -> Result<comp::Stats, Error> {
     let (character_data, body_data, stats_data) = schema::character::dsl::character
         .filter(schema::character::id.eq(character_id))
         .inner_join(schema::body::table)
         .inner_join(schema::stats::table)
-        .first::<(Character, Body, Stats)>(&establish_connection())?;
+        .first::<(Character, Body, Stats)>(&establish_connection(db_dir))?;
 
     Ok(comp::Stats::from(StatsJoinData {
         alias: &character_data.alias,
@@ -37,13 +37,13 @@ pub fn load_character_data(character_id: i32) -> Result<comp::Stats, Error> {
 /// In the event that a join fails, for a character (i.e. they lack an entry for
 /// stats, body, etc...) the character is skipped, and no entry will be
 /// returned.
-pub fn load_character_list(player_uuid: &str) -> CharacterListResult {
+pub fn load_character_list(player_uuid: &str, db_dir: &str) -> CharacterListResult {
     let data: Vec<(Character, Body, Stats)> = schema::character::dsl::character
         .filter(schema::character::player_uuid.eq(player_uuid))
         .order(schema::character::id.desc())
         .inner_join(schema::body::table)
         .inner_join(schema::stats::table)
-        .load::<(Character, Body, Stats)>(&establish_connection())?;
+        .load::<(Character, Body, Stats)>(&establish_connection(db_dir))?;
 
     Ok(data
         .iter()
@@ -72,10 +72,11 @@ pub fn create_character(
     character_alias: String,
     character_tool: Option<String>,
     body: &comp::Body,
+    db_dir: &str,
 ) -> CharacterListResult {
-    check_character_limit(uuid)?;
+    check_character_limit(uuid, db_dir)?;
 
-    let connection = establish_connection();
+    let connection = establish_connection(db_dir);
 
     connection.transaction::<_, diesel::result::Error, _>(|| {
         use schema::{body, character, character::dsl::*, stats};
@@ -136,26 +137,26 @@ pub fn create_character(
         Ok(())
     })?;
 
-    load_character_list(uuid)
+    load_character_list(uuid, db_dir)
 }
 
 /// Delete a character. Returns the updated character list.
-pub fn delete_character(uuid: &str, character_id: i32) -> CharacterListResult {
+pub fn delete_character(uuid: &str, character_id: i32, db_dir: &str) -> CharacterListResult {
     use schema::character::dsl::*;
 
-    diesel::delete(character.filter(id.eq(character_id))).execute(&establish_connection())?;
+    diesel::delete(character.filter(id.eq(character_id))).execute(&establish_connection(db_dir))?;
 
-    load_character_list(uuid)
+    load_character_list(uuid, db_dir)
 }
 
-fn check_character_limit(uuid: &str) -> Result<(), Error> {
+fn check_character_limit(uuid: &str, db_dir: &str) -> Result<(), Error> {
     use diesel::dsl::count_star;
     use schema::character::dsl::*;
 
     let character_count = character
         .select(count_star())
         .filter(player_uuid.eq(uuid))
-        .load::<i64>(&establish_connection())?;
+        .load::<i64>(&establish_connection(db_dir))?;
 
     match character_count.first() {
         Some(count) => {

server/src/persistence/mod.rs

@@ -9,35 +9,37 @@ extern crate diesel;
 
 use diesel::prelude::*;
 use diesel_migrations::embed_migrations;
-use std::{env, fs, path::Path};
+use std::{env, fs, path::PathBuf};
 
 // See: https://docs.rs/diesel_migrations/1.4.0/diesel_migrations/macro.embed_migrations.html
 // This macro is called at build-time, and produces the necessary migration info
 // for the `embedded_migrations` call below.
 embed_migrations!();
 
-pub fn run_migrations() -> Result<(), diesel_migrations::RunMigrationsError> {
-    let _ = fs::create_dir(format!("{}/saves/", binary_absolute_path()));
-    embedded_migrations::run_with_output(&establish_connection(), &mut std::io::stdout())
+pub fn run_migrations(db_dir: &str) -> Result<(), diesel_migrations::RunMigrationsError> {
+    let db_dir = &apply_saves_dir_override(db_dir);
+    let _ = fs::create_dir(format!("{}/", db_dir));
+    embedded_migrations::run_with_output(&establish_connection(db_dir), &mut std::io::stdout())
 }
 
-fn establish_connection() -> SqliteConnection {
-    let database_url = format!("{}/saves/db.sqlite", binary_absolute_path());
+fn establish_connection(db_dir: &str) -> SqliteConnection {
+    let db_dir = &apply_saves_dir_override(db_dir);
+    let database_url = format!("{}/db.sqlite", db_dir);
     SqliteConnection::establish(&database_url)
         .unwrap_or_else(|_| panic!("Error connecting to {}", database_url))
 }
 
-// Get the absolute path of the binary so that the database is always stored
-// beside it, no matter where the binary is run from
-fn binary_absolute_path() -> String {
-    let binary_path;
-    match env::current_exe() {
-        Ok(exe_path) => binary_path = exe_path,
-        Err(e) => panic!("Failed to get current exe path: {}", e),
-    };
-
-    match Path::new(&binary_path.display().to_string()).parent() {
-        Some(path) => return path.display().to_string(),
-        None => panic!("Failed to get current exe parent path"),
-    };
+fn apply_saves_dir_override(db_dir: &str) -> String {
+    if let Some(val) = env::var_os("VELOREN_SAVES_DIR") {
+        let path = PathBuf::from(val);
+        if path.exists() || path.parent().map(|x| x.exists()).unwrap_or(false) {
+            // Only allow paths with valid unicode characters
+            match path.to_str() {
+                Some(path) => return path.to_owned(),
+                None => {},
+            }
+        }
+        log::warn!("VELOREN_SAVES_DIR points to an invalid path.");
+    }
+    db_dir.to_string()
 }

server/src/persistence/stats.rs

@@ -4,13 +4,18 @@ use super::{establish_connection, models::StatsUpdate, schema};
 use crate::comp;
 use diesel::prelude::*;
 
-pub fn update(character_id: i32, stats: &comp::Stats, conn: Option<&SqliteConnection>) {
+pub fn update(
+    character_id: i32,
+    stats: &comp::Stats,
+    conn: Option<&SqliteConnection>,
+    db_dir: &str,
+) {
     log::warn!("stats persisting...");
 
     if let Err(error) =
         diesel::update(schema::stats::table.filter(schema::stats::character_id.eq(character_id)))
             .set(&StatsUpdate::from(stats))
-            .execute(conn.unwrap_or(&establish_connection()))
+            .execute(conn.unwrap_or(&establish_connection(db_dir)))
     {
         log::warn!(
             "Failed to update stats for character: {:?}: {:?}",
@@ -20,8 +25,8 @@ pub fn update(character_id: i32, stats: &comp::Stats, conn: Option<&SqliteConnec
     }
 }
 
-pub fn batch_update<'a>(updates: impl Iterator<Item = (i32, &'a comp::Stats)>) {
-    let connection = &establish_connection();
+pub fn batch_update<'a>(updates: impl Iterator<Item = (i32, &'a comp::Stats)>, db_dir: &str) {
+    let connection = &establish_connection(db_dir);
 
-    updates.for_each(|(character_id, stats)| update(character_id, stats, Some(connection)));
+    updates.for_each(|(character_id, stats)| update(character_id, stats, Some(connection), db_dir));
 }

server/src/settings.rs

@@ -21,6 +21,7 @@ pub struct ServerSettings {
     /// When set to None, loads the default map file (if available); otherwise,
     /// uses the value of the file options to decide how to proceed.
     pub map_file: Option<FileOpts>,
+    pub persistence_db_dir: String,
 }
 
 impl Default for ServerSettings {
@@ -56,6 +57,7 @@ impl Default for ServerSettings {
             .iter()
             .map(|n| n.to_string())
             .collect(),
+            persistence_db_dir: "saves".to_owned(),
         }
     }
 }
@@ -95,7 +97,7 @@ impl ServerSettings {
         Ok(())
     }
 
-    pub fn singleplayer() -> Self {
+    pub fn singleplayer(persistence_db_dir: String) -> Self {
         let load = Self::load();
         Self {
             //BUG: theoretically another process can grab the port between here and server
@@ -121,9 +123,12 @@ impl ServerSettings {
             start_time: 9.0 * 3600.0,
             admins: vec!["singleplayer".to_string()], /* TODO: Let the player choose if they want
                                                        * to use admin commands or not */
+            persistence_db_dir,
             ..load // Fill in remaining fields from server_settings.ron.
         }
     }
 
     fn get_settings_path() -> PathBuf { PathBuf::from(r"server_settings.ron") }
 }
+
+pub struct PersistenceDBDir(pub String);

server/src/state_ext.rs

@@ -163,7 +163,10 @@ impl StateExt for State {
         // Grab persisted character data from the db and insert their associated
         // components. If for some reason the data can't be returned (missing
         // data, DB error), kick the client back to the character select screen.
-        match persistence::character::load_character_data(character_id) {
+        match persistence::character::load_character_data(
+            character_id,
+            &server_settings.persistence_db_dir,
+        ) {
             Ok(stats) => self.write_component(entity, stats),
             Err(error) => {
                 log::warn!(

server/src/sys/message.rs

@@ -1,5 +1,8 @@
 use super::SysTimer;
-use crate::{auth_provider::AuthProvider, client::Client, persistence, CLIENT_TIMEOUT};
+use crate::{
+    auth_provider::AuthProvider, client::Client, persistence, settings::PersistenceDBDir,
+    CLIENT_TIMEOUT,
+};
 use common::{
     comp::{Admin, CanBuild, ControlEvent, Controller, ForceUpdate, Ori, Player, Pos, Stats, Vel},
     event::{EventBus, ServerEvent},
@@ -24,6 +27,7 @@ impl<'a> System<'a> for Sys {
         Entities<'a>,
         Read<'a, EventBus<ServerEvent>>,
         Read<'a, Time>,
+        ReadExpect<'a, PersistenceDBDir>,
         ReadExpect<'a, TerrainGrid>,
         Write<'a, SysTimer<Self>>,
         ReadStorage<'a, Uid>,
@@ -47,6 +51,7 @@ impl<'a> System<'a> for Sys {
             entities,
             server_event_bus,
             time,
+            persistence_db_dir,
             terrain,
             mut timer,
             uids,
@@ -68,6 +73,8 @@ impl<'a> System<'a> for Sys {
 
         let time = time.0;
 
+        let persistence_db_dir = &persistence_db_dir.0;
+
         let mut server_emitter = server_event_bus.emitter();
 
         let mut new_chat_msgs = Vec::new();
@@ -317,6 +324,7 @@ impl<'a> System<'a> for Sys {
                         if let Some(player) = players.get(entity) {
                             match persistence::character::load_character_list(
                                 &player.uuid().to_string(),
+                                persistence_db_dir,
                             ) {
                                 Ok(character_list) => {
                                     client.notify(ServerMsg::CharacterListUpdate(character_list));
@@ -335,6 +343,7 @@ impl<'a> System<'a> for Sys {
                                 alias,
                                 tool,
                                 &body,
+                                persistence_db_dir,
                             ) {
                                 Ok(character_list) => {
                                     client.notify(ServerMsg::CharacterListUpdate(character_list));
@@ -351,6 +360,7 @@ impl<'a> System<'a> for Sys {
                             match persistence::character::delete_character(
                                 &player.uuid().to_string(),
                                 character_id,
+                                persistence_db_dir,
                             ) {
                                 Ok(character_list) => {
                                     client.notify(ServerMsg::CharacterListUpdate(character_list));

server/src/sys/persistence/stats.rs

@@ -1,9 +1,10 @@
 use crate::{
     persistence::stats,
+    settings::PersistenceDBDir,
     sys::{SysScheduler, SysTimer},
 };
 use common::comp::{Player, Stats};
-use specs::{Join, ReadStorage, System, Write};
+use specs::{Join, ReadExpect, ReadStorage, System, Write};
 
 pub struct Sys;
 
@@ -11,11 +12,15 @@ impl<'a> System<'a> for Sys {
     type SystemData = (
         ReadStorage<'a, Player>,
         ReadStorage<'a, Stats>,
+        ReadExpect<'a, PersistenceDBDir>,
         Write<'a, SysScheduler<Self>>,
         Write<'a, SysTimer<Self>>,
     );
 
-    fn run(&mut self, (players, player_stats, mut scheduler, mut timer): Self::SystemData) {
+    fn run(
+        &mut self,
+        (players, player_stats, persistence_db_dir, mut scheduler, mut timer): Self::SystemData,
+    ) {
         if scheduler.should_run() {
             timer.start();
 
@@ -23,6 +28,7 @@ impl<'a> System<'a> for Sys {
                 (&players, &player_stats)
                     .join()
                     .filter_map(|(player, stats)| player.character_id.map(|id| (id, stats))),
+                &persistence_db_dir.0,
             );
 
             timer.end();

voxygen/src/singleplayer.rs

@@ -32,7 +32,14 @@ impl Singleplayer {
         let (sender, receiver) = unbounded();
 
         // Create server
-        let settings = ServerSettings::singleplayer();
+        let settings = ServerSettings::singleplayer(
+            crate::settings::Settings::get_settings_path()
+                .parent()
+                .unwrap()
+                .join("saves")
+                .to_string_lossy()
+                .to_string(),
+        );
 
         let thread_pool = client.map(|c| c.thread_pool().clone());
         let settings2 = settings.clone();