fix all mapmanip rust linting warnings and tests (#28874)

* fix all mapmanip rust linting warnings and tests

* Build Rust library

* Build Rust library

---------

Co-authored-by: paradisess13[bot] <165046124+paradisess13[bot]@users.noreply.github.com>

.gitignore

@@ -72,7 +72,8 @@ $RECYCLE.BIN
 # VIM swap files.
 *.swp
 
-# Rust output.
+# Rust output and test output files.
+/rust/src/mapmanip/test/*_out.dmm
 /rust/target/*
 rustlibs_panic.txt
 

rust/src/logging/mod.rs

@@ -2,7 +2,6 @@ use byondapi::global_call::call_global;
 use byondapi::prelude::ByondValue;
 use byondapi::threadsync::thread_sync;
 use chrono::prelude::Utc;
-use eyre;
 
 /// Call stack trace dm method with message.
 pub(crate) fn dm_call_stack_trace(msg: String) -> eyre::Result<()> {
@@ -21,12 +20,22 @@ pub(crate) fn setup_panic_handler() {
             || -> ByondValue {
                 if let Err(error) = dm_call_stack_trace(msg_copy) {
                     let second_msg = format!("BYOND error \n {:#?}", error);
-                    let _ = std::fs::write(Utc::now().format("data/rustlibs_dm_trace_failed_%Y%m%d_%H%M%S.txt").to_string(), second_msg.clone());
+                    let _ = std::fs::write(
+                        Utc::now()
+                            .format("data/rustlibs_dm_trace_failed_%Y%m%d_%H%M%S.txt")
+                            .to_string(),
+                        second_msg.clone(),
+                    );
                 }
                 Default::default()
             },
             true,
         );
-        let _ = std::fs::write(Utc::now().format("data/rustlibs_panic_%Y%m%d_%H%M%S.txt").to_string(), msg.clone());
+        let _ = std::fs::write(
+            Utc::now()
+                .format("data/rustlibs_panic_%Y%m%d_%H%M%S.txt")
+                .to_string(),
+            msg.clone(),
+        );
     }))
 }

rust/src/mapmanip/core/mod.rs

@@ -114,6 +114,7 @@ impl TileGrid {
             .map(|(i, t)| (self.index_to_coord(i), t))
     }
 
+    #[allow(dead_code)]
     pub fn keys(&self) -> impl Iterator<Item = Coord3> + '_ {
         self.grid
             .iter()
@@ -136,14 +137,16 @@ impl TileGrid {
 /// It is not memory efficient, but it allows for much greater flexibility of manipulation.
 #[derive(Clone, Debug)]
 pub struct GridMap {
-    ///
+    /// The x, y, and z dimensions of the map.
     pub size: dmm::Coord3,
-    ///
+    /// The key-value data of the map in TileGrid format.
     pub grid: crate::mapmanip::core::TileGrid,
 }
 
 impl GridMap {
     pub fn from_file(path: &std::path::Path) -> eyre::Result<GridMap> {
-        Ok(to_grid_map(&dmm::Map::from_file(&path).wrap_err(format!("failure to read from dmm parser"))?))
+        Ok(to_grid_map(
+            &dmm::Map::from_file(path).wrap_err("failure to read from dmm parser")?,
+        ))
     }
 }

rust/src/mapmanip/core/to_dict_map.rs

@@ -38,14 +38,13 @@ pub fn to_dict_map(grid_map: &GridMap) -> eyre::Result<dmm::Map> {
             if used_dict_keys.contains(&tile.key_suggestion) {
                 let next_free_key = (0..65534)
                     .map(int_to_key)
-                    .filter(|k| !used_dict_keys.contains(k))
-                    .next()
-                    .wrap_err(format!("ran out of free keys"))?;
+                    .find(|k| !used_dict_keys.contains(k))
+                    .wrap_err("ran out of free keys")?;
                 dictionary_reverse.insert(tile.prefabs.clone(), next_free_key);
                 used_dict_keys.insert(next_free_key);
             } else {
-                dictionary_reverse.insert(tile.prefabs.clone(), tile.key_suggestion.clone());
-                used_dict_keys.insert(tile.key_suggestion.clone());
+                dictionary_reverse.insert(tile.prefabs.clone(), tile.key_suggestion);
+                used_dict_keys.insert(tile.key_suggestion);
             }
         }
     }
@@ -55,7 +54,7 @@ pub fn to_dict_map(grid_map: &GridMap) -> eyre::Result<dmm::Map> {
             for z in 1..(grid_map.size.z + 1) {
                 let coord = Coord3::new(x, y, z);
                 if let Some(tile) = grid_map.grid.get(&coord) {
-                    let key = dictionary_reverse.get(&tile.prefabs).unwrap().clone();
+                    let key = *dictionary_reverse.get(&tile.prefabs).unwrap();
                     dict_map.dictionary.insert(key, tile.prefabs.clone());
                     dict_map.grid[coord3_to_index(coord, grid_map.size)] = key;
                 } else {

rust/src/mapmanip/mod.rs

@@ -20,7 +20,6 @@ use serde::{Deserialize, Serialize};
 use tools::extract_submap;
 use tools::insert_submap;
 
-use crate::logging::dm_call_stack_trace;
 use crate::logging::setup_panic_handler;
 
 mod core;
@@ -29,7 +28,12 @@ mod tools;
 #[cfg(test)]
 mod test;
 
-///
+/// A specific map transformation. Currently implemented transformations are
+/// `SubmapExtractInsert`, which looks up matching markers in another DMM file
+/// to replace the contents of the specified submap dimensions, and
+/// `RandomOrientation`, which rotates the map 0, 90, 180, or 270 degrees,
+/// performing specialized transformations for atoms which require it in order
+/// to make sense when rotated.
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(tag = "type")]
 pub enum MapManipulation {
@@ -74,7 +78,7 @@ pub fn mapmanip_config_parse(config_path: &std::path::Path) -> eyre::Result<Vec<
 pub fn mapmanip(
     map_dir_path: &std::path::Path,
     map: dmmtools::dmm::Map,
-    config: &Vec<MapManipulation>,
+    config: &[MapManipulation],
 ) -> eyre::Result<dmmtools::dmm::Map> {
     // convert to gridmap
     let mut map = to_grid_map(&map);
@@ -112,15 +116,17 @@ pub fn mapmanip(
 					submaps path: {submaps_dmm:?};
 					markers: {marker_extract}, {marker_insert};"
             )),
-            MapManipulation::RandomOrientation => mapmanip_orientation_randomize(&mut map)
-                .wrap_err(format!("randomize orientation fail")),
+            MapManipulation::RandomOrientation => {
+                mapmanip_orientation_randomize(&mut map).wrap_err("randomize orientation failure")
+            }
         }
         .wrap_err(format!("mapmanip fail; manip n is: {n}/{config_len}"))?;
     }
 
-    Ok(core::to_dict_map(&map).wrap_err("failed on `to_dict_map`")?)
+    core::to_dict_map(&map).wrap_err("failed on `to_dict_map`")
 }
 
+#[allow(clippy::too_many_arguments)]
 fn mapmanip_submap_extract_insert(
     map_dir_path: &std::path::Path,
     map: &mut GridMap,
@@ -140,7 +146,7 @@ fn mapmanip_submap_extract_insert(
     );
 
     // get the submaps map
-    let submaps_dmm: std::path::PathBuf = submaps_dmm.try_into().wrap_err("invalid path")?;
+    let submaps_dmm: std::path::PathBuf = submaps_dmm.into();
     let submaps_dmm = map_dir_path.join(submaps_dmm);
     let submaps_map = GridMap::from_file(&submaps_dmm)
         .wrap_err(format!("can't read and parse submap dmm: {submaps_dmm:?}"))?;
@@ -420,13 +426,10 @@ fn mapmanip_read_dmm_file(path: ByondValue) -> eyre::Result<ByondValue> {
 pub(crate) fn internal_mapmanip_read_dmm_file(path: ByondValue) -> eyre::Result<ByondValue> {
     setup_panic_handler();
 
-    let path: String = path
-        .get_string()
-        .wrap_err(format!("path arg is not a string: {:?}", path))?;
     let path: std::path::PathBuf = path
-        .clone()
-        .try_into()
-        .wrap_err(format!("path arg is not a valid file path: {}", path))?;
+        .get_string()
+        .wrap_err(format!("path arg is not a string: {:?}", path))?
+        .into();
 
     // just return null if path is bad for whatever reason
     if !path.is_file() || !path.exists() {
@@ -465,32 +468,17 @@ pub(crate) fn internal_mapmanip_read_dmm_file(path: ByondValue) -> eyre::Result<
     Ok(ByondValue::new_str(dmm)?)
 }
 
-///
-#[no_mangle]
-pub unsafe extern "C" fn read_dmm_file_ffi(
-    argc: byondapi::sys::u4c,
-    argv: *mut byondapi::value::ByondValue,
-) -> byondapi::value::ByondValue {
-    setup_panic_handler();
-    let args = unsafe { ::byondapi::parse_args(argc, argv) };
-    match crate::mapmanip::internal_mapmanip_read_dmm_file(
-        args.get(0).map(ByondValue::clone).unwrap_or_default(),
-    ) {
-        Ok(val) => val,
-        Err(info) => {
-            let _ = dm_call_stack_trace(format!("Rustlibs ERROR read_dmm_file_ffi() \n {info:#?}"));
-            ByondValue::null()
-        }
-    }
-}
-
 /// To be used by the `tools/rustlib_tools/mapmanip.ps1` script.
 /// Not to be called from the game server, so bad error-handling is fine.
 /// This should run map manipulations on every `.dmm` map that has a `.jsonc` config file,
 /// and write it to a `.mapmanipout.dmm` file in the same location.
 #[no_mangle]
 pub unsafe extern "C" fn all_mapmanip_configs_execute_ffi() {
-    let mapmanip_configs = walkdir::WalkDir::new("./_maps")
+    all_mapmanip_configs_execute("./_maps".into());
+}
+
+fn all_mapmanip_configs_execute(root_path: String) {
+    let mapmanip_configs = walkdir::WalkDir::new(root_path)
         .into_iter()
         .map(|d| d.unwrap().path().to_owned())
         .filter(|p| p.extension().is_some())

rust/src/mapmanip/test.rs

@@ -1,10 +1,10 @@
 use dmmtools::dmm::{self, Coord3};
 use itertools::Itertools;
 
-use super::all_mapmanip_configs_execute_ffi;
+use super::all_mapmanip_configs_execute;
 
 fn print_diff(left: &str, right: &str) {
-    for (i, diff) in diff::lines(&left, &right).iter().enumerate() {
+    for (i, diff) in diff::lines(left, right).iter().enumerate() {
         match diff {
             diff::Result::Left(l) => println!("{} diff - : {}", i, l),
             diff::Result::Both(l, r) => {
@@ -16,7 +16,7 @@ fn print_diff(left: &str, right: &str) {
 }
 
 fn all_test_dmm() -> Vec<std::path::PathBuf> {
-    std::fs::read_dir("src/mapmanip/test-in")
+    std::fs::read_dir("src/mapmanip/test/")
         .unwrap()
         .map(|r| r.unwrap().path())
         .filter(|p| p.extension().unwrap() == "dmm")
@@ -36,25 +36,25 @@ fn to_grid_and_back() {
         let map_str_from_grid = crate::mapmanip::core::map_to_string(&dict_map_again).unwrap();
 
         dict_map_again
-            .to_file(&std::path::Path::new("src/mapmanip/test-out").join(path.file_name().unwrap()))
+            .to_file(&std::path::Path::new("src/mapmanip/test/").join(path.file_name().unwrap()))
             .unwrap();
 
         print_diff(&map_str_original, &map_str_from_grid);
 
         if map_str_original != map_str_from_grid {
-            assert!(false);
+            panic!("map from grid does not match original");
         }
     }
 }
 
 #[test]
 fn extract() {
-    let path_src = std::path::Path::new("src/mapmanip/test-in/_tiny_test_map.dmm");
-    let path_xtr = std::path::Path::new("src/mapmanip/test-in/extracted.dmm");
-    let path_xtr_out = std::path::Path::new("src/mapmanip/test-out/extracted_out.dmm");
+    let path_src = std::path::Path::new("src/mapmanip/test/_tiny_test_map.dmm");
+    let path_xtr = std::path::Path::new("src/mapmanip/test/extracted.dmm");
+    let path_xtr_out = std::path::Path::new("src/mapmanip/test/extracted_out.dmm");
 
-    let dict_map_src = dmmtools::dmm::Map::from_file(&path_src).unwrap();
-    let dict_map_xtr_expected = dmmtools::dmm::Map::from_file(&path_xtr).unwrap();
+    let dict_map_src = dmmtools::dmm::Map::from_file(path_src).unwrap();
+    let dict_map_xtr_expected = dmmtools::dmm::Map::from_file(path_xtr).unwrap();
 
     let grid_map_src = crate::mapmanip::core::to_grid_map(&dict_map_src);
     let grid_map_xtr = crate::mapmanip::tools::extract_submap(
@@ -82,14 +82,14 @@ fn extract() {
 
 #[test]
 fn insert() {
-    let path_xtr = std::path::Path::new("src/mapmanip/test-in/extracted.dmm");
-    let path_dst = std::path::Path::new("src/mapmanip/test-in/_tiny_test_map.dmm");
-    let path_dst_expected = std::path::Path::new("src/mapmanip/test-in/inserted.dmm");
+    let path_xtr = std::path::Path::new("src/mapmanip/test/extracted.dmm");
+    let path_dst = std::path::Path::new("src/mapmanip/test/_tiny_test_map.dmm");
+    let path_dst_expected = std::path::Path::new("src/mapmanip/test/inserted.dmm");
 
     let grid_map_dst_expected =
-        crate::mapmanip::core::GridMap::from_file(&path_dst_expected).unwrap();
-    let grid_map_xtr = crate::mapmanip::core::GridMap::from_file(&path_xtr).unwrap();
-    let mut grid_map_dst = crate::mapmanip::core::GridMap::from_file(&path_dst).unwrap();
+        crate::mapmanip::core::GridMap::from_file(path_dst_expected).unwrap();
+    let grid_map_xtr = crate::mapmanip::core::GridMap::from_file(path_xtr).unwrap();
+    let mut grid_map_dst = crate::mapmanip::core::GridMap::from_file(path_dst).unwrap();
     crate::mapmanip::tools::insert_submap(&grid_map_xtr, Coord3::new(6, 4, 1), &mut grid_map_dst)
         .unwrap();
 
@@ -110,8 +110,8 @@ fn keys_deduplicated() {
     // make sure that if multiple tiles have the same key_suggestion
     // they get assigned different keys
 
-    let path_src = std::path::Path::new("src/mapmanip/test-in/_tiny_test_map.dmm");
-    let dict_map_src = dmmtools::dmm::Map::from_file(&path_src).unwrap();
+    let path_src = std::path::Path::new("src/mapmanip/test/_tiny_test_map.dmm");
+    let dict_map_src = dmmtools::dmm::Map::from_file(path_src).unwrap();
     let grid_map_src = crate::mapmanip::core::to_grid_map(&dict_map_src);
 
     let mut grid_map_out = crate::mapmanip::core::to_grid_map(&dict_map_src);
@@ -143,7 +143,7 @@ fn mapmanip_configs_parse() {
     }];
     dbg!(serde_json::to_string(&foo).unwrap());
 
-    let mapmanip_configs = walkdir::WalkDir::new("../../maps")
+    let mapmanip_configs = walkdir::WalkDir::new("../_maps")
         .into_iter()
         .map(|d| d.unwrap().path().to_owned())
         .filter(|p| p.extension().is_some())
@@ -157,7 +157,5 @@ fn mapmanip_configs_parse() {
 
 #[test]
 fn mapmanip_configs_execute() {
-    // this is only "unsafe" cause that function is `extern "C"`
-    // it does not do anything actually unsafe
-    unsafe { all_mapmanip_configs_execute_ffi() }
+    all_mapmanip_configs_execute("../_maps".into());
 }

rust/src/milla/tick.rs

@@ -111,7 +111,7 @@ pub(crate) fn tick_z_level(
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::constants::*;
+    use crate::milla::constants::*;
 
     fn set_with_defaults<F>(legend: F) -> impl Fn(char) -> Tile
     where