From b0f551c307c27481f0e30ac54e50ddbcc85e6ed7 Mon Sep 17 00:00:00 2001
From: Thomas Harte <thomas.harte@gmail.com>
Date: Sat, 5 Jun 2021 21:09:35 -0400
Subject: [PATCH] Ensures only _missing_ ROMs are reported.

---
 Machines/Utility/ROMCatalogue.cpp | 43 +++++++++++++++++++++++++++++--
 Machines/Utility/ROMCatalogue.hpp | 10 ++++---
 OSBindings/SDL/main.cpp           | 11 ++++----
 3 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/Machines/Utility/ROMCatalogue.cpp b/Machines/Utility/ROMCatalogue.cpp
index 93607a66d..5a1181bfd 100644
--- a/Machines/Utility/ROMCatalogue.cpp
+++ b/Machines/Utility/ROMCatalogue.cpp
@@ -59,11 +59,50 @@ void Request::Node::add_descriptions(std::vector<Description> &result) const {
 		return;
 	}
 
-	for(const auto &node: children) {
-		node.add_descriptions(result);
+	for(const auto &child: children) {
+		child.add_descriptions(result);
 	}
 }
 
+Request Request::subtract(const ROM::Map &map) const {
+	Request copy(*this);
+	if(copy.node.subtract(map)) {
+		copy.node.name = Name::None;
+		copy.node.type = Node::Type::One;
+	}
+	return copy;
+}
+
+bool Request::Node::subtract(const ROM::Map &map) {
+	switch(type) {
+		case Type::One:
+		return map.find(name) != map.end();
+
+		default: {
+			bool has_all = true;
+			bool has_any = false;
+
+			auto iterator = children.begin();
+			while(iterator != children.end()) {
+				const bool did_subtract = iterator->subtract(map);
+				has_all &= did_subtract;
+				has_any |= did_subtract;
+				if(did_subtract) {
+					iterator = children.erase(iterator);
+				} else {
+					++iterator;
+				}
+			}
+
+			return (type == Type::All && has_all) || (type == Type::Any && has_any);
+		}
+	}
+}
+
+bool Request::empty() {
+	return node.type == Node::Type::One && node.name == Name::None;
+}
+
 bool Request::Node::validate(Map &map) const {
 	// Leaf nodes are easy: check that the named ROM is present,
 	// unless it's optional, in which case it is always valid.
diff --git a/Machines/Utility/ROMCatalogue.hpp b/Machines/Utility/ROMCatalogue.hpp
index 634ddc7a5..0cb759507 100644
--- a/Machines/Utility/ROMCatalogue.hpp
+++ b/Machines/Utility/ROMCatalogue.hpp
@@ -18,7 +18,7 @@
 namespace ROM {
 
 enum Name {
-	Invalid,
+	None,
 
 	// Acorn.
 	AcornBASICII,
@@ -121,7 +121,7 @@ using Map = std::map<ROM::Name, std::vector<uint8_t>>;
 
 struct Description {
 	/// The ROM's enum name.
-	Name name = Name::Invalid;
+	Name name = Name::None;
 	/// The machine with which this ROM is associated, in a form that is safe for using as
 	/// part of a file name.
 	std::string machine_name;
@@ -177,13 +177,16 @@ struct Request {
 		const std::function<void(ROM::Request::ListType type, const ROM::Description &, bool is_optional, size_t remaining)> &add_item
 	) const;
 
+	Request subtract(const ROM::Map &map) const;
+	bool empty();
+
 	private:
 		struct Node {
 			enum class Type {
 				Any, All, One
 			};
 			Type type = Type::One;
-			Name name = Name::Invalid;
+			Name name = Name::None;
 			/// @c true if this ROM is optional for machine startup. Generally indicates something
 			/// that would make emulation more accurate, but not sufficiently so to make it
 			/// a necessity.
@@ -197,6 +200,7 @@ struct Request {
 				const std::function<void(void)> &exit_list,
 				const std::function<void(ROM::Request::ListType type, const ROM::Description &, bool is_optional, size_t remaining)> &add_item
 			) const;
+			bool subtract(const ROM::Map &map);
 		};
 		Node node;
 
diff --git a/OSBindings/SDL/main.cpp b/OSBindings/SDL/main.cpp
index dfdf84377..5b73c5b5d 100644
--- a/OSBindings/SDL/main.cpp
+++ b/OSBindings/SDL/main.cpp
@@ -694,12 +694,10 @@ int main(int argc, char *argv[]) {
 	//	/usr/local/share/CLK/[system];
 	//	/usr/share/CLK/[system]; or
 	//	[user-supplied path]/[system]
-	ROM::Request requested_roms;
+	ROM::Request missing_roms;
 	std::vector<std::string> checked_paths;
-	ROMMachine::ROMFetcher rom_fetcher = [&requested_roms, &arguments, &checked_paths]
+	ROMMachine::ROMFetcher rom_fetcher = [&missing_roms, &arguments, &checked_paths]
 		(const ROM::Request &roms) -> ROM::Map {
-			requested_roms = roms;
-
 			std::vector<std::string> paths = {
 				"/usr/local/share/CLK/",
 				"/usr/share/CLK/"
@@ -756,6 +754,7 @@ int main(int argc, char *argv[]) {
 				}
 			}
 
+			missing_roms = roms.subtract(results);
 			return results;
 		};
 
@@ -775,7 +774,7 @@ int main(int argc, char *argv[]) {
 			default: break;
 			case ::Machine::Error::MissingROM: {
 				std::cerr << "Could not find system ROMs; please install to /usr/local/share/CLK/ or /usr/share/CLK/, or provide a --rompath, e.g. --rompath=~/ROMs." << std::endl;
-				std::cerr << "Needed ";
+				std::cerr << "Needed — but didn't find — ";
 
 				int indentation_level = 0;
 				const auto indent = [&indentation_level] {
@@ -786,7 +785,7 @@ int main(int argc, char *argv[]) {
 					}
 				};
 
-				requested_roms.visit([&indentation_level, indent] (ROM::Request::ListType type) {
+				missing_roms.visit([&indentation_level, indent] (ROM::Request::ListType type) {
 					indent();
 					switch(type) {
 						default: