From 1e4b4db2201a9b2eaa0acedf169e3105e8f34680 Mon Sep 17 00:00:00 2001 From: PhatPhuckDave Date: Tue, 18 Mar 2025 16:51:36 +0100 Subject: [PATCH] Rework everything again --- lib/mod.dart | 3 - lib/mod_list.dart | 236 +++++++++++++---------------- test/mod_list_regressive_test.dart | 6 +- test/mod_list_test.dart | 97 +++++------- 4 files changed, 144 insertions(+), 198 deletions(-) diff --git a/lib/mod.dart b/lib/mod.dart index ce80c52..25df596 100644 --- a/lib/mod.dart +++ b/lib/mod.dart @@ -31,9 +31,6 @@ class Mod { final bool isBaseGame; // Is this the base RimWorld game final bool isExpansion; // Is this a RimWorld expansion - int loadBeforeNotPlaced = 0; - int loadAfterPlaced = 0; - Mod({ required this.name, required this.id, diff --git a/lib/mod_list.dart b/lib/mod_list.dart index 82e187e..58c4016 100644 --- a/lib/mod_list.dart +++ b/lib/mod_list.dart @@ -6,9 +6,12 @@ import 'package:rimworld_modman/mod.dart'; import 'package:xml/xml.dart'; class LoadOrder { - final List order = []; + List order = []; final List errors = []; - List get loadOrder => order.map((e) => e.id).toList(); + + List get loadOrder { + return order.map((mod) => mod.id).toList(); + } LoadOrder(); @@ -251,150 +254,119 @@ class ModList { LoadOrder generateLoadOrder([LoadOrder? loadOrder]) { loadOrder ??= LoadOrder(); - final modMap = {for (final m in activeMods.values) m.id: m}; - final errors = _validateIncompatibilities(modMap); - if (errors.isNotEmpty) { - loadOrder.errors.addAll(errors); - } - - // Hard dependency graph - final inDegree = {}; - final adjacency = >{}; - - // Soft constraint reverse mappings - final reverseLoadBefore = >{}; - final reverseLoadAfter = >{}; - - // Initialize data structures + final logger = Logger.instance; for (final mod in activeMods.values) { - mod.loadBeforeNotPlaced = mod.loadBefore.length; - mod.loadAfterPlaced = 0; - - reverseLoadBefore[mod.id] = []; - reverseLoadAfter[mod.id] = []; - inDegree[mod.id] = 0; - adjacency[mod.id] = []; - } - - // Build dependency graph and reverse soft constraints - for (final mod in activeMods.values) { - for (final depId in mod.dependencies) { - adjacency[depId]!.add(mod.id); - inDegree[mod.id] = (inDegree[mod.id] ?? 0) + 1; - } - - for (final targetId in mod.loadBefore) { - final target = modMap[targetId]; - if (target != null) { - reverseLoadBefore[targetId]!.add(mod); - } - } - - for (final targetId in mod.loadAfter) { - final target = modMap[targetId]; - if (target != null) { - reverseLoadAfter[targetId]!.add(mod); + for (final incomp in mod.incompatibilities) { + if (activeMods.containsKey(incomp)) { + loadOrder.errors.add( + 'Incompatibility detected: ${mod.id} is incompatible with $incomp', + ); } } } - final heap = PriorityQueue((a, b) { - // 1. Base game first - if (a.isBaseGame != b.isBaseGame) { - return a.isBaseGame ? -1 : 1; - } - // 2. Expansions next - if (a.isExpansion != b.isExpansion) { - return a.isExpansion ? -1 : 1; - } - // 3. Soft constraints: Prioritize mods that need to be placed earlier - final aUnmetBefore = a.loadBeforeNotPlaced; - final bUnmetBefore = b.loadBeforeNotPlaced; - if (aUnmetBefore != bUnmetBefore) { - return bUnmetBefore.compareTo(aUnmetBefore); // Higher unmetBefore first - } - // If tied, deprioritize mods with more unmet `loadAfter` - final aUnmetAfter = a.loadAfter.length - a.loadAfterPlaced; - final bUnmetAfter = b.loadAfter.length - b.loadAfterPlaced; - if (aUnmetAfter != bUnmetAfter) { - return aUnmetAfter.compareTo(bUnmetAfter); // Lower unmetAfter first - } - // 4. Smaller size last + loadOrder.order.addAll(activeMods.values.toList()); + + loadOrder.order.sort((a, b) { + if (a.isBaseGame && !b.isBaseGame) return -1; + if (!a.isBaseGame && b.isBaseGame) return 1; + if (a.isExpansion && !b.isExpansion) return -1; + if (!a.isExpansion && b.isExpansion) return 1; return b.size.compareTo(a.size); }); - // Initialize heap with available mods - for (final mod in activeMods.values) { - if (inDegree[mod.id] == 0) { - heap.add(mod); - } + Map> relations = {}; + for (int i = loadOrder.order.length - 1; i >= 0; i--) { + final mod = loadOrder!.order[i]; + logger.info('Processing mod: ${mod.id}'); + loadOrder = shuffleMod(mod, loadOrder, relations); } - while (heap.isNotEmpty) { - final current = heap.removeFirst(); - loadOrder.order.add(current); - - // Update dependents' in-degree - for (final neighborId in adjacency[current.id]!) { - inDegree[neighborId] = inDegree[neighborId]! - 1; - if (inDegree[neighborId] == 0) { - heap.add(modMap[neighborId]!); - } - } - - // Update soft constraints - _updateReverseConstraints( - current, - reverseLoadBefore, - loadOrder, - heap, - (mod) => mod.loadBeforeNotPlaced--, - ); - _updateReverseConstraints( - current, - reverseLoadAfter, - loadOrder, - heap, - (mod) => mod.loadAfterPlaced++, - ); - } - - if (loadOrder.order.length != activeMods.length) { - loadOrder.errors.add("Cyclic dependencies detected"); - } - - return loadOrder; + return loadOrder!; } - List _validateIncompatibilities(Map modMap) { - final errors = []; - for (final mod in modMap.values) { - for (final incompatibleId in mod.incompatibilities) { - if (modMap.values.any((m) => m.id == incompatibleId)) { - errors.add("Incompatible mods: ${mod.id} and $incompatibleId"); - } - } - } - return errors; - } - - void _updateReverseConstraints( - Mod current, - Map> reverseMap, + // The point of relations and the recursive call is to handle the case where + // A mod depends on a mod that depends on another mod + // So we move our first mod A to after B + // But then we move B after C and A is no longer guranteed to be after B + // So we update it too just in case + // To make sure we have A B C + // Now it opens us to a stack overflow... + LoadOrder shuffleMod( + Mod mod, LoadOrder loadOrder, - PriorityQueue heap, - void Function(Mod) update, - ) { - reverseMap[current.id]?.forEach((affectedMod) { - if (!loadOrder.order.contains(affectedMod)) { - update(affectedMod); - // If mod is already in heap, re-add to update position - if (heap.contains(affectedMod)) { - heap.remove(affectedMod); - heap.add(affectedMod); - } + Map> relations, [ + Map? seen, + ]) { + final logger = Logger.instance; + logger.info('Starting shuffleMod for mod: ${mod.id}'); + + // Prevent infinite loops + seen ??= {}; + if (seen[mod.id] == true) { + logger.info('Mod ${mod.id} has already been seen, skipping.'); + return loadOrder; + } + seen[mod.id] = true; + logger.info('Marking mod ${mod.id} as seen.'); + + for (final dependency in mod.dependencies) { + logger.info('Checking dependency: $dependency for mod ${mod.id}'); + final depMod = mods[dependency]; + if (depMod == null) { + loadOrder.errors.add( + 'Missing dependency: ${mod.id} requires mod with ID $dependency', + ); + logger.warning( + 'Missing dependency: ${mod.id} requires mod with ID $dependency', + ); + continue; } - }); + + if (loadOrder.order.indexOf(mod) < loadOrder.order.indexOf(depMod)) { + logger.info('Reordering: ${mod.id} should come after ${depMod.id}'); + loadOrder.order.removeAt(loadOrder.order.indexOf(mod)); + loadOrder.order.insert(loadOrder.order.indexOf(depMod) + 1, mod); + relations[mod.id] = [...relations[mod.id] ?? [], depMod]; + } + } + + for (final loadAfter in mod.loadAfter) { + logger.info('Checking loadAfter: $loadAfter for mod ${mod.id}'); + final loadAfterMod = mods[loadAfter]; + if (loadAfterMod != null && + loadOrder.order.indexOf(mod) < + loadOrder.order.indexOf(loadAfterMod)) { + logger.info( + 'Reordering: ${mod.id} should come after ${loadAfterMod.id}', + ); + loadOrder.order.removeAt(loadOrder.order.indexOf(mod)); + loadOrder.order.insert(loadOrder.order.indexOf(loadAfterMod) + 1, mod); + relations[mod.id] = [...relations[mod.id] ?? [], loadAfterMod]; + } + } + + for (final loadBefore in mod.loadBefore) { + logger.info('Checking loadBefore: $loadBefore for mod ${mod.id}'); + final loadBeforeMod = mods[loadBefore]; + if (loadBeforeMod != null && + loadOrder.order.indexOf(mod) > + loadOrder.order.indexOf(loadBeforeMod)) { + logger.info( + 'Reordering: ${mod.id} should come before ${loadBeforeMod.id}', + ); + loadOrder.order.removeAt(loadOrder.order.indexOf(mod)); + loadOrder.order.insert(loadOrder.order.indexOf(loadBeforeMod), mod); + relations[mod.id] = [...relations[mod.id] ?? [], loadBeforeMod]; + } + } + + for (final relatedMod in relations[mod.id] ?? []) { + logger.info('Recursively shuffling related mod: ${relatedMod.id}'); + loadOrder = shuffleMod(relatedMod, loadOrder, relations, seen); + } + logger.info('Completed shuffleMod for mod: ${mod.id}'); + return loadOrder; } LoadOrder loadDependencies( diff --git a/test/mod_list_regressive_test.dart b/test/mod_list_regressive_test.dart index 3245e43..7ebb715 100644 --- a/test/mod_list_regressive_test.dart +++ b/test/mod_list_regressive_test.dart @@ -200,10 +200,10 @@ void main() { final expected = [ 'brrainz.harmony', 'ludeon.rimworld', - 'ludeon.rimworld.anomaly', - 'ludeon.rimworld.biotech', - 'ludeon.rimworld.ideology', 'ludeon.rimworld.royalty', + 'ludeon.rimworld.ideology', + 'ludeon.rimworld.biotech', + 'ludeon.rimworld.anomaly', 'dubwise.rimatomics', 'jecrell.doorsexpanded', 'dubwise.rimefeller', diff --git a/test/mod_list_test.dart b/test/mod_list_test.dart index f11c637..432ee35 100644 --- a/test/mod_list_test.dart +++ b/test/mod_list_test.dart @@ -1,4 +1,3 @@ -import 'package:rimworld_modman/logger.dart'; import 'package:rimworld_modman/mod.dart'; import 'package:rimworld_modman/mod_list.dart'; import 'package:test/test.dart'; @@ -246,7 +245,7 @@ void main() { final result = list.loadRequired(); // We say the mods are incompatible but load them anyway, who are we to decide what isn't loaded? - final expected = ['harmony', 'prepatcher', 'incompatible']; + final expected = ['incompatible', 'harmony', 'prepatcher']; expect(result.errors, isNotEmpty); expect(result.errors.any((e) => e.contains('incompatible')), isTrue); expect(result.errors.any((e) => e.contains('harmony')), isTrue); @@ -303,7 +302,7 @@ void main() { // We try to not disable mods...... But cyclic dependencies are just hell // Can not handle it - final expected = []; + final expected = ['modB', 'modA', 'modC']; expect(result.errors, isNotEmpty); expect(result.errors.any((e) => e.contains('modA')), isTrue); expect(result.errors.any((e) => e.contains('modB')), isTrue); @@ -446,26 +445,9 @@ void main() { final result = list.loadRequired(); + final expected = ['modD', 'modC', 'modB', 'modA']; expect(result.errors, isEmpty); - // All mods in the chain should be enabled - expect(result.loadOrder.contains('modA'), isTrue); - expect(result.loadOrder.contains('modB'), isTrue); - expect(result.loadOrder.contains('modC'), isTrue); - expect(result.loadOrder.contains('modD'), isTrue); - - // The order should be D -> C -> B -> A - expect( - result.loadOrder.indexOf('modD'), - lessThan(result.loadOrder.indexOf('modC')), - ); - expect( - result.loadOrder.indexOf('modC'), - lessThan(result.loadOrder.indexOf('modB')), - ); - expect( - result.loadOrder.indexOf('modB'), - lessThan(result.loadOrder.indexOf('modA')), - ); + expect(result.loadOrder, equals(expected)); }); }); @@ -504,11 +486,12 @@ void main() { }; list.enableAll(); - // Should return errors with both missing dependencies mentioned final result = list.generateLoadOrder(); + final expected = ['modA', 'modB']; expect(result.errors, isNotEmpty); expect(result.errors.any((e) => e.contains('missing1')), isTrue); expect(result.errors.any((e) => e.contains('missing2')), isTrue); + expect(result.loadOrder, equals(expected)); }); }); @@ -527,8 +510,9 @@ void main() { // Should not throw exception for soft constraints // But might generate a warning that we could check for final order = list.generateLoadOrder(); + final expected = ['modA']; expect(order.errors, isEmpty); - expect(order.loadOrder.contains('modA'), isTrue); + expect(order.loadOrder, equals(expected)); }); test('Should handle missing loadAfter relationships', () { @@ -544,8 +528,9 @@ void main() { // Should not throw exception for soft constraints final order = list.generateLoadOrder(); + final expected = ['modA']; expect(order.errors, isEmpty); - expect(order.loadOrder.contains('modA'), isTrue); + expect(order.loadOrder, equals(expected)); }); }); @@ -563,10 +548,12 @@ void main() { list.enableAll(); final result = list.generateLoadOrder(); + final expected = ['modA', 'modB']; expect(result.errors, isNotEmpty); - expect(result.errors.any((e) => e.contains('Incompatible')), isTrue); + expect(result.errors.any((e) => e.contains('incompatible')), isTrue); expect(result.errors.any((e) => e.contains('modA')), isTrue); expect(result.errors.any((e) => e.contains('modB')), isTrue); + expect(result.loadOrder, equals(expected)); }); test('Should handle a combination of errors simultaneously', () { @@ -583,9 +570,11 @@ void main() { list.enableAll(); final result = list.generateLoadOrder(); + final expected = ['modA', 'modB']; expect(result.errors, isNotEmpty); expect(result.errors.any((e) => e.contains('missingDep')), isTrue); - expect(result.errors.any((e) => e.contains('Incompatible')), isTrue); + expect(result.errors.any((e) => e.contains('incompatible')), isTrue); + expect(result.loadOrder, equals(expected)); }); }); @@ -664,11 +653,10 @@ void main() { list.enableAll(); final result = list.generateLoadOrder(); + final expected = ['modA', 'modB']; expect(result.errors, isEmpty); - expect(result.loadOrder.length, equals(2)); - expect(result.loadOrder.contains('modA'), isTrue); - expect(result.loadOrder.contains('modB'), isTrue); + expect(result.loadOrder, equals(expected)); }); test('Should return errors for missing dependencies', () { @@ -683,10 +671,11 @@ void main() { list.enableAll(); final result = list.generateLoadOrder(); + final expected = ['modA']; expect(result.errors, isNotEmpty); expect(result.errors.any((e) => e.contains('nonExistentMod')), isTrue); - // The load order might be empty or contain valid mods only + expect(result.loadOrder, equals(expected)); }); test('Should return both valid load order and errors', () { @@ -702,12 +691,11 @@ void main() { list.enableAll(); final result = list.generateLoadOrder(); + final expected = ['modA', 'modB']; expect(result.errors, isNotEmpty); expect(result.errors.any((e) => e.contains('nonExistentMod')), isTrue); - // modA should still be in the load order even though modB has errors - expect(result.loadOrder.contains('modA'), isTrue); - // modB might be excluded from load order due to its error + expect(result.loadOrder, equals(expected)); }); }); @@ -731,7 +719,7 @@ void main() { list.enableAll(); final result = list.generateLoadOrder(); - + final expected = ['modA', 'modB']; // Verify all missing dependencies are reported expect(result.errors, isNotEmpty); expect(result.errors.any((e) => e.contains('missingDep1')), isTrue); @@ -739,12 +727,11 @@ void main() { expect(result.errors.any((e) => e.contains('missingDep3')), isTrue); // Verify errors include the mod that requires the missing dependency - expect(result.errors.any((e) => e.contains('Mod A')), isTrue); - expect(result.errors.any((e) => e.contains('Mod B')), isTrue); + expect(result.errors.any((e) => e.contains('modA')), isTrue); + expect(result.errors.any((e) => e.contains('modB')), isTrue); // But mods should still be loaded anyway (the "It's fucked but anyway" philosophy) - expect(result.loadOrder.contains('modA'), isTrue); - expect(result.loadOrder.contains('modB'), isTrue); + expect(result.loadOrder, equals(expected)); }, ); }); @@ -762,9 +749,10 @@ void main() { list.enableAll(); final result = list.generateLoadOrder(); + final expected = ['modA']; // Should still generate a valid load order despite missing soft constraints - expect(result.loadOrder.contains('modA'), isTrue); + expect(result.loadOrder, equals(expected)); // System should track or report the missing loadBefore targets // This may be implementation-specific - modify if needed based on how your system handles this @@ -788,9 +776,10 @@ void main() { list.enableAll(); final result = list.generateLoadOrder(); + final expected = ['existingMod', 'modA']; - // Should still generate a valid load order despite missing soft constraints - expect(result.loadOrder.contains('modA'), isTrue); + + // Should still generatdeequals(mopected) expect(result.loadOrder.contains('existingMod'), isTrue); // The existing loadAfter relationship should be honored @@ -820,7 +809,7 @@ void main() { }; list.enableAll(); - final result = list.generateLoadOrder(); + final expected = ['modA']; final result = list.generateLoadOrder(); // Should report the missing dependency expect(result.errors, isNotEmpty); @@ -833,7 +822,7 @@ void main() { isFalse, ); - // Mod should still be loaded despite missing dependencies + // Mod should still be , equals(expected)pendencies expect(result.loadOrder.contains('modA'), isTrue); }, ); @@ -863,6 +852,7 @@ void main() { list.enableAll(); final result = list.generateLoadOrder(); + final expected = ['modB', 'modC', 'modA']; // Should report all missing dependencies expect(result.errors, isNotEmpty); @@ -870,24 +860,11 @@ void main() { expect(result.errors.any((e) => e.contains('missingDep2')), isTrue); // Should indicate which mods are affected by these missing dependencies - expect(result.errors.any((e) => e.contains('Mod B')), isTrue); - expect(result.errors.any((e) => e.contains('Mod C')), isTrue); + expect(result.errors.any((e) => e.contains('modB')), isTrue); + expect(result.errors.any((e) => e.contains('modC')), isTrue); // But all mods should still be loaded in the best possible order - expect(result.loadOrder.contains('modA'), isTrue); - expect(result.loadOrder.contains('modB'), isTrue); - expect(result.loadOrder.contains('modC'), isTrue); - - // And existing constraints should be respected - // modA depends on modB and modC, so it should load after them - expect( - result.loadOrder.indexOf('modB'), - lessThan(result.loadOrder.indexOf('modA')), - ); - expect( - result.loadOrder.indexOf('modC'), - lessThan(result.loadOrder.indexOf('modA')), - ); + expect(result.loadOrder, equals(expected)); }, );