Completion Part 3

Manual.md

@@ -50,6 +50,8 @@ WorldEdit `//fixlight` command.
 
 ## Commands
 
+TODO: Unify documentation style, provide examples.
+
 ### clone
 
 Usage: `clone --p1 x y z --p2 x y z --offset x y z`
@@ -59,9 +61,9 @@ Clone (copy) the contents of an area to a new location.
 Arguments:
 
 - `--p1, --p2`: Area to clone.
-- `--offset`: Vector to shift the area by. For example, to copy an area 50
+- `--offset x y z`: Vector to shift the area by. For example, to copy an area
-nodes downward (negative Y direction), use `--offset 0 -50 0`. Directions may
+50 nodes downward (negative Y direction), use `--offset 0 -50 0`. Directions
-be determined using Minetest's F5 debug menu.
+may be determined using Minetest's F5 debug menu.
 
 This command copies nodes, param1, param2, and metadata. Nothing will be copied
 from or into mapblocks that are not yet generated.
@@ -92,55 +94,57 @@ contents) are also deleted.
 
 Arguments:
 
-- `--node`: (Optional) Name of node to delete metadata from. If not specified,
+- `--node <node>`: (Optional) Name of node to delete metadata from. If not
-metadata will be deleted from any node.
+specified, metadata will be deleted from any node.
 - `--p1, --p2`: (Optional) Area in which to delete metadata. If not specified,
 metadata will be deleted everywhere.
 - `--invert`: If present, delete metadata *outside* the given area.
 
 ### deleteobjects
 
-Usage: `deleteobjects [--obj <object>] [--items [item]] [--p1 x y z] [--p2 x y z] [--invert]`
+Usage: `deleteobjects [--obj <object>] [--items [items]] [--p1 x y z] [--p2 x y z] [--invert]`
 
-Delete objects/entities, including item entities (dropped items).
+Delete certain objects (entities) and/or item entities (dropped items).
 
 Arguments:
 
-- `--obj`: Name of object to delete, e.g. "boats:boat". If not specified,
+- `--obj <object>`: (Optional) Name of object to delete, e.g. "boats:boat".
-all objects will be deleted.
+- `--items [items]`: If present, delete only item entities (dropped items). If
-- `--items [item]`: Delete item entities (dropped items). If an optional item
+one or more item names are listed after `--items`, only those items will be
-name is specified, only items with that name will be deleted.
+deleted.
-- `--p1, --p2`: Area in which to delete objects. If not specified, objects will
+- `--p1, --p2`: (Optional) Area in which to delete objects. If not specified,
-be deleted everywhere.
+objects will be deleted everywhere.
-- `--invert`: Delete objects *outside* the given area.
+- `--invert`: If present, delete objects *outside* the given area.
+
+`--obj` and `--items` cannot be used simultaneously.
 
 ### deletetimers
 
 Usage: `deletetimers [--node <node>] [--p1 x y z] [--p2 x y z] [--invert]`
 
-Delete node timers of a certain node and/or within a certain area.
+Delete node timers of certain nodes.
 
 Arguments:
 
-- `--node`: Name of node to modify. If not specified, the node timers of all
+- `--node <node>`: If specified, only delete node timers from nodes with the
-nodes will be deleted.
+given name.
-- `--p1, --p2`: Area in which to delete node timers.
+- `--p1, --p2`: (Optional) Area in which to delete node timers.
-- `--invert`: Only delete node timers *outside* the given area.
+- `--invert`: Delete node timers *outside* the given area.
 
 ### fill
 
 Usage: `fill --p1 x y z --p2 x y z [--invert] <new_node>`
 
-Fills the given area with one node. The affected mapblocks must be already
+Fills everything inside or outside an area with one node. Mapblocks that are
-generated for fill to work.
+not yet generated will not be affected.
 
 This command does not affect param2, node metadata, etc.
 
 Arguments:
 
-- `new_node`: Name of node to fill the area with.
 - `--p1, --p2`: Area to fill.
-- `--invert`: Fill everything *outside* the given area.
+- `--invert`: Fill all generated nodes *outside* the given area.
+- `<new_node>`: Name of node to fill the area with.
 
 ### overlay
 
@@ -150,12 +154,12 @@ Copy part or all of a source map into the main map.
 
 Arguments:
 
-- `input_map`: Path to source map/world. This will not be modified.
+- `<input_map>`: Path to the source map/world. This world will not be modified.
 - `--p1, --p2`: Area to copy from. If not specified, MapEditr will try to
-copy everything from the input map file.
+copy everything from the source map.
 - `--invert`: If present, copy everything *outside* the given area.
-- `--offset`: Offset to move nodes by when copying; default is no offset.
+- `--offset x y z`: Vector to shift nodes by when copying; default is no
-Currently, an offset cannot be used with an inverted selection.
+offset. Currently, an offset cannot be used with an inverted selection.
 
 This command will always copy nodes, param1, param2, and metadata. If no
 offset is used, objects/entities and node timers may also be copied.
@@ -168,8 +172,7 @@ mapblocks can be copied verbatim.
 
 ### replaceininv
 
-Usage: `replaceininv [--delete] [--deletemeta] [--nodes <nodes>] [--p1 x y z]
+Usage: `replaceininv [--delete] [--deletemeta] [--nodes <nodes>] [--p1 x y z] [--p2 x y z] [--invert] <item> [new_item]`
-[--p2 x y z] [--invert] <item> [new_item]`
 
 Replace or delete certain items in node inventories.
 

src/block_utils.rs

@@ -1,7 +1,6 @@
-// TODO: Move this file somewhere else?
 use std::collections::BTreeMap;
 
-use crate::map_block::{MapBlock, NodeMetadataList};
+use crate::map_block::{MapBlock, NodeMetadataList, NameIdMap};
 use crate::spatial::{Vec3, Area};
 
 
@@ -81,7 +80,7 @@ pub fn merge_metadata(
 	// Warning: diff can be negative!
 	let diff = offset.x + offset.y * 16 + offset.z * 256;
 
-	// Delete any existing metadata in the destination block
+	// Delete any existing metadata in the destination area.
 	let mut to_delete = Vec::with_capacity(dst_meta.len());
 	for (&idx, _) in dst_meta.iter() {
 		let pos = Vec3::from_u16_key(idx);
@@ -115,27 +114,26 @@ pub fn clean_name_id_map(block: &mut MapBlock) {
 	}
 
 	// Rebuild the name-ID map.
-	let mut new_nimap = BTreeMap::new();
+	let mut new_nimap = NameIdMap(BTreeMap::new());
-	let mut map = vec![0u16; id_count];
+	let mut map = vec![0u16; id_count]; // map[old_node_id] == new_node_id
-	for id in 0..id_count {
+	for (&id, name) in &block.nimap.0 {
 		// Skip unused IDs.
-		if !used[id] {
+		if !used[id as usize] {
 			continue;
 		}
 
-		let name = &block.nimap.0[&(id as u16)];
+		if let Some(first_id) = new_nimap.get_id(&name) {
-		if let Some(first_id) = new_nimap.iter().position(|(_, v)| v == name) {
 			// Name is already in the map; map old, duplicate ID to the
 			// existing ID.
-			map[id] = first_id as u16;
+			map[id as usize] = first_id as u16;
 		} else {
 			// Name is not yet in the map; assign it to the next ID.
-			new_nimap.insert(new_nimap.len() as u16, name.clone());
+			new_nimap.0.insert(new_nimap.0.len() as u16, name.clone());
 			// Map old ID to newly-inserted ID.
-			map[id] = new_nimap.len() as u16 - 1;
+			map[id as usize] = new_nimap.0.len() as u16 - 1;
 		}
 	}
-	block.nimap.0 = new_nimap;
+	block.nimap = new_nimap;
 
 	// Re-assign node IDs.
 	for id in &mut nd.nodes {

src/cmd_line.rs

@@ -92,8 +92,7 @@ fn to_cmd_line_args<'a>(tup: &(ArgType, &'a str))
 		ArgType::Items =>
 			Arg::with_name("items")
 				.long("items")
-				.min_values(0)
+				.min_values(0),
-				.max_values(1),
 		ArgType::NewItem =>
 			Arg::with_name("new_item")
 				.takes_value(true),
@@ -252,6 +251,7 @@ fn print_log(log_type: LogType, msg: String) {
 
 fn get_confirmation() -> bool {
 	print!("Proceed? (Y/n): ");
+	std::io::stdout().flush().unwrap();
 	let mut result = String::new();
 	std::io::stdin().read_line(&mut result).unwrap();
 	result.trim().to_ascii_lowercase() == "y"

src/commands/delete_objects.rs

@@ -1,48 +1,77 @@
-use super::Command;
+use super::{Command, ArgResult};
 
 use crate::unwrap_or;
 use crate::spatial::Area;
-use crate::instance::{ArgType, InstBundle};
+use crate::instance::{ArgType, InstArgs, InstBundle};
 use crate::map_block::{MapBlock, StaticObject, LuaEntityData};
 use crate::utils::{query_keys, to_bytes, to_slice, fmt_big_num};
 
 use memmem::{Searcher, TwoWaySearcher};
 
 const ITEM_ENT_NAME: &[u8] = b"__builtin:item";
+const ITEM_NAME_PAT: &[u8] = b"[\"itemstring\"] = \"";
+
+
+fn verify_args(args: &InstArgs) -> ArgResult {
+	if args.object.is_some() && args.items.is_some() {
+		return ArgResult::error("Cannot use both --obj and --items.");
+	}
+	ArgResult::Ok
+}
 
 
 #[inline]
+fn get_item_name<'a>(data: &'a [u8], searcher: &TwoWaySearcher) -> &'a[u8] {
+	if data.starts_with(b"return") {
+		if let Some(idx) = searcher.search_in(data) {
+			let name = &data[idx + ITEM_NAME_PAT.len()..]
+				.split(|&c| c == b' ' || c == b'"').next();
+			if let Some(n) = name {
+				return n;
+			}
+		}
+		b""
+	} else {
+		data
+	}
+}
+
+
 fn can_delete(
 	obj: &StaticObject,
 	area: &Option<Area>,
 	invert: bool,
 	obj_name: &Option<Vec<u8>>,
-	item_searcher: &Option<TwoWaySearcher>
+	item_names: &[Vec<u8>],
+	item_name_searcher: &TwoWaySearcher
 ) -> bool {
 	// Check area requirement
 	if let Some(a) = area {
 		const DIV_FAC: i32 = 10_000;
-		let rounded_pos = obj.f_pos.map(
+		let rounded_pos = obj.f_pos
-			|v| (v - DIV_FAC / 2).div_euclid(DIV_FAC));
+			.map(|v| (v + DIV_FAC / 2).div_euclid(DIV_FAC));
 		if a.contains(rounded_pos) == invert {
 			return false; // Object not included in area.
 		}
 	}
 
-	// Check name requirement
+	// Check name requirements
-	if let Some(n) = obj_name {
+	if let Some(name) = obj_name {
 		if let Ok(le_data) = LuaEntityData::deserialize(obj) {
-			if &le_data.name != n {
+			if &le_data.name != name {
 				return false; // Object name does not match.
 			}
 
-			if let Some(is) = item_searcher {
+			if !item_names.is_empty() {
-				if is.search_in(&le_data.data).is_none() {
+				let item_name =
-					return false; // Item entity name does not match.
+					get_item_name(&le_data.data, &item_name_searcher);
+				if !item_names.iter().any(|n| n == item_name) {
+					// Item entity's item name does not match.
+					return false
 				}
 			}
 		} else {
-			return false; // Unsupported object type, don't delete it.
+			return false; // Keep invalid or unsupported objects.
 		}
 	}
 
@@ -51,20 +80,18 @@ fn can_delete(
 
 
 fn delete_objects(inst: &mut InstBundle) {
-	let search_obj = if inst.args.items.is_some() {
+	let obj_name = if inst.args.items.is_some() {
 		Some(ITEM_ENT_NAME.to_owned())
 	} else {
 		inst.args.object.as_ref().map(to_bytes)
 	};
 
-	// search_item will be Some if (1) item search is enabled and (2) an item
+	let item_names: Vec<_> = inst.args.items.as_ref().unwrap_or(&Vec::new())
-	// is specified.
+		.iter().map(to_bytes).collect();
-	let search_item = inst.args.items.as_ref().and_then(|items| items.get(0))
+	let item_name_searcher = TwoWaySearcher::new(ITEM_NAME_PAT);
-		.map(to_bytes);
-	let item_searcher = search_item.as_ref().map(|s| TwoWaySearcher::new(s));
 
 	let keys = query_keys(&mut inst.db, &mut inst.status,
-		to_slice(&search_obj), inst.args.area, inst.args.invert, true);
+		to_slice(&obj_name), inst.args.area, inst.args.invert, true);
 
 	inst.status.begin_editing();
 	let mut count: u64 = 0;
@@ -75,13 +102,14 @@ fn delete_objects(inst: &mut InstBundle) {
 			{ inst.status.inc_failed(); continue; });
 
 		let mut modified = false;
-		for i in (0..block.static_objects.len()).rev() {
+		for i in (0 .. block.static_objects.len()).rev() {
 			if can_delete(
 				&block.static_objects[i],
 				&inst.args.area,
 				inst.args.invert,
-				&search_obj,
+				&obj_name,
-				&item_searcher
+				&item_names,
+				&item_name_searcher
 			) {
 				block.static_objects.remove(i);
 				modified = true;
@@ -102,17 +130,38 @@ fn delete_objects(inst: &mut InstBundle) {
 pub fn get_command() -> Command {
 	Command {
 		func: delete_objects,
-		verify_args: None,
+		verify_args: Some(verify_args),
 		args: vec![
 			(ArgType::Area(false), "Area in which to delete objects"),
-			(ArgType::Invert, "Delete all objects outside the area"),
+			(ArgType::Invert,
-			(ArgType::Object,
+					"If present, delete objects *outside* the given area."),
-				"Name of object to delete. If not specified, all objects will \
+			(ArgType::Object, "Name of object to delete"),
-				be deleted"),
 			(ArgType::Items,
-				"Delete item entities. Optionally specify an item name to \
+				"If present, delete item entities. Optionally list one or \
-				delete."),
+				more item names after `--items` to delete only those items."),
 		],
-		help: "Delete certain objects (entities)."
+		help: "Delete certain objects and/or item entities."
+	}
+}
+
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+
+	#[test]
+	fn test_delete_objects() {
+		let searcher = TwoWaySearcher::new(ITEM_NAME_PAT);
+		let pairs: &[(&[u8], &[u8])] = &[
+			(b"default:glass", b"default:glass"),
+			(b"return {}", b""),
+			(b"return {[\"itemstring\"] = \"\", [\"age\"] = 100}", b""),
+			(b"return {[\"itemstring\"] = \"mod:item\"}", b"mod:item"),
+			(b"return {[\"age\"] = 400, [\"itemstring\"] = \"one:two 99 32\"}",
+				b"one:two"),
+		];
+		for &(data, name) in pairs {
+			assert_eq!(get_item_name(data, &searcher), name);
+		}
 	}
 }

src/commands/delete_timers.rs

@@ -19,7 +19,8 @@ fn delete_timers(inst: &mut InstBundle) {
 	for key in keys {
 		inst.status.inc_done();
 		let data = inst.db.get_block(key).unwrap();
-		let mut block = unwrap_or!(MapBlock::deserialize(&data), continue);
+		let mut block = unwrap_or!(MapBlock::deserialize(&data),
+			{ inst.status.inc_failed(); continue; });
 
 		let node_id = node.as_deref().and_then(|n| block.nimap.get_id(n));
 		if node.is_some() && node_id.is_none() {
@@ -68,11 +69,11 @@ pub fn get_command() -> Command {
 		verify_args: None,
 		args: vec![
 			(ArgType::Area(false), "Area in which to delete timers"),
-			(ArgType::Invert, "Delete all timers outside the given area."),
+			(ArgType::Invert, "Delete node timers *outside* the given area."),
 			(ArgType::Node(false),
-				"Node to delete timers from. If not specified, all node \
+				"Node to delete timers from. If not specified, node timers \
-				timers will be deleted.")
+				will be deleted from any node.")
 		],
-		help: "Delete node timers."
+		help: "Delete node timers of certain nodes."
 	}
 }

src/commands/fill.rs

@@ -44,10 +44,8 @@ fn fill(inst: &mut InstBundle) {
 
 		let pos = Vec3::from_block_key(key);
 		let data = inst.db.get_block(key).unwrap();
-		let mut block = unwrap_or!(MapBlock::deserialize(&data), {
+		let mut block = unwrap_or!(MapBlock::deserialize(&data),
-			inst.status.inc_failed();
+			{ inst.status.inc_failed(); continue; });
-			continue;
-		});
 
 		if area.contains_block(pos) != area.touches_block(pos) {
 			// Fill part of block
@@ -82,8 +80,9 @@ pub fn get_command() -> Command {
 		verify_args: None,
 		args: vec![
 			(ArgType::Area(true), "Area to fill"),
-			(ArgType::NewNode, "Name of node to fill area with"),
+			(ArgType::Invert,
-			(ArgType::Invert, "Fill all generated areas outside the area.")
+				"Fill all generated nodes *outside* the given area."),
+			(ArgType::NewNode, "Name of node to fill the area with"),
 		],
 		help: "Fill the entire area with one node."
 	}

src/commands/overlay.rs

@@ -167,8 +167,9 @@ fn overlay_with_offset(inst: &mut InstBundle) {
 		);
 
 		let dst_part_abs = dst_area.map_or(
+			// If no area is given, the destination part is the whole mapblock.
 			Area::new(dst_pos * 16, dst_pos * 16 + 15),
-			|ref a| a.abs_block_overlap(dst_pos).unwrap()
+			|a| a.abs_block_overlap(dst_pos).unwrap()
 		);
 		let src_part_abs = dst_part_abs - offset;
 		let src_blocks_needed = src_part_abs.to_touching_block_area();
@@ -224,11 +225,13 @@ pub fn get_command() -> Command {
 		func: overlay,
 		verify_args: Some(verify_args),
 		args: vec![
-			(ArgType::InputMapPath, "Path to input map file"),
+			(ArgType::InputMapPath, "Path to the source map/world"),
-			(ArgType::Area(false), "Area to overlay"),
+			(ArgType::Area(false), "Area to copy from. If not specified, \
-			(ArgType::Invert, "Overlay all nodes outside the given area"),
+				everything from the source map will be copied."),
-			(ArgType::Offset(false), "Vector to offset nodes by"),
+			(ArgType::Invert,
+				"If present, copy everything *outside* the given area."),
+			(ArgType::Offset(false), "Vector to shift nodes by when copying"),
 		],
-		help: "Copy part or all of one world/map into another."
+		help: "Copy part or all of a source map into the main map."
 	}
 }

src/commands/replace_in_inv.rs

@@ -79,7 +79,8 @@ fn replace_in_inv(inst: &mut InstBundle) {
 	for key in keys {
 		inst.status.inc_done();
 		let data = inst.db.get_block(key).unwrap();
-		let mut block = unwrap_or!(MapBlock::deserialize(&data), continue);
+		let mut block = unwrap_or!(MapBlock::deserialize(&data),
+			{ inst.status.inc_failed(); continue; });
 
 		let node_data = block.node_data.get_ref();
 		let node_ids: Vec<_> = nodes.iter()
@@ -89,7 +90,8 @@ fn replace_in_inv(inst: &mut InstBundle) {
 		}
 
 		let mut meta = unwrap_or!(
-			NodeMetadataList::deserialize(block.metadata.get_ref()), continue);
+			NodeMetadataList::deserialize(block.metadata.get_ref()),
+			{ inst.status.inc_failed(); continue; });
 
 		let block_corner = Vec3::from_block_key(key) * 16;
 		let mut modified = false;

src/commands/set_meta_var.rs

@@ -8,7 +8,7 @@ use crate::utils::{query_keys, to_bytes, fmt_big_num};
 
 
 fn set_meta_var(inst: &mut InstBundle) {
-	// TODO: Bytes input
+	// TODO: Bytes input, create/delete variables
 	let key = to_bytes(inst.args.key.as_ref().unwrap());
 	let value = to_bytes(inst.args.value.as_ref().unwrap());
 	let nodes: Vec<_> = inst.args.nodes.iter().map(to_bytes).collect();
@@ -22,7 +22,8 @@ fn set_meta_var(inst: &mut InstBundle) {
 	for block_key in keys {
 		inst.status.inc_done();
 		let data = inst.db.get_block(block_key).unwrap();
-		let mut block = unwrap_or!(MapBlock::deserialize(&data), continue);
+		let mut block = unwrap_or!(MapBlock::deserialize(&data),
+			{ inst.status.inc_failed(); continue; });
 
 		let node_data = block.node_data.get_ref();
 		let node_ids: Vec<_> = nodes.iter()
@@ -32,7 +33,8 @@ fn set_meta_var(inst: &mut InstBundle) {
 		}
 
 		let mut meta = unwrap_or!(
-			NodeMetadataList::deserialize(block.metadata.get_ref()), continue);
+			NodeMetadataList::deserialize(block.metadata.get_ref()),
+			{ inst.status.inc_failed(); continue; });
 
 		let block_corner = Vec3::from_block_key(block_key) * 16;
 		let mut modified = false;

src/instance.rs

@@ -140,7 +140,6 @@ impl StatusServer {
 	}
 
 	pub fn inc_failed(&mut self) {
-		// TODO: Proper error handling for all commands.
 		self.status.lock().unwrap().blocks_failed += 1;
 	}
 

src/main.rs

@@ -10,7 +10,6 @@ mod cmd_line;
 
 
 // TODO: Check for unnecessary #derives!
-// TODO: Check mapedit TODOs and implement what's needed.
 fn main() {
 	// TODO: Add GUI. hmm...
 	cmd_line::run_cmd_line();