Gives MachineForTargets complete responsibility for initial machine state.

Machines/ROMMachine.hpp

@@ -15,11 +15,13 @@
 
 namespace ROMMachine {
 
+typedef std::function<std::vector<std::unique_ptr<std::vector<uint8_t>>>(const std::string &machine, const std::vector<std::string> &names)> ROMFetcher;
+
 struct Machine {
 	/*!
 		Provides the machine with a way to obtain such ROMs as it needs.
 	*/
-	virtual bool set_rom_fetcher(const std::function<std::vector<std::unique_ptr<std::vector<uint8_t>>>(const std::string &machine, const std::vector<std::string> &names)> &rom_with_name) { return true; }
+	virtual bool set_rom_fetcher(const ROMFetcher &rom_with_name) { return true; }
 };
 
 }

Machines/Utility/MachineForTarget.cpp

@@ -18,19 +18,41 @@
 
 #include "TypedDynamicMachine.hpp"
 
-::Machine::DynamicMachine *::Machine::MachineForTargets(const std::vector<std::unique_ptr<Analyser::Static::Target>> &targets) {
+::Machine::DynamicMachine *::Machine::MachineForTargets(const std::vector<std::unique_ptr<Analyser::Static::Target>> &targets, const ROMMachine::ROMFetcher &rom_fetcher, Error &error) {
 	// TODO: deal with target lists containing more than one machine.
-	switch(targets.front()->machine) {
-		case Analyser::Machine::AmstradCPC:	return new TypedDynamicMachine<AmstradCPC::Machine>(AmstradCPC::Machine::AmstradCPC());
-		case Analyser::Machine::Atari2600:	return new TypedDynamicMachine<Atari2600::Machine>(Atari2600::Machine::Atari2600());
-		case Analyser::Machine::Electron:	return new TypedDynamicMachine<Electron::Machine>(Electron::Machine::Electron());
-		case Analyser::Machine::MSX:		return new TypedDynamicMachine<MSX::Machine>(MSX::Machine::MSX());
-		case Analyser::Machine::Oric:		return new TypedDynamicMachine<Oric::Machine>(Oric::Machine::Oric());
-		case Analyser::Machine::Vic20:		return new TypedDynamicMachine<Commodore::Vic20::Machine>(Commodore::Vic20::Machine::Vic20());
-		case Analyser::Machine::ZX8081:		return new TypedDynamicMachine<ZX8081::Machine>(ZX8081::Machine::ZX8081(*targets.front()));
 
-		default:	return nullptr;
+	error = Error::None;
+	::Machine::DynamicMachine *machine = nullptr;
+	switch(targets.front()->machine) {
+		case Analyser::Machine::AmstradCPC:	machine = new TypedDynamicMachine<AmstradCPC::Machine>(AmstradCPC::Machine::AmstradCPC());			break;
+		case Analyser::Machine::Atari2600:	machine = new TypedDynamicMachine<Atari2600::Machine>(Atari2600::Machine::Atari2600());				break;
+		case Analyser::Machine::Electron:	machine = new TypedDynamicMachine<Electron::Machine>(Electron::Machine::Electron());				break;
+		case Analyser::Machine::MSX:		machine = new TypedDynamicMachine<MSX::Machine>(MSX::Machine::MSX());								break;
+		case Analyser::Machine::Oric:		machine = new TypedDynamicMachine<Oric::Machine>(Oric::Machine::Oric());							break;
+		case Analyser::Machine::Vic20:		machine = new TypedDynamicMachine<Commodore::Vic20::Machine>(Commodore::Vic20::Machine::Vic20());	break;
+		case Analyser::Machine::ZX8081:		machine = new TypedDynamicMachine<ZX8081::Machine>(ZX8081::Machine::ZX8081(*targets.front()));		break;
+
+		default:
+			error = Error::UnknownMachine;
+		return nullptr;
 	}
+
+	// TODO: this shouldn't depend on CRT machine's inclusion of ROM machine.
+	CRTMachine::Machine *crt_machine = machine->crt_machine();
+	if(crt_machine) {
+		if(!machine->crt_machine()->set_rom_fetcher(rom_fetcher)) {
+			delete machine;
+			error = Error::MissingROM;
+			return nullptr;
+		}
+	}
+
+	ConfigurationTarget::Machine *configuration_target = machine->configuration_target();
+	if(configuration_target) {
+		machine->configuration_target()->configure_as_target(*targets.front());
+	}
+
+	return machine;
 }
 
 std::string Machine::ShortNameForTargetMachine(const Analyser::Machine machine) {

Machines/Utility/MachineForTarget.hpp

@@ -29,6 +29,7 @@ namespace Machine {
 	the machine's parent class or, therefore, the need to establish a common one.
 */
 struct DynamicMachine {
+	virtual ~DynamicMachine() {}
 	virtual ConfigurationTarget::Machine *configuration_target() = 0;
 	virtual CRTMachine::Machine *crt_machine() = 0;
 	virtual JoystickMachine::Machine *joystick_machine() = 0;
@@ -37,12 +38,18 @@ struct DynamicMachine {
 	virtual Utility::TypeRecipient *type_recipient() = 0;
 };
 
+enum class Error {
+	None,
+	UnknownMachine,
+	MissingROM
+};
+
 /*!
 	Allocates an instance of DynamicMachine holding a machine that can
 	receive the supplied static analyser result. The machine has been allocated
 	on the heap. It is the caller's responsibility to delete the class when finished.
 */
-DynamicMachine *MachineForTargets(const std::vector<std::unique_ptr<Analyser::Static::Target>> &targets);
+DynamicMachine *MachineForTargets(const std::vector<std::unique_ptr<Analyser::Static::Target>> &targets, const ::ROMMachine::ROMFetcher &rom_fetcher, Error &error);
 
 /*!
 	Returns a short string name for the machine identified by the target,

OSBindings/Mac/Clock Signal/Machine/CSMachine+Target.h

@@ -12,7 +12,6 @@
 
 @interface CSMachine(Target)
 
-- (void)applyTarget:(const Analyser::Static::Target &)target;
 - (void)applyMedia:(const Analyser::Static::Media &)media;
 
 @end

OSBindings/Mac/Clock Signal/Machine/CSMachine.mm

@@ -71,7 +71,11 @@ struct MachineDelegate: CRTMachine::Machine::Delegate, public LockProtectedDeleg
 	self = [super init];
 	if(self) {
 		_analyser = result;
-		_machine.reset(Machine::MachineForTargets(_analyser.targets));
+
+		Machine::Error error;
+		_machine.reset(Machine::MachineForTargets(_analyser.targets, CSROMFetcher(), error));
+		if(!_machine) return nil;
+
 		_delegateMachineAccessLock = [[NSLock alloc] init];
 
 		_machineDelegate.machine = self;
@@ -80,9 +84,6 @@ struct MachineDelegate: CRTMachine::Machine::Delegate, public LockProtectedDeleg
 		_speakerDelegate.machineAccessLock = _delegateMachineAccessLock;
 
 		_machine->crt_machine()->set_delegate(&_machineDelegate);
-		CSApplyROMFetcher(*_machine->crt_machine());
-
-		[self applyTarget:*_analyser.targets.front()];
 	}
 	return self;
 }
@@ -185,13 +186,6 @@ struct MachineDelegate: CRTMachine::Machine::Delegate, public LockProtectedDeleg
 		keyboardMachine->type_string([paste UTF8String]);
 }
 
-- (void)applyTarget:(const Analyser::Static::Target &)target {
-	@synchronized(self) {
-		ConfigurationTarget::Machine *const configurationTarget = _machine->configuration_target();
-		if(configurationTarget) configurationTarget->configure_as_target(target);
-	}
-}
-
 - (void)applyMedia:(const Analyser::Static::Media &)media {
 	@synchronized(self) {
 		ConfigurationTarget::Machine *const configurationTarget = _machine->configuration_target();

OSBindings/Mac/Clock Signal/Machine/CSROMFetcher.hpp

@@ -8,4 +8,4 @@
 
 #include "ROMMachine.hpp"
 
-void CSApplyROMFetcher(ROMMachine::Machine &rom_machine);
+ROMMachine::ROMFetcher CSROMFetcher();

OSBindings/Mac/Clock Signal/Machine/CSROMFetcher.mm

@@ -14,8 +14,8 @@
 
 #include <string>
 
-void CSApplyROMFetcher(ROMMachine::Machine &rom_machine) {
+ROMMachine::ROMFetcher CSROMFetcher() {
-	rom_machine.set_rom_fetcher( [] (const std::string &machine, const std::vector<std::string> &names) -> std::vector<std::unique_ptr<std::vector<std::uint8_t>>> {
+	return [] (const std::string &machine, const std::vector<std::string> &names) -> std::vector<std::unique_ptr<std::vector<std::uint8_t>>> {
 		NSString *subDirectory = [@"ROMImages/" stringByAppendingString:[NSString stringWithUTF8String:machine.c_str()]];
 		std::vector<std::unique_ptr<std::vector<std::uint8_t>>> results;
 		for(auto &name: names) {
@@ -31,5 +31,5 @@ void CSApplyROMFetcher(ROMMachine::Machine &rom_machine) {
 		}
 
 		return results;
-	});
+	};
 }

OSBindings/Mac/Clock SignalTests/AtariStaticAnalyserTests.mm

@@ -9,7 +9,7 @@
 #import <XCTest/XCTest.h>
 
 #import <CommonCrypto/CommonDigest.h>
-#include "../../../StaticAnalyser/StaticAnalyser.hpp"
+#include "../../../Analyser/Static/StaticAnalyser.hpp"
 
 @interface AtariROMRecord : NSObject
 @property(nonatomic, readonly) Analyser::Static::Atari2600PagingModel pagingModel;
@@ -591,15 +591,15 @@ static NSDictionary<NSString *, AtariROMRecord *> *romRecordsBySHA1 = @{
 		for(int c = 0; c < CC_SHA1_DIGEST_LENGTH; c++) [sha1 appendFormat:@"%02x", sha1Bytes[c]];
 
 		// get an analysis of the file
-		std::list<Analyser::Static::Target> targets = Analyser::Static::GetTargets([fullPath UTF8String]);
+		std::vector<std::unique_ptr<Analyser::Static::Target>> targets = Analyser::Static::GetTargets([fullPath UTF8String]);
 
 		// grab the ROM record
 		AtariROMRecord *romRecord = romRecordsBySHA1[sha1];
 		if(!romRecord) continue;
 
 		// assert equality
-		XCTAssert(targets.front().atari.paging_model == romRecord.pagingModel, @"%@; should be %d, is %d", testFile, romRecord.pagingModel, targets.front().atari.paging_model);
+		XCTAssert(targets.front()->atari.paging_model == romRecord.pagingModel, @"%@; should be %d, is %d", testFile, romRecord.pagingModel, targets.front()->atari.paging_model);
-		XCTAssert(targets.front().atari.uses_superchip == romRecord.usesSuperchip, @"%@; should be %@", testFile, romRecord.usesSuperchip ? @"true" : @"false");
+		XCTAssert(targets.front()->atari.uses_superchip == romRecord.usesSuperchip, @"%@; should be %@", testFile, romRecord.usesSuperchip ? @"true" : @"false");
 	}
 }
 

OSBindings/Mac/Clock SignalTests/Bridges/C1540Bridge.mm

@@ -33,7 +33,7 @@ class VanillaSerialPort: public Commodore::Serial::Port {
 		_serialPort.reset(new VanillaSerialPort);
 
 		_c1540.reset(new Commodore::C1540::Machine(Commodore::C1540::Machine::C1540));
-		CSApplyROMFetcher(*_c1540);
+		_c1540->set_rom_fetcher(CSROMFetcher());
 		_c1540->set_serial_bus(_serialBus);
 		Commodore::Serial::AttachPortAndBus(_serialPort, _serialBus);
 	}

OSBindings/Mac/Clock SignalTests/MSXStaticAnalyserTests.mm

@@ -9,7 +9,7 @@
 #import <XCTest/XCTest.h>
 
 #import <CommonCrypto/CommonDigest.h>
-#include "../../../StaticAnalyser/StaticAnalyser.hpp"
+#include "../../../Analyser/Static/StaticAnalyser.hpp"
 
 @interface MSXROMRecord : NSObject
 @property(nonatomic, readonly) Analyser::Static::MSXCartridgeType cartridgeType;
@@ -211,7 +211,7 @@ static NSDictionary<NSString *, MSXROMRecord *> *romRecordsBySHA1 = @{
 		for(int c = 0; c < CC_SHA1_DIGEST_LENGTH; c++) [sha1 appendFormat:@"%02x", sha1Bytes[c]];
 
 		// get an analysis of the file
-		std::list<Analyser::Static::Target> targets = Analyser::Static::GetTargets([fullPath UTF8String]);
+		std::vector<std::unique_ptr<Analyser::Static::Target>> targets = Analyser::Static::GetTargets([fullPath UTF8String]);
 
 		// grab the ROM record
 		MSXROMRecord *romRecord = romRecordsBySHA1[sha1];
@@ -222,7 +222,7 @@ static NSDictionary<NSString *, MSXROMRecord *> *romRecordsBySHA1 = @{
 		// assert equality
 		XCTAssert(!targets.empty(), "%@ should be recognised as an MSX file", testFile);
 		if(!targets.empty()) {
-			XCTAssert(targets.front().msx.cartridge_type == romRecord.cartridgeType, @"%@; should be %d, is %d", testFile, romRecord.cartridgeType, targets.front().msx.cartridge_type);
+			XCTAssert(targets.front()->msx.cartridge_type == romRecord.cartridgeType, @"%@; should be %d, is %d", testFile, romRecord.cartridgeType, targets.front()->msx.cartridge_type);
 		}
 	}
 }

OSBindings/SDL/main.cpp

@@ -261,8 +261,65 @@ int main(int argc, char *argv[]) {
 	CRTMachineDelegate crt_delegate;
 	SpeakerDelegate speaker_delegate;
 
+	// For vanilla SDL purposes, assume system ROMs can be found in one of:
+	//
+	//	/usr/local/share/CLK/[system]; or
+	//	/usr/share/CLK/[system]
+	std::vector<std::string> rom_names;
+	std::string machine_name;
+	ROMMachine::ROMFetcher rom_fetcher = [&rom_names, &machine_name]
+		(const std::string &machine, const std::vector<std::string> &names) -> std::vector<std::unique_ptr<std::vector<uint8_t>>> {
+			rom_names.insert(rom_names.end(), names.begin(), names.end());
+			machine_name = machine;
+
+			std::vector<std::unique_ptr<std::vector<uint8_t>>> results;
+			for(auto &name: names) {
+				std::string local_path = "/usr/local/share/CLK/" + machine + "/" + name;
+				FILE *file = std::fopen(local_path.c_str(), "rb");
+				if(!file) {
+					std::string path = "/usr/share/CLK/" + machine + "/" + name;
+					file = std::fopen(path.c_str(), "rb");
+				}
+
+				if(!file) {
+					results.emplace_back(nullptr);
+					continue;
+				}
+
+				std::unique_ptr<std::vector<uint8_t>> data(new std::vector<uint8_t>);
+
+				std::fseek(file, 0, SEEK_END);
+				data->resize(std::ftell(file));
+				std::fseek(file, 0, SEEK_SET);
+				std::size_t read = fread(data->data(), 1, data->size(), file);
+				std::fclose(file);
+
+				if(read == data->size())
+					results.emplace_back(std::move(data));
+				else
+					results.emplace_back(nullptr);
+			}
+
+			return results;
+		};
+
 	// Create and configure a machine.
-	std::unique_ptr<::Machine::DynamicMachine> machine(::Machine::MachineForTargets(targets));
+	::Machine::Error error;
+	std::unique_ptr<::Machine::DynamicMachine> machine(::Machine::MachineForTargets(targets, rom_fetcher, error));
+	if(!machine) {
+		switch(error) {
+			default: break;
+			case ::Machine::Error::MissingROM:
+				std::cerr << "Could not find system ROMs; please install to /usr/local/share/CLK/ or /usr/share/CLK/." << std::endl;
+				std::cerr << "One or more of the following were needed but not found:" << std::endl;
+				for(auto &name: rom_names) {
+					std::cerr << machine_name << '/' << name << std::endl;
+				}
+			break;
+		}
+
+		return -1;
+	}
 
 	updater.set_clock_rate(machine->crt_machine()->get_clock_rate());
 	crt_delegate.best_effort_updater = &updater;
@@ -302,57 +359,6 @@ int main(int argc, char *argv[]) {
 	GLint target_framebuffer = 0;
 	glGetIntegerv(GL_FRAMEBUFFER_BINDING, &target_framebuffer);
 
-	// For vanilla SDL purposes, assume system ROMs can be found in one of:
-	//
-	//	/usr/local/share/CLK/[system]; or
-	//	/usr/share/CLK/[system]
-	std::vector<std::string> rom_names;
-	std::string machine_name;
-	bool roms_loaded = machine->crt_machine()->set_rom_fetcher( [&rom_names, &machine_name]
-		(const std::string &machine, const std::vector<std::string> &names) -> std::vector<std::unique_ptr<std::vector<uint8_t>>> {
-			rom_names.insert(rom_names.end(), names.begin(), names.end());
-			machine_name = machine;
-
-			std::vector<std::unique_ptr<std::vector<uint8_t>>> results;
-			for(auto &name: names) {
-				std::string local_path = "/usr/local/share/CLK/" + machine + "/" + name;
-				FILE *file = std::fopen(local_path.c_str(), "rb");
-				if(!file) {
-					std::string path = "/usr/share/CLK/" + machine + "/" + name;
-					file = std::fopen(path.c_str(), "rb");
-				}
-
-				if(!file) {
-					results.emplace_back(nullptr);
-					continue;
-				}
-
-				std::unique_ptr<std::vector<uint8_t>> data(new std::vector<uint8_t>);
-
-				std::fseek(file, 0, SEEK_END);
-				data->resize(std::ftell(file));
-				std::fseek(file, 0, SEEK_SET);
-				std::size_t read = fread(data->data(), 1, data->size(), file);
-				std::fclose(file);
-
-				if(read == data->size())
-					results.emplace_back(std::move(data));
-				else
-					results.emplace_back(nullptr);
-			}
-
-			return results;
-		});
-
-	if(!roms_loaded) {
-		std::cerr << "Could not find system ROMs; please install to /usr/local/share/CLK/ or /usr/share/CLK/." << std::endl;
-		std::cerr << "One or more of the following were needed but not found:" << std::endl;
-		for(auto &name: rom_names) {
-			std::cerr << machine_name << '/' << name << std::endl;
-		}
-		return -1;
-	}
-
 	machine->configuration_target()->configure_as_target(*targets.front());
 
 	// Setup output, assuming a CRT machine for now, and prepare a best-effort updater.