Refactor large portions of database.rs, and fix the remaining clippy warnings (#111)

* Address warning "warning: very complex type used. Consider factoring parts into `type` definitions"
-But instead of making a new type we just create PlayerData directly

* Address "warning: this expression creates a reference which is immediately dereferenced by the compiler"


* Address "warning: calling .bytes() is very inefficient when data is not in memory"

src/common/workdefinitions/chara_make.rs

@@ -1,3 +1,4 @@
+use rusqlite::types::{FromSql, FromSqlResult, ValueRef};
 use serde_json::{Value, json};
 
 use crate::common::CustomizeData;
@@ -13,6 +14,12 @@ pub struct CharaMake {
     pub unk2: i32,
 }
 
+impl FromSql for CharaMake {
+    fn column_result(value: ValueRef<'_>) -> FromSqlResult<Self> {
+        Ok(CharaMake::from_json(&String::column_result(value)?))
+    }
+}
+
 impl CharaMake {
     pub fn from_json(json: &str) -> Self {
         let v: Value = serde_json::from_str(json).unwrap();

src/common/workdefinitions/client_select_data.rs

@@ -1,3 +1,4 @@
+use rusqlite::types::{FromSql, FromSqlResult, ValueRef};
 use serde::{Deserialize, Serialize};
 use serde_json::json;
 
@@ -28,6 +29,12 @@ impl TryFrom<i32> for RemakeMode {
     }
 }
 
+impl FromSql for RemakeMode {
+    fn column_result(value: ValueRef<'_>) -> FromSqlResult<Self> {
+        Ok(RemakeMode::try_from(i32::column_result(value)?).unwrap())
+    }
+}
+
 /// See https://github.com/aers/FFXIVClientStructs/blob/main/FFXIVClientStructs/FFXIV/Application/Network/WorkDefinitions/ClientSelectData.cs
 #[derive(Debug)]
 pub struct ClientSelectData {

src/inventory/mod.rs

@@ -1,9 +1,9 @@
+use crate::common::GameData;
 use icarus::{ClassJob::ClassJobSheet, Race::RaceSheet};
 use physis::common::Language;
+use rusqlite::types::{FromSql, FromSqlResult, ValueRef};
 use serde::{Deserialize, Serialize};
 
-use crate::common::GameData;
-
 use crate::ipc::zone::ItemOperation;
 
 mod equipped;
@@ -78,6 +78,12 @@ impl<'a> IntoIterator for &'a Inventory {
     }
 }
 
+impl FromSql for Inventory {
+    fn column_result(value: ValueRef<'_>) -> FromSqlResult<Self> {
+        Ok(serde_json::from_str(&String::column_result(value)?).unwrap())
+    }
+}
+
 pub struct InventoryIterator<'a> {
     inventory: &'a Inventory,
     curr: u16,

src/ipc/zone/common_spawn.rs

@@ -1,11 +1,11 @@
 use binrw::binrw;
 
-use bitflags::bitflags;
-
 use crate::common::{
     CHAR_NAME_MAX_LENGTH, CustomizeData, ObjectId, ObjectTypeId, Position, read_quantized_rotation,
     read_string, write_quantized_rotation, write_string,
 };
+use bitflags::bitflags;
+use rusqlite::types::{FromSql, FromSqlResult, ValueRef};
 
 use super::StatusEffect;
 
@@ -116,6 +116,12 @@ pub enum GameMasterRank {
     Debug = 90,
 }
 
+impl FromSql for GameMasterRank {
+    fn column_result(value: ValueRef<'_>) -> FromSqlResult<Self> {
+        Ok(GameMasterRank::try_from(u8::column_result(value)?).unwrap())
+    }
+}
+
 impl TryFrom<u8> for GameMasterRank {
     type Error = ();
 

src/world/database.rs

@@ -1,4 +1,4 @@
-use std::{io::Read, sync::Mutex};
+use std::{io::BufReader, io::Read, sync::Mutex};
 
 use rusqlite::Connection;
 use serde::Deserialize;
@@ -10,10 +10,7 @@ use crate::{
         workdefinitions::{CharaMake, ClientSelectData, RemakeMode},
     },
     inventory::{Inventory, Item, Storage},
-    ipc::{
+    ipc::lobby::{CharacterDetails, CharacterFlag},
-        lobby::{CharacterDetails, CharacterFlag},
-        zone::GameMasterRank,
-    },
 };
 
 use super::PlayerData;
@@ -36,6 +33,10 @@ impl Default for WorldDatabase {
     }
 }
 
+fn json_unpack<T: for<'a> Deserialize<'a>>(json_str: String) -> T {
+    serde_json::from_str(&json_str).unwrap()
+}
+
 impl WorldDatabase {
     pub fn new() -> Self {
         let connection = Connection::open("world.db").expect("Failed to open database!");
@@ -146,7 +147,15 @@ impl WorldDatabase {
         }
 
         let charsave_file = archive.by_name("FFXIV_CHARA_01.dat").unwrap();
-        let charsave_bytes: Vec<u8> = charsave_file.bytes().map(|x| x.unwrap()).collect();
+        let mut charsave_bytes = Vec::<u8>::new();
+        let mut bufrdr = BufReader::new(charsave_file);
+        if let Err(err) = bufrdr.read_to_end(&mut charsave_bytes) {
+            tracing::error!(
+                "Unable to read FFXIV_CHARA_01.dat from archive! Additional information: {err}"
+            );
+            return;
+        };
+
         let charsave =
             physis::savedata::chardat::CharacterData::from_existing(&charsave_bytes).unwrap();
 
@@ -273,84 +282,40 @@ impl WorldDatabase {
         let mut stmt = connection
             .prepare("SELECT content_id, service_account_id FROM characters WHERE actor_id = ?1")
             .unwrap();
-        let (content_id, account_id) = stmt
+        let (content_id, account_id): (u64, u32) = stmt
             .query_row((actor_id,), |row| Ok((row.get(0)?, row.get(1)?)))
             .unwrap();
 
         stmt = connection
             .prepare("SELECT pos_x, pos_y, pos_z, rotation, zone_id, inventory, gm_rank, classjob_id, classjob_levels, classjob_exp, unlocks, aetherytes, completed_quests FROM character_data WHERE content_id = ?1")
             .unwrap();
-        let (
+        let player_data: PlayerData = stmt
-            pos_x,
-            pos_y,
-            pos_z,
-            rotation,
-            zone_id,
-            inventory_json,
-            gm_rank,
-            classjob_id,
-            classjob_levels,
-            classjob_exp,
-            unlocks,
-            aetherytes,
-            completed_quests,
-        ): (
-            f32,
-            f32,
-            f32,
-            f32,
-            u16,
-            String,
-            u8,
-            i32,
-            String,
-            String,
-            String,
-            String,
-            String,
-        ) = stmt
             .query_row((content_id,), |row| {
-                Ok((
+                Ok(PlayerData {
-                    row.get(0)?,
-                    row.get(1)?,
-                    row.get(2)?,
-                    row.get(3)?,
-                    row.get(4)?,
-                    row.get(5)?,
-                    row.get(6)?,
-                    row.get(7)?,
-                    row.get(8)?,
-                    row.get(9)?,
-                    row.get(10)?,
-                    row.get(11)?,
-                    row.get(12)?,
-                ))
-            })
-            .unwrap();
-
-        let inventory = serde_json::from_str(&inventory_json).unwrap();
-
-        PlayerData {
                     actor_id,
                     content_id,
                     account_id,
                     position: Position {
-                x: pos_x,
+                        x: row.get(0)?,
-                y: pos_y,
+                        y: row.get(1)?,
-                z: pos_z,
+                        z: row.get(2)?,
                     },
-            rotation,
+                    rotation: row.get(3)?,
-            zone_id,
+                    zone_id: row.get(4)?,
-            inventory,
+                    inventory: row.get(5)?,
-            gm_rank: GameMasterRank::try_from(gm_rank).unwrap(),
+                    gm_rank: row.get(6)?,
-            classjob_id: classjob_id as u8,
+                    classjob_id: row.get(7)?,
-            classjob_levels: serde_json::from_str(&classjob_levels).unwrap(),
+                    classjob_levels: json_unpack::<[i32; 32]>(row.get(8)?),
-            classjob_exp: serde_json::from_str(&classjob_exp).unwrap(),
+                    classjob_exp: json_unpack::<[u32; 32]>(row.get(9)?),
-            unlocks: serde_json::from_str(&unlocks).unwrap(),
+                    unlocks: json_unpack::<Vec<u8>>(row.get(10)?),
-            aetherytes: serde_json::from_str(&aetherytes).unwrap(),
+                    aetherytes: json_unpack::<Vec<u8>>(row.get(11)?),
-            completed_quests: serde_json::from_str(&completed_quests).unwrap(),
+                    completed_quests: json_unpack::<Vec<u8>>(row.get(12)?),
                     ..Default::default()
-        }
+                })
+            })
+            .unwrap();
+
+        player_data
     }
 
     /// Commit the dynamic player data back to the database
@@ -425,56 +390,53 @@ impl WorldDatabase {
                 )
                 .unwrap();
 
-            let result: Result<(String, String, u16, String, i32, i32, String), rusqlite::Error> =
+            struct CharaListQuery {
+                name: String,
+                chara_make: CharaMake,
+                zone_id: u16,
+                inventory: Inventory,
+                remake_mode: RemakeMode,
+                classjob_id: i32,
+                classjob_levels: [i32; 32],
+            }
+
+            let result: Result<CharaListQuery, rusqlite::Error> =
                 stmt.query_row((content_id,), |row| {
-                    Ok((
+                    Ok(CharaListQuery {
-                        row.get(0)?,
+                        name: row.get(0)?,
-                        row.get(1)?,
+                        chara_make: row.get(1)?,
-                        row.get(2)?,
+                        zone_id: row.get(2)?,
-                        row.get(3)?,
+                        inventory: row.get(3)?,
-                        row.get(4)?,
+                        remake_mode: row.get(4)?,
-                        row.get(5)?,
+                        classjob_id: row.get(5)?,
-                        row.get(6)?,
+                        classjob_levels: json_unpack::<[i32; 32]>(row.get(6)?),
-                    ))
+                    })
                 });
 
-            if let Ok((
+            if let Ok(query) = result {
-                name,
-                chara_make,
-                zone_id,
-                inventory_json,
-                remake_mode,
-                classjob_id,
-                classjob_levels,
-            )) = result
-            {
-                let chara_make = CharaMake::from_json(&chara_make);
-
-                let inventory: Inventory = serde_json::from_str(&inventory_json).unwrap();
-
                 let select_data = ClientSelectData {
-                    character_name: name.clone(),
+                    character_name: query.name.clone(),
-                    current_class: classjob_id,
+                    current_class: query.classjob_id,
-                    class_levels: serde_json::from_str(&classjob_levels).unwrap(),
+                    class_levels: query.classjob_levels,
-                    race: chara_make.customize.race as i32,
+                    race: query.chara_make.customize.race as i32,
-                    subrace: chara_make.customize.subrace as i32,
+                    subrace: query.chara_make.customize.subrace as i32,
-                    gender: chara_make.customize.gender as i32,
+                    gender: query.chara_make.customize.gender as i32,
-                    birth_month: chara_make.birth_month,
+                    birth_month: query.chara_make.birth_month,
-                    birth_day: chara_make.birth_day,
+                    birth_day: query.chara_make.birth_day,
-                    guardian: chara_make.guardian,
+                    guardian: query.chara_make.guardian,
                     unk8: 0,
                     unk9: 0,
-                    zone_id: zone_id as i32,
+                    zone_id: query.zone_id as i32,
                     content_finder_condition: 0,
-                    customize: chara_make.customize,
+                    customize: query.chara_make.customize,
-                    model_main_weapon: inventory.get_main_weapon_id(game_data),
+                    model_main_weapon: query.inventory.get_main_weapon_id(game_data),
                     model_sub_weapon: 0,
-                    model_ids: inventory.get_model_ids(game_data),
+                    model_ids: query.inventory.get_model_ids(game_data),
                     equip_stain: [0; 10],
                     glasses: [0; 2],
-                    remake_mode: RemakeMode::try_from(remake_mode).unwrap(),
+                    remake_mode: query.remake_mode,
                     remake_minutes_remaining: 0,
-                    voice_id: chara_make.voice_id,
+                    voice_id: query.chara_make.voice_id,
                     unk20: 0,
                     unk21: 0,
                     world_name: String::new(),
@@ -490,7 +452,7 @@ impl WorldDatabase {
                     unk1: [255; 6],
                     origin_server_id: world_id,
                     current_server_id: world_id,
-                    character_name: name.clone(),
+                    character_name: query.name.clone(),
                     origin_server_name: world_name.to_string(),
                     current_server_name: world_name.to_string(),
                     character_detail_json: select_data.to_json(),

src/world/zone.rs

@@ -61,7 +61,7 @@ impl Zone {
             let mut nvm_path = PathBuf::from(config.filesystem.navimesh_path);
             nvm_path.push(&zone.navimesh_path);
 
-            Self::load_navimesh(&nvm_path.to_str().unwrap());
+            Self::load_navimesh(nvm_path.to_str().unwrap());
         }
 
         let mut load_lgb = |path: &str| -> Option<LayerGroup> {