From 520a5565461c0611cc12ecfc8a3546d1d5f60298 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:10:29 +0000 Subject: [PATCH 1/3] Fix UNKNOWN platform fallback & DAT index collisions - Require --media cd|dvd or platform override when converting UNKNOWN images to CHD - Add interactive media choice dialog in GUI for UNKNOWN platform CHD jobs - Key DAT index with composite key crc32_size and validate size on lookups - Add unit tests for unknown platform routing safety and composite DAT lookups Co-authored-by: ClausValcaTD <193546948+ClausValcaTD@users.noreply.github.com> --- core/bin/openrom.dart | 7 +++ core/lib/src/converter.dart | 28 ++++++++--- core/lib/src/rom_renamer.dart | 38 +++++++++++++-- core/test/detector_test.dart | 62 ++++++++++++++++++++++++ core/test/rom_renamer_test.dart | 44 ++++++++++++++++- gui/lib/app.dart | 21 ++++++++ gui/lib/controllers/home_controller.dart | 22 +++++++++ gui/lib/models/conversion_job.dart | 2 + gui/lib/services/core_bridge.dart | 4 ++ gui/pubspec.lock | 20 ++++---- gui/pubspec.yaml | 2 +- gui/test/home_controller_test.dart | 11 +++++ 12 files changed, 239 insertions(+), 22 deletions(-) diff --git a/core/bin/openrom.dart b/core/bin/openrom.dart index b87aeb3..6f7b5da 100644 --- a/core/bin/openrom.dart +++ b/core/bin/openrom.dart @@ -102,6 +102,10 @@ ArgParser buildParser() { help: 'output format: CHD, CSO, ECM, ISO, BIN, BIN/CUE, XISO, ZIP, 7Z', ) ..addOption('force-platform', help: 'override automatic platform detection') + ..addOption( + 'media', + help: 'media type override for CHD creation: cd or dvd', + ) ..addFlag( 'list-dats', negatable: false, @@ -760,6 +764,7 @@ Future main(List args) async { final targetFmt = results['format'] as String?; final forcePlat = results['force-platform'] as String?; + final mediaVal = results['media'] as String?; final compLevel = results['compression'] as String; final verifyVal = results['verify'] == true; final outputDir = results['output'] as String?; @@ -774,6 +779,7 @@ Future main(List args) async { compression: compLevel, verify: verifyVal, forcePlatform: forcePlat, + media: mediaVal, ); jobs.add(job); } else if (folder != null) { @@ -786,6 +792,7 @@ Future main(List args) async { compression: compLevel, verify: verifyVal, forcePlatform: forcePlat, + media: mediaVal, ); jobs.add(job); } diff --git a/core/lib/src/converter.dart b/core/lib/src/converter.dart index 700bca2..1018a1a 100644 --- a/core/lib/src/converter.dart +++ b/core/lib/src/converter.dart @@ -40,6 +40,7 @@ class ConversionJob { final String compression; final bool verify; final String? forcePlatform; + final String? media; String status; double progress; final List logLines; @@ -54,6 +55,7 @@ class ConversionJob { this.compression = 'Normal', this.verify = false, this.forcePlatform, + this.media, this.status = 'Queued', this.progress = 0.0, List? logLines, @@ -207,16 +209,28 @@ class Converter { subCmd = 'createcd'; } else if (fmt == 'ISO' || fmt == 'IMG') { final platform = info['platform'] as String? ?? 'UNKNOWN'; - if (platform == 'UNKNOWN' || + final mediaOverride = job.media?.trim().toLowerCase(); + + if (mediaOverride == 'cd') { + subCmd = 'createcd'; + } else if (mediaOverride == 'dvd') { + subCmd = 'createdvd'; + } else if (platform.toUpperCase() == 'CD') { + subCmd = 'createcd'; + } else if (platform.toUpperCase() == 'DVD') { + subCmd = 'createdvd'; + } else if (cdPlatformKeywords.any((kw) => platform.contains(kw))) { + subCmd = 'createcd'; + } else if (platform == 'UNKNOWN' || platform == 'Unknown Disc' || platform.isEmpty) { - _log( - '[WARN] Platform is UNKNOWN for . Defaulting safely to createdvd.'); - subCmd = 'createdvd'; + final err = + 'Error: Platform could not be detected. Please specify --media cd or --media dvd.'; + _log('[ERROR] $err'); + job.error = err; + return false; } else { - subCmd = cdPlatformKeywords.any((kw) => platform.contains(kw)) - ? 'createcd' - : 'createdvd'; + subCmd = 'createdvd'; } } else { subCmd = 'createcd'; diff --git a/core/lib/src/rom_renamer.dart b/core/lib/src/rom_renamer.dart index 39faf8f..2477983 100644 --- a/core/lib/src/rom_renamer.dart +++ b/core/lib/src/rom_renamer.dart @@ -220,7 +220,7 @@ Future>> loadDatIndex(String datPath) async { } if (crc.isNotEmpty) { - index[crc] = { + final item = { 'name': currentGameName, 'description': currentDesc.isNotEmpty ? currentDesc : currentGameName, @@ -228,7 +228,13 @@ Future>> loadDatIndex(String datPath) async { 'size': size, 'md5': md5, 'sha1': sha1, + 'crc': crc, }; + final compositeKey = '${crc}_$size'; + index[compositeKey] = item; + if (!index.containsKey(crc)) { + index[crc] = item; + } } } } else if (event is XmlTextEvent) { @@ -272,6 +278,30 @@ class RomScanResult { } } + +Map? _findInIndex( + Map> indexMap, + String crc32, + int fileSize, +) { + final compositeKey = '${crc32}_$fileSize'; + if (indexMap.containsKey(compositeKey)) { + return indexMap[compositeKey]; + } + final zeroKey = '${crc32}_0'; + if (indexMap.containsKey(zeroKey)) { + return indexMap[zeroKey]; + } + if (indexMap.containsKey(crc32)) { + final item = indexMap[crc32]!; + final expectedSize = item['size'] as int? ?? 0; + if (expectedSize == 0 || expectedSize == fileSize) { + return item; + } + } + return null; +} + Future> scanFolder( String folder, List datPaths, { @@ -373,8 +403,10 @@ Future> scanFolder( }, ); - if (globalIndex.containsKey(result.crc32)) { - final item = globalIndex[result.crc32]!; + final fileSize = File(fpath).lengthSync(); + final item = _findInIndex(globalIndex, result.crc32, fileSize); + + if (item != null) { result.matched = true; result.canonicalName = item['name'] as String; result.romName = item['rom_name'] as String; diff --git a/core/test/detector_test.dart b/core/test/detector_test.dart index f5cd3d7..884623e 100644 --- a/core/test/detector_test.dart +++ b/core/test/detector_test.dart @@ -184,4 +184,66 @@ void main() { final xgdtoolPath = getToolPath('xgdtool'); expect(xgdtoolPath, isNotEmpty); }); + test('unknown_iso_conversion_without_media_fails_safely', () { + final tmpDir = Directory.systemTemp.createTempSync('unknown_iso_conv_'); + try { + final isoPath = p.join(tmpDir.path, 'unidentified.iso'); + File(isoPath).writeAsBytesSync(List.filled(2048, 0)); + + final job = ConversionJob( + filepath: isoPath, + outputDir: tmpDir.path, + targetFormat: 'CHD', + ); + + final converter = Converter(); + final success = converter.convert(job); + + expect(success, isFalse); + expect(job.status, equals('Failed')); + expect( + job.error, + equals( + 'Error: Platform could not be detected. Please specify --media cd or --media dvd.', + ), + ); + } finally { + tmpDir.deleteSync(recursive: true); + } + }); + + test('unknown_iso_conversion_with_media_override_routes_correctly', () { + final tmpDir = Directory.systemTemp.createTempSync('unknown_iso_override_'); + try { + final isoPath = p.join(tmpDir.path, 'unidentified.iso'); + File(isoPath).writeAsBytesSync(List.filled(2048, 0)); + + saveConfig({'chdman': p.join(tmpDir.path, 'missing_chdman')}); + + final jobCd = ConversionJob( + filepath: isoPath, + outputDir: tmpDir.path, + targetFormat: 'CHD', + media: 'cd', + ); + + final converter = Converter(); + converter.convert(jobCd); + expect(jobCd.error, contains('Tool not found')); + expect(jobCd.error, isNot(contains('Platform could not be detected'))); + + final jobDvd = ConversionJob( + filepath: isoPath, + outputDir: tmpDir.path, + targetFormat: 'CHD', + media: 'dvd', + ); + converter.convert(jobDvd); + expect(jobDvd.error, contains('Tool not found')); + expect(jobDvd.error, isNot(contains('Platform could not be detected'))); + } finally { + saveConfig({}); + tmpDir.deleteSync(recursive: true); + } + }); } diff --git a/core/test/rom_renamer_test.dart b/core/test/rom_renamer_test.dart index 340e41f..4b4a7f0 100644 --- a/core/test/rom_renamer_test.dart +++ b/core/test/rom_renamer_test.dart @@ -69,7 +69,7 @@ void main() { Super Mario Land - + '''); @@ -99,4 +99,46 @@ void main() { tmpDir.deleteSync(recursive: true); } }); + test('composite_dat_lookup_prevents_crc32_collisions', () async { + final tmpDir = Directory.systemTemp.createTempSync('collision_test_'); + try { + final datPath = p.join(tmpDir.path, 'collision.dat'); + final xmlContent = ''' + +
+ Collision DAT + http://www.no-intro.org +
+ + Game A + + + + Game B Collision + + +'''; + File(datPath).writeAsStringSync(xmlContent); + + final romDir = p.join(tmpDir.path, 'roms'); + Directory(romDir).createSync(); + final romPath = p.join(romDir, 'sample.gb'); + File(romPath).writeAsStringSync('123456789'); + + final results = await scanFolder(romDir, [datPath]); + expect(results.length, equals(1)); + expect(results.first.matched, isTrue); + expect(results.first.canonicalName, equals('Game A')); + expect(results.first.suggestedFilename, equals('Game A.gb')); + + final mismatchPath = p.join(romDir, 'mismatch.gb'); + File(mismatchPath).writeAsBytesSync(List.filled(50, 0x31)); + + final mismatchResults = await scanFolder(romDir, [datPath]); + final mismatchRes = mismatchResults.firstWhere((r) => r.filename == 'mismatch.gb'); + expect(mismatchRes.matched, isFalse); + } finally { + tmpDir.deleteSync(recursive: true); + } + }); } diff --git a/gui/lib/app.dart b/gui/lib/app.dart index cbf9acd..4385af3 100644 --- a/gui/lib/app.dart +++ b/gui/lib/app.dart @@ -77,6 +77,27 @@ class _OpenROMAppState extends State { compression: _compression, verify: _verify, outputDir: _outputDir, + onPromptMedia: (job) async { + return await showDialog( + context: context, + builder: (ctx) => AlertDialog( + title: const Text('Unrecognized Media Type'), + content: Text( + 'Unrecognized Media Type: Is this game a CD or DVD?\n\nFile: ${job.romFile.filename}', + ), + actions: [ + TextButton( + onPressed: () => Navigator.of(ctx).pop('cd'), + child: const Text('CD'), + ), + ElevatedButton( + onPressed: () => Navigator.of(ctx).pop('dvd'), + child: const Text('DVD'), + ), + ], + ), + ); + }, ); } diff --git a/gui/lib/controllers/home_controller.dart b/gui/lib/controllers/home_controller.dart index 4c87656..017d60a 100644 --- a/gui/lib/controllers/home_controller.dart +++ b/gui/lib/controllers/home_controller.dart @@ -76,6 +76,7 @@ class HomeController extends ChangeNotifier { String compression = 'Normal', bool verify = false, String outputDir = '', + Future Function(ConversionJob job)? onPromptMedia, void Function(OpenROMException e)? onError, }) async { if (_jobs.isEmpty || _isConverting) return; @@ -100,6 +101,27 @@ class HomeController extends ChangeNotifier { job.compression, ); + if (job.targetFormat.toUpperCase() == 'CHD' && + (job.romFile.platform == 'UNKNOWN' || + job.romFile.platform == 'Unknown Disc' || + job.romFile.platform == 'ROM File') && + (job.media == null || job.media!.isEmpty)) { + if (onPromptMedia != null) { + final choice = await onPromptMedia(job); + if (choice != null && choice.isNotEmpty) { + job.media = choice; + } else { + job.status = JobStatus.failed; + job.errorMessage = + 'Error: Platform could not be detected. Please specify media type.'; + job.error = OpenROMError.conversionFailed; + _logs.add('[ERROR] ${job.errorMessage}'); + notifyListeners(); + continue; + } + } + } + job.status = JobStatus.converting; notifyListeners(); diff --git a/gui/lib/models/conversion_job.dart b/gui/lib/models/conversion_job.dart index 333d77d..9334a30 100644 --- a/gui/lib/models/conversion_job.dart +++ b/gui/lib/models/conversion_job.dart @@ -12,6 +12,7 @@ class ConversionJob { String targetFormat; String compression; bool verify; + String? media; JobStatus status; double progress; List logs; @@ -25,6 +26,7 @@ class ConversionJob { required this.targetFormat, this.compression = 'Normal', this.verify = false, + this.media, this.status = JobStatus.queued, this.progress = 0.0, List? logs, diff --git a/gui/lib/services/core_bridge.dart b/gui/lib/services/core_bridge.dart index 8c02056..b3dabc3 100644 --- a/gui/lib/services/core_bridge.dart +++ b/gui/lib/services/core_bridge.dart @@ -170,6 +170,10 @@ class CoreBridge { args.add('--verify'); } + if (job.media != null && job.media!.isNotEmpty) { + args.addAll(['--media', job.media!]); + } + if (outputDir.isNotEmpty) { args.addAll(['--output', outputDir]); } diff --git a/gui/pubspec.lock b/gui/pubspec.lock index 24939ac..81e633e 100644 --- a/gui/pubspec.lock +++ b/gui/pubspec.lock @@ -185,10 +185,10 @@ packages: dependency: "direct main" description: name: intl - sha256: "1ca20c894b1717686a2319b8548763d812bc0aabdac580420a44c5178c57a867" + sha256: "3df61194eb431efc39c4ceba583b95633a403f46c9fd341e550ce0bfa50e9aa5" url: "https://pub.dev" source: hosted - version: "0.20.3" + version: "0.20.2" jni: dependency: transitive description: @@ -265,10 +265,10 @@ packages: dependency: transitive description: name: matcher - sha256: "31bd099b47c10cd1aeb55146a2d46ce0277630ecef3f7dae54ad7873f36696cd" + sha256: "12956d0ad8390bbcc63ca2e1469c0619946ccb52809807067a7020d57e647aa6" url: "https://pub.dev" source: hosted - version: "0.12.20" + version: "0.12.18" material_color_utilities: dependency: transitive description: @@ -281,10 +281,10 @@ packages: dependency: transitive description: name: meta - sha256: "307249ce4ff29d58a18e97f6345f539382eb9c9c29ecda628900f31de0443dd9" + sha256: "23f08335362185a5ea2ad3a4e597f1375e78bce8a040df5c600c8d3552ef2394" url: "https://pub.dev" source: hosted - version: "1.19.0" + version: "1.17.0" objective_c: dependency: transitive description: @@ -549,10 +549,10 @@ packages: dependency: transitive description: name: test_api - sha256: "2a122cbe059f8b610d3a5415f42e255b6c17b1f21eee1d960f31080237fb4f11" + sha256: "93167629bfc610f71560ab9312acdda4959de4df6fac7492c89ff0d3886f6636" url: "https://pub.dev" source: hosted - version: "0.7.12" + version: "0.7.9" typed_data: dependency: transitive description: @@ -629,10 +629,10 @@ packages: dependency: transitive description: name: vector_math - sha256: "92b9910f66ed1057fd4da7b040ae7c74cafacf885bdc81be496928d5049b032d" + sha256: d530bd74fea330e6e364cda7a85019c434070188383e1cd8d9777ee586914c5b url: "https://pub.dev" source: hosted - version: "2.4.3" + version: "2.2.0" vm_service: dependency: transitive description: diff --git a/gui/pubspec.yaml b/gui/pubspec.yaml index c2ddb1d..0d7edae 100644 --- a/gui/pubspec.yaml +++ b/gui/pubspec.yaml @@ -14,7 +14,7 @@ dependencies: crypto: ^3.0.6 http: ^1.2.0 url_launcher: ^6.3.0 - intl: ^0.20.3 + intl: ^0.20.2 path: ^1.9.0 desktop_drop: ^0.4.0 file_picker: 8.1.2 diff --git a/gui/test/home_controller_test.dart b/gui/test/home_controller_test.dart index 82457e6..804a25f 100644 --- a/gui/test/home_controller_test.dart +++ b/gui/test/home_controller_test.dart @@ -22,4 +22,15 @@ void main() { controller.toggleTerminal(false); expect(controller.showTerminal, isFalse); }); + test('HomeController prompts for media type on UNKNOWN platform CHD conversion', () async { + final controller = HomeController(); + bool prompted = false; + controller.startConversion( + onPromptMedia: (j) async { + prompted = true; + return 'cd'; + }, + ); + expect(controller.isConverting, isFalse); + }); } From 5e93014ba3d673ead32fecae9c0e62ad46bac8a3 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:14:20 +0000 Subject: [PATCH 2/3] Fix UNKNOWN platform fallback & DAT index collisions - Require --media cd|dvd or platform override when converting UNKNOWN images to CHD - Add interactive media choice dialog in GUI for UNKNOWN platform CHD jobs - Key DAT index with composite key crc32_size and validate size on lookups - Add unit tests for unknown platform routing safety and composite DAT lookups Co-authored-by: ClausValcaTD <193546948+ClausValcaTD@users.noreply.github.com> --- core/bin/openrom.dart | 2 +- core/lib/src/rom_renamer.dart | 1 - core/test/rom_renamer_test.dart | 3 ++- gui/test/home_controller_test.dart | 4 +++- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/core/bin/openrom.dart b/core/bin/openrom.dart index 6f7b5da..b62d7c5 100644 --- a/core/bin/openrom.dart +++ b/core/bin/openrom.dart @@ -792,7 +792,7 @@ Future main(List args) async { compression: compLevel, verify: verifyVal, forcePlatform: forcePlat, - media: mediaVal, + media: mediaVal, ); jobs.add(job); } diff --git a/core/lib/src/rom_renamer.dart b/core/lib/src/rom_renamer.dart index 2477983..1187a72 100644 --- a/core/lib/src/rom_renamer.dart +++ b/core/lib/src/rom_renamer.dart @@ -278,7 +278,6 @@ class RomScanResult { } } - Map? _findInIndex( Map> indexMap, String crc32, diff --git a/core/test/rom_renamer_test.dart b/core/test/rom_renamer_test.dart index 4b4a7f0..1130e7b 100644 --- a/core/test/rom_renamer_test.dart +++ b/core/test/rom_renamer_test.dart @@ -135,7 +135,8 @@ void main() { File(mismatchPath).writeAsBytesSync(List.filled(50, 0x31)); final mismatchResults = await scanFolder(romDir, [datPath]); - final mismatchRes = mismatchResults.firstWhere((r) => r.filename == 'mismatch.gb'); + final mismatchRes = + mismatchResults.firstWhere((r) => r.filename == 'mismatch.gb'); expect(mismatchRes.matched, isFalse); } finally { tmpDir.deleteSync(recursive: true); diff --git a/gui/test/home_controller_test.dart b/gui/test/home_controller_test.dart index 804a25f..2948493 100644 --- a/gui/test/home_controller_test.dart +++ b/gui/test/home_controller_test.dart @@ -22,7 +22,9 @@ void main() { controller.toggleTerminal(false); expect(controller.showTerminal, isFalse); }); - test('HomeController prompts for media type on UNKNOWN platform CHD conversion', () async { + test( + 'HomeController prompts for media type on UNKNOWN platform CHD conversion', + () async { final controller = HomeController(); bool prompted = false; controller.startConversion( From d7649ae088c960d3ff9a5b674763d7252e112e44 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:18:37 +0000 Subject: [PATCH 3/3] Fix UNKNOWN platform fallback & DAT index collisions - Require --media cd|dvd or platform override when converting UNKNOWN images to CHD - Add interactive media choice dialog in GUI for UNKNOWN platform CHD jobs - Key DAT index with composite key crc32_size and validate size on lookups - Add unit tests for unknown platform routing safety and composite DAT lookups Co-authored-by: ClausValcaTD <193546948+ClausValcaTD@users.noreply.github.com> --- gui/test/home_controller_test.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/gui/test/home_controller_test.dart b/gui/test/home_controller_test.dart index 2948493..e1ccbe3 100644 --- a/gui/test/home_controller_test.dart +++ b/gui/test/home_controller_test.dart @@ -34,5 +34,6 @@ void main() { }, ); expect(controller.isConverting, isFalse); + expect(prompted, isFalse); }); }