From 66faed40087ec77e1cfb21ca5efe04197ccd16ad Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 25 Jan 2018 18:28:19 -0500 Subject: [PATCH] Gives `MachineForTargets` complete responsibility for initial machine state. --- Machines/ROMMachine.hpp | 4 +- Machines/Utility/MachineForTarget.cpp | 42 +++++-- Machines/Utility/MachineForTarget.hpp | 9 +- .../Clock Signal/Machine/CSMachine+Target.h | 1 - .../Mac/Clock Signal/Machine/CSMachine.mm | 16 +-- .../Mac/Clock Signal/Machine/CSROMFetcher.hpp | 2 +- .../Mac/Clock Signal/Machine/CSROMFetcher.mm | 6 +- .../AtariStaticAnalyserTests.mm | 8 +- .../Clock SignalTests/Bridges/C1540Bridge.mm | 2 +- .../MSXStaticAnalyserTests.mm | 6 +- OSBindings/SDL/main.cpp | 112 +++++++++--------- 11 files changed, 119 insertions(+), 89 deletions(-) diff --git a/Machines/ROMMachine.hpp b/Machines/ROMMachine.hpp index d95ae7042..4c40e8bad 100644 --- a/Machines/ROMMachine.hpp +++ b/Machines/ROMMachine.hpp @@ -15,11 +15,13 @@ namespace ROMMachine { +typedef std::function>>(const std::string &machine, const std::vector &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>>(const std::string &machine, const std::vector &names)> &rom_with_name) { return true; } + virtual bool set_rom_fetcher(const ROMFetcher &rom_with_name) { return true; } }; } diff --git a/Machines/Utility/MachineForTarget.cpp b/Machines/Utility/MachineForTarget.cpp index 6769e8aa0..c71e33303 100644 --- a/Machines/Utility/MachineForTarget.cpp +++ b/Machines/Utility/MachineForTarget.cpp @@ -18,19 +18,41 @@ #include "TypedDynamicMachine.hpp" -::Machine::DynamicMachine *::Machine::MachineForTargets(const std::vector> &targets) { +::Machine::DynamicMachine *::Machine::MachineForTargets(const std::vector> &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()); - case Analyser::Machine::Atari2600: return new TypedDynamicMachine(Atari2600::Machine::Atari2600()); - case Analyser::Machine::Electron: return new TypedDynamicMachine(Electron::Machine::Electron()); - case Analyser::Machine::MSX: return new TypedDynamicMachine(MSX::Machine::MSX()); - case Analyser::Machine::Oric: return new TypedDynamicMachine(Oric::Machine::Oric()); - case Analyser::Machine::Vic20: return new TypedDynamicMachine(Commodore::Vic20::Machine::Vic20()); - case Analyser::Machine::ZX8081: return new TypedDynamicMachine(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()); break; + case Analyser::Machine::Atari2600: machine = new TypedDynamicMachine(Atari2600::Machine::Atari2600()); break; + case Analyser::Machine::Electron: machine = new TypedDynamicMachine(Electron::Machine::Electron()); break; + case Analyser::Machine::MSX: machine = new TypedDynamicMachine(MSX::Machine::MSX()); break; + case Analyser::Machine::Oric: machine = new TypedDynamicMachine(Oric::Machine::Oric()); break; + case Analyser::Machine::Vic20: machine = new TypedDynamicMachine(Commodore::Vic20::Machine::Vic20()); break; + case Analyser::Machine::ZX8081: machine = new TypedDynamicMachine(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) { diff --git a/Machines/Utility/MachineForTarget.hpp b/Machines/Utility/MachineForTarget.hpp index e78b215ad..b54fbc120 100644 --- a/Machines/Utility/MachineForTarget.hpp +++ b/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> &targets); +DynamicMachine *MachineForTargets(const std::vector> &targets, const ::ROMMachine::ROMFetcher &rom_fetcher, Error &error); /*! Returns a short string name for the machine identified by the target, diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine+Target.h b/OSBindings/Mac/Clock Signal/Machine/CSMachine+Target.h index c754dd1a2..83f25a981 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine+Target.h +++ b/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 diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm index 4d0b5900c..15fd31701 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm +++ b/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(); diff --git a/OSBindings/Mac/Clock Signal/Machine/CSROMFetcher.hpp b/OSBindings/Mac/Clock Signal/Machine/CSROMFetcher.hpp index 0580df92e..0cf1201b2 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSROMFetcher.hpp +++ b/OSBindings/Mac/Clock Signal/Machine/CSROMFetcher.hpp @@ -8,4 +8,4 @@ #include "ROMMachine.hpp" -void CSApplyROMFetcher(ROMMachine::Machine &rom_machine); +ROMMachine::ROMFetcher CSROMFetcher(); diff --git a/OSBindings/Mac/Clock Signal/Machine/CSROMFetcher.mm b/OSBindings/Mac/Clock Signal/Machine/CSROMFetcher.mm index b6e995c7c..e95259a07 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSROMFetcher.mm +++ b/OSBindings/Mac/Clock Signal/Machine/CSROMFetcher.mm @@ -14,8 +14,8 @@ #include -void CSApplyROMFetcher(ROMMachine::Machine &rom_machine) { - rom_machine.set_rom_fetcher( [] (const std::string &machine, const std::vector &names) -> std::vector>> { +ROMMachine::ROMFetcher CSROMFetcher() { + return [] (const std::string &machine, const std::vector &names) -> std::vector>> { NSString *subDirectory = [@"ROMImages/" stringByAppendingString:[NSString stringWithUTF8String:machine.c_str()]]; std::vector>> results; for(auto &name: names) { @@ -31,5 +31,5 @@ void CSApplyROMFetcher(ROMMachine::Machine &rom_machine) { } return results; - }); + }; } diff --git a/OSBindings/Mac/Clock SignalTests/AtariStaticAnalyserTests.mm b/OSBindings/Mac/Clock SignalTests/AtariStaticAnalyserTests.mm index 8d6319118..f4c4264ee 100644 --- a/OSBindings/Mac/Clock SignalTests/AtariStaticAnalyserTests.mm +++ b/OSBindings/Mac/Clock SignalTests/AtariStaticAnalyserTests.mm @@ -9,7 +9,7 @@ #import #import -#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 *romRecordsBySHA1 = @{ for(int c = 0; c < CC_SHA1_DIGEST_LENGTH; c++) [sha1 appendFormat:@"%02x", sha1Bytes[c]]; // get an analysis of the file - std::list targets = Analyser::Static::GetTargets([fullPath UTF8String]); + std::vector> 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.uses_superchip == romRecord.usesSuperchip, @"%@; should be %@", testFile, romRecord.usesSuperchip ? @"true" : @"false"); + 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"); } } diff --git a/OSBindings/Mac/Clock SignalTests/Bridges/C1540Bridge.mm b/OSBindings/Mac/Clock SignalTests/Bridges/C1540Bridge.mm index 7cd2f495a..89abd2b98 100644 --- a/OSBindings/Mac/Clock SignalTests/Bridges/C1540Bridge.mm +++ b/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); } diff --git a/OSBindings/Mac/Clock SignalTests/MSXStaticAnalyserTests.mm b/OSBindings/Mac/Clock SignalTests/MSXStaticAnalyserTests.mm index 89db00c89..3f3f6afce 100644 --- a/OSBindings/Mac/Clock SignalTests/MSXStaticAnalyserTests.mm +++ b/OSBindings/Mac/Clock SignalTests/MSXStaticAnalyserTests.mm @@ -9,7 +9,7 @@ #import #import -#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 *romRecordsBySHA1 = @{ for(int c = 0; c < CC_SHA1_DIGEST_LENGTH; c++) [sha1 appendFormat:@"%02x", sha1Bytes[c]]; // get an analysis of the file - std::list targets = Analyser::Static::GetTargets([fullPath UTF8String]); + std::vector> targets = Analyser::Static::GetTargets([fullPath UTF8String]); // grab the ROM record MSXROMRecord *romRecord = romRecordsBySHA1[sha1]; @@ -222,7 +222,7 @@ static NSDictionary *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); } } } diff --git a/OSBindings/SDL/main.cpp b/OSBindings/SDL/main.cpp index da7098b1d..e27b213c6 100644 --- a/OSBindings/SDL/main.cpp +++ b/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 rom_names; + std::string machine_name; + ROMMachine::ROMFetcher rom_fetcher = [&rom_names, &machine_name] + (const std::string &machine, const std::vector &names) -> std::vector>> { + rom_names.insert(rom_names.end(), names.begin(), names.end()); + machine_name = machine; + + std::vector>> 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> data(new std::vector); + + 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 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 &names) -> std::vector>> { - rom_names.insert(rom_names.end(), names.begin(), names.end()); - machine_name = machine; - - std::vector>> 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> data(new std::vector); - - 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. @@ -435,7 +441,7 @@ int main(int argc, char *argv[]) { Analyser::Static::Media media = Analyser::Static::GetMedia(event.drop.file); machine->configuration_target()->insert_media(media); } break; - + case SDL_KEYDOWN: // Syphon off the key-press if it's control+shift+V (paste). if(event.key.keysym.sym == SDLK_v && (SDL_GetModState()&KMOD_CTRL) && (SDL_GetModState()&KMOD_SHIFT)) {