BAPI - mapmanip submap perf improv + mapmanip script (#19634)

changes:
  - rscadd: "BAPI - mapmanip submap performance improvements."
  - rscadd: "BAPI - mapmanip script."

perf improv in the form of changing the map container from hashmap to a
flat grid
hashmap was only bad for big maps with lots of submaps - did not affect
horizon with its one small submap

the script:


![image](https://github.com/user-attachments/assets/136365c3-5cbc-4189-90a0-f163ea836735)

---------

Co-authored-by: DreamySkrell <>
Co-authored-by: AuroraBuildBot <[email protected]>

Author: DreamySkrell

bapi.dll

@@ -0,0 +1,8 @@
+
+author: DreamySkrell
+
+delete-after: True
+
+changes:
+  - rscadd: "BAPI - mapmanip submap performance improvements."
+  - rscadd: "BAPI - mapmanip script."

html/changelogs/DreamySkrell-bapi-perf-improv-and-script.yml

@@ -22,6 +22,8 @@ eyre = "0.6.12"
 diff = "0.1"
 # general utility lib for iterator operations
 itertools = "0.10.5"
+# simple flat grid, basically just wrapper over Vec<Vec<T>>
+grid = "0.14.0"
 # fast hashmap
 fxhash = "0.2.1"
 # generating random numbers

rust/bapi/Cargo.lock

@@ -3,6 +3,7 @@ mod mapmanip;
 
 use byondapi::prelude::*;
 use eyre::{Context, ContextCompat};
+use itertools::Itertools;
 
 /// Call stack trace dm method with message.
 pub(crate) fn dm_call_stack_trace(msg: String) {
@@ -94,3 +95,43 @@ fn read_dmm_file(path: ByondValue) -> eyre::Result<ByondValue> {
     // and return it
     Ok(ByondValue::new_str(dmm)?)
 }
+
+/// To be used by the `tools/bapi/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")
+        .into_iter()
+        .map(|d| d.unwrap().path().to_owned())
+        .filter(|p| p.extension().is_some())
+        .filter(|p| p.extension().unwrap() == "jsonc")
+        .collect_vec();
+    assert_ne!(mapmanip_configs.len(), 0);
+
+    for config_path in mapmanip_configs {
+        let dmm_path = {
+            let mut p = config_path.clone();
+            p.set_extension("dmm");
+            p
+        };
+
+        let path_dir: &std::path::Path = dmm_path.parent().unwrap();
+
+        let mut dmm = dmmtools::dmm::Map::from_file(&dmm_path).unwrap();
+
+        let config = crate::mapmanip::mapmanip_config_parse(&config_path).unwrap();
+
+        dmm = crate::mapmanip::mapmanip(path_dir, dmm, &config).unwrap();
+
+        let dmm = crate::mapmanip::core::map_to_string(&dmm).unwrap();
+
+        let dmm_out_path = {
+            let mut p = dmm_path.clone();
+            p.set_extension("mapmanipout.dmm");
+            p
+        };
+        std::fs::write(dmm_out_path, dmm).unwrap();
+    }
+}

rust/bapi/Cargo.toml

@@ -7,9 +7,10 @@ pub mod map_to_string;
 pub use map_to_string::map_to_string;
 
 use dmmtools::dmm;
+use dmmtools::dmm::Coord2;
 
 ///
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, Default)]
 pub struct Tile {
     ///
     pub key_suggestion: dmm::Key,
@@ -53,6 +54,61 @@ impl Tile {
     }
 }
 
+/// Thin abstraction over `grid::Grid`, to provide a hashmap-like interface,
+/// and to translate between dmm coords (start at 1) and grid coords (start at 0).
+/// The translation is so that it looks better in logs/errors/etc,
+/// where shown coords would correspond to coords seen in game or in strongdmm.
+#[derive(Clone, Debug)]
+pub struct TileGrid {
+    pub grid: grid::Grid<crate::mapmanip::core::Tile>,
+}
+
+impl TileGrid {
+    pub fn new(size_x: i32, size_y: i32) -> TileGrid {
+        Self {
+            grid: grid::Grid::new(size_x as usize, size_y as usize),
+        }
+    }
+
+    pub fn len(&self) -> usize {
+        self.grid.size().0 * self.grid.size().1
+    }
+
+    pub fn iter(&self) -> impl Iterator<Item = (Coord2, &Tile)> {
+        self.grid
+            .indexed_iter()
+            .map(|((x, y), t)| (Coord2::new((x + 1) as i32, (y + 1) as i32), t))
+    }
+
+    pub fn get_mut(&mut self, coord: &Coord2) -> Option<&mut Tile> {
+        self.grid
+            .get_mut((coord.x - 1) as usize, (coord.y - 1) as usize)
+    }
+
+    pub fn get(&self, coord: &Coord2) -> Option<&Tile> {
+        self.grid
+            .get((coord.x - 1) as usize, (coord.y - 1) as usize)
+    }
+
+    pub fn insert(&mut self, coord: &Coord2, tile: Tile) {
+        *self.get_mut(coord).unwrap() = tile;
+    }
+
+    pub fn keys(&self) -> impl Iterator<Item = Coord2> + '_ {
+        self.grid
+            .indexed_iter()
+            .map(|((x, y), _t)| Coord2::new((x + 1) as i32, (y + 1) as i32))
+    }
+
+    pub fn values(&self) -> impl Iterator<Item = &Tile> {
+        self.grid.iter()
+    }
+
+    pub fn values_mut(&mut self) -> impl Iterator<Item = &mut Tile> {
+        self.grid.iter_mut()
+    }
+}
+
 /// This is analogous to `dmmtools::dmm::Map`, but instead of being structured like dmm maps are,
 /// where they have a dictionary of keys-to-prefabs and a separate grid of keys,
 /// this is only a direct coord-to-prefab grid.
@@ -62,7 +118,7 @@ pub struct GridMap {
     ///
     pub size: dmm::Coord3,
     ///
-    pub grid: std::collections::BTreeMap<dmm::Coord2, crate::mapmanip::core::Tile>,
+    pub grid: crate::mapmanip::core::TileGrid,
 }
 
 impl GridMap {

rust/bapi/src/lib.rs

@@ -10,7 +10,10 @@ fn tuple_to_size(xyz: (usize, usize, usize)) -> Coord3 {
 pub fn to_grid_map(dict_map: &dmm::Map) -> GridMap {
     let mut grid_map = GridMap {
         size: tuple_to_size(dict_map.dim_xyz()),
-        grid: Default::default(),
+        grid: crate::mapmanip::core::TileGrid::new(
+            dict_map.dim_xyz().0 as i32,
+            dict_map.dim_xyz().1 as i32,
+        ),
     };
 
     for x in 1..grid_map.size.x + 1 {
@@ -23,7 +26,7 @@ pub fn to_grid_map(dict_map: &dmm::Map) -> GridMap {
                 prefabs,
             };
             let coord = dmm::Coord2::new(coord.x, coord.y);
-            grid_map.grid.insert(coord, tile);
+            grid_map.grid.insert(&coord, tile);
         }
     }
 

rust/bapi/src/mapmanip/core/mod.rs

@@ -111,15 +111,15 @@ fn mapmanip_submap_extract_insert(
     let mut marker_extract_coords = vec![];
     for (coord, tile) in submaps_map.grid.iter() {
         if tile.prefabs.iter().any(|p| p.path == *marker_extract) {
-            marker_extract_coords.push(*coord);
+            marker_extract_coords.push(coord);
         }
     }
 
     // find all the insert markers
     let mut marker_insert_coords = vec![];
     for (coord, tile) in map.grid.iter() {
         if tile.prefabs.iter().any(|p| p.path == *marker_insert) {
-            marker_insert_coords.push(*coord);
+            marker_insert_coords.push(coord);
         }
     }
 
@@ -132,17 +132,13 @@ fn mapmanip_submap_extract_insert(
             .enumerate()
             .choose(&mut rand::thread_rng())
             .wrap_err(format!(
-                "can't pick a submap to extract; no extract markers in the submaps dmm; marker type: {marker_extract}"
+                "can't pick a submap to extract; no more extract markers in the submaps dmm; marker type: {marker_extract}"
             ))?;
 
         // if submaps should not be repeating, remove this one from the list
         if !submaps_can_repeat {
             marker_extract_coords.remove(extract_coord_index);
         }
-        eyre::ensure!(
-            !marker_extract_coords.is_empty(),
-            format!("no more submaps left to extract; marker type: {marker_extract}")
-        );
 
         // extract that submap from the submap dmm
         let extracted = tools::extract_submap(&submaps_map, extract_coord, submap_size)

rust/bapi/src/mapmanip/core/to_grid_map.rs

@@ -22,38 +22,14 @@ fn all_test_dmm() -> Vec<std::path::PathBuf> {
         .collect_vec()
 }
 
-#[test]
-fn grid_check() {
-    let path = std::path::Path::new("src/mapmanip/test-in/_tiny_test_map.dmm");
-    println!("path: {}", path.display());
-
-    let grid_map = crate::mapmanip::core::GridMap::from_file(&path).unwrap();
-    assert!(grid_map.grid[&dmm::Coord2::new(2, 1)]
-        .prefabs
-        .iter()
-        .any(|p| p.path == "/obj/random/firstaid"));
-    assert!(grid_map.grid[&dmm::Coord2::new(1, 2)]
-        .prefabs
-        .iter()
-        .any(|p| p.path == "/obj/random/finances"));
-    assert!(grid_map.grid[&dmm::Coord2::new(14, 15)]
-        .prefabs
-        .iter()
-        .any(|p| p.path == "/obj/random/handgun"));
-    assert!(grid_map.grid[&dmm::Coord2::new(15, 14)]
-        .prefabs
-        .iter()
-        .any(|p| p.path == "/obj/random/handgun"));
-}
-
 #[test]
 fn to_grid_and_back() {
     for path in all_test_dmm() {
         println!("path: {}", path.display());
 
         let dict_map_original = dmmtools::dmm::Map::from_file(&path).unwrap();
         let grid_map = crate::mapmanip::core::to_grid_map(&dict_map_original);
-        let dict_map_again = crate::mapmanip::core::to_dict_map(&grid_map);
+        let dict_map_again = crate::mapmanip::core::to_dict_map(&grid_map).unwrap();
         let map_str_original = crate::mapmanip::core::map_to_string(&dict_map_original).unwrap();
         let map_str_from_grid = crate::mapmanip::core::map_to_string(&dict_map_again).unwrap();
 
@@ -83,10 +59,11 @@ fn extract() {
         &grid_map_src,
         Coord2::new(4, 7),
         Coord2::new(10, 5),
-    );
+    )
+    .unwrap();
     let grid_map_xtr_expected = crate::mapmanip::core::to_grid_map(&dict_map_xtr_expected);
 
-    let dict_map_xtr = crate::mapmanip::core::to_dict_map(&grid_map_xtr);
+    let dict_map_xtr = crate::mapmanip::core::to_dict_map(&grid_map_xtr).unwrap();
     dict_map_xtr.to_file(path_xtr_out).unwrap();
 
     assert_eq!(
@@ -95,8 +72,8 @@ fn extract() {
     );
 
     for key in grid_map_xtr_expected.grid.keys() {
-        let tile_xtr_expected = grid_map_xtr_expected.grid.get(key).unwrap();
-        let tile_xtr = grid_map_xtr.grid.get(key).unwrap();
+        let tile_xtr_expected = grid_map_xtr_expected.grid.get(&key).unwrap();
+        let tile_xtr = grid_map_xtr.grid.get(&key).unwrap();
         assert_eq!(tile_xtr_expected.prefabs, tile_xtr.prefabs);
     }
 }
@@ -111,16 +88,17 @@ fn insert() {
         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, Coord2::new(6, 4), &mut grid_map_dst);
+    crate::mapmanip::tools::insert_submap(&grid_map_xtr, Coord2::new(6, 4), &mut grid_map_dst)
+        .unwrap();
 
     assert_eq!(
         grid_map_dst_expected.grid.keys().collect::<Vec<_>>(),
         grid_map_dst.grid.keys().collect::<Vec<_>>(),
     );
 
     for key in grid_map_dst_expected.grid.keys() {
-        let tile_dst_expected = grid_map_dst_expected.grid.get(key).unwrap();
-        let tile_dst = grid_map_dst.grid.get(key).unwrap();
+        let tile_dst_expected = grid_map_dst_expected.grid.get(&key).unwrap();
+        let tile_dst = grid_map_dst.grid.get(&key).unwrap();
         assert_eq!(tile_dst_expected.prefabs, tile_dst.prefabs);
     }
 }
@@ -138,12 +116,12 @@ fn keys_deduplicated() {
     for tile in grid_map_out.grid.values_mut() {
         tile.key_suggestion = dmm::Key::default();
     }
-    let dict_map_out = crate::mapmanip::core::to_dict_map(&grid_map_out);
+    let dict_map_out = crate::mapmanip::core::to_dict_map(&grid_map_out).unwrap();
     let grid_map_out = crate::mapmanip::core::to_grid_map(&dict_map_out);
 
     for key in grid_map_src.grid.keys() {
-        let tile_src = grid_map_src.grid.get(key).unwrap();
-        let tile_out = grid_map_out.grid.get(key).unwrap();
+        let tile_src = grid_map_src.grid.get(&key).unwrap();
+        let tile_out = grid_map_out.grid.get(&key).unwrap();
         assert_eq!(tile_src.prefabs, tile_out.prefabs);
     }
 
@@ -152,14 +130,15 @@ fn keys_deduplicated() {
 
 #[test]
 fn mapmanip_configs_parse() {
-    let foo = vec![crate::mapmanip::MapManipulation::InsertExtract {
+    let foo = vec![crate::mapmanip::MapManipulation::SubmapExtractInsert {
         submap_size_x: 1,
         submap_size_y: 2,
         submaps_dmm: "a".to_owned(),
         marker_extract: "b".to_owned(),
         marker_insert: "c".to_owned(),
+        submaps_can_repeat: true,
     }];
-    dbg!(serde_json::to_string(&foo));
+    dbg!(serde_json::to_string(&foo).unwrap());
 
     let mapmanip_configs = walkdir::WalkDir::new("../../maps")
         .into_iter()
@@ -172,3 +151,10 @@ fn mapmanip_configs_parse() {
         let _ = crate::mapmanip::mapmanip_config_parse(&config);
     }
 }
+
+#[test]
+fn mapmanip_configs_execute() {
+    // this is only "unsafe" cause that function is `extern "C"`
+    // it does not do anything actually unsafe
+    unsafe { crate::all_mapmanip_configs_execute_ffi() }
+}

rust/bapi/src/mapmanip/mod.rs

@@ -2,7 +2,6 @@ use crate::mapmanip::core::GridMap;
 use dmmtools::dmm;
 use dmmtools::dmm::Coord2;
 use eyre::ContextCompat;
-use std::collections::BTreeMap;
 
 /// Returns part of map of `xtr_size` and at `xtr_coord` from `src_map`.
 pub fn extract_submap(
@@ -12,7 +11,7 @@ pub fn extract_submap(
 ) -> eyre::Result<GridMap> {
     let mut dst_map = GridMap {
         size: xtr_size.z(1),
-        grid: BTreeMap::new(),
+        grid: crate::mapmanip::core::TileGrid::new(xtr_size.x, xtr_size.y),
     };
 
     for x in 1..(xtr_size.x + 1) {
@@ -27,7 +26,7 @@ pub fn extract_submap(
                     "cannot extract submap; coords out of bounds; x: {src_x}; y: {src_y};"
                 ))?;
 
-            dst_map.grid.insert(Coord2::new(x, y), tile.clone());
+            dst_map.grid.insert(&Coord2::new(x, y), tile.clone());
         }
     }