Fix size sorting and add regressive test

lib/mod_list.dart

@@ -310,7 +310,14 @@ class ModList {
     }
 
     // Optimize for soft constraints
-    return _optimizeSoftConstraints(loadOrder: loadOrder);
+    _optimizeSoftConstraints(loadOrder: loadOrder);
+    for (final modId in loadOrder.loadOrder) {
+      final mod = mods[modId]!;
+      print(
+        'Mod ID: ${mod.id}, Name: ${mod.name}, Enabled: ${mod.enabled}, Size: ${mod.size}, Dependencies: ${mod.dependencies}, Load After: ${mod.loadAfter}, Load Before: ${mod.loadBefore}, Incompatibilities: ${mod.incompatibilities}',
+      );
+    }
+    return loadOrder;
   }
 
   /// Calculate how many soft constraints are satisfied
@@ -356,34 +363,75 @@ class ModList {
     LoadOrder? loadOrder,
   }) {
     loadOrder ??= LoadOrder();
-    Map<String, int> scoreInfo = _calculateSoftConstraintsScore(
+
-      loadOrder.loadOrder,
+    // First, ensure base game and expansions are at the beginning in the correct order
-    );
+    List<String> baseAndExpansions = [];
+    List<String> harmony = [];
+    List<String> otherMods = [];
+
+    // Separate mods into categories
+    for (final modId in loadOrder.loadOrder) {
+      final mod = mods[modId]!;
+      if (modId == 'brrainz.harmony') {
+        harmony.add(modId);
+      } else if (mod.isBaseGame) {
+        baseAndExpansions.add(modId);
+      } else if (mod.isExpansion) {
+        baseAndExpansions.add(modId);
+      } else {
+        otherMods.add(modId);
+      }
+    }
+
+    // Sort expansions to ensure correct order
+    baseAndExpansions.sort((a, b) {
+      final modA = mods[a]!;
+      final modB = mods[b]!;
+
+      // Base game always first
+      if (modA.isBaseGame) return -1;
+      if (modB.isBaseGame) return 1;
+
+      // Sort expansions alphabetically by ID (which should work for Ludeon expansions)
+      return a.compareTo(b);
+    });
+
+    // Combine the lists with harmony first, then base game and expansions, then other mods
+    loadOrder.loadOrder.clear();
+    loadOrder.loadOrder.addAll(harmony);
+    loadOrder.loadOrder.addAll(baseAndExpansions);
+
+    // Now apply the normal optimization for the remaining mods
+    List<String> remainingMods = otherMods;
+
+    Map<String, int> scoreInfo = _calculateSoftConstraintsScore(remainingMods);
     int bestScore = scoreInfo['satisfied']!;
     int total = scoreInfo['total']!;
 
     if (total == 0 || bestScore == total) {
-      // All constraints satisfied or no constraints, sort by size where possible
+      // All constraints satisfied or no constraints for remaining mods, sort by size where possible
-      return _sortSizeWithinConstraints(loadOrder: loadOrder);
+      _sortSizeWithinConstraints(loadOrder: loadOrder, modList: remainingMods);
+      loadOrder.loadOrder.addAll(remainingMods);
+      return loadOrder;
     }
 
-    // Use a limited number of improvement passes
+    // Use a limited number of improvement passes for the remaining mods
     for (int iteration = 0; iteration < maxIterations; iteration++) {
       bool improved = false;
 
       // Try moving each mod to improve score
-      for (int i = 0; i < loadOrder.loadOrder.length; i++) {
+      for (int i = 0; i < remainingMods.length; i++) {
-        String modId = loadOrder.loadOrder[i];
+        String modId = remainingMods[i];
         Mod mod = mods[modId]!;
 
         // Calculate current local score for this mod
         Map<String, int> currentPositions = {};
-        for (int idx = 0; idx < loadOrder.loadOrder.length; idx++) {
+        for (int idx = 0; idx < remainingMods.length; idx++) {
-          currentPositions[loadOrder.loadOrder[idx]] = idx;
+          currentPositions[remainingMods[idx]] = idx;
         }
 
         // Try moving this mod to different positions
-        for (int newPos = 0; newPos < loadOrder.loadOrder.length; newPos++) {
+        for (int newPos = 0; newPos < remainingMods.length; newPos++) {
           if (newPos == i) continue;
 
           // Skip if move would break hard dependencies
@@ -392,7 +440,7 @@ class ModList {
             // Moving earlier
             // Check if any mod between newPos and i depends on this mod
             for (int j = newPos; j < i; j++) {
-              String depModId = loadOrder.loadOrder[j];
+              String depModId = remainingMods[j];
               if (mods[depModId]!.dependencies.contains(modId)) {
                 skip = true;
                 break;
@@ -402,7 +450,7 @@ class ModList {
             // Moving later
             // Check if this mod depends on any mod between i and newPos
             for (int j = i + 1; j <= newPos; j++) {
-              String depModId = loadOrder.loadOrder[j];
+              String depModId = remainingMods[j];
               if (mod.dependencies.contains(depModId)) {
                 skip = true;
                 break;
@@ -413,7 +461,7 @@ class ModList {
           if (skip) continue;
 
           // Create a new order with the mod moved
-          List<String> newOrder = List.from(loadOrder.loadOrder);
+          List<String> newOrder = List.from(remainingMods);
           newOrder.removeAt(i);
           newOrder.insert(newPos, modId);
 
@@ -425,8 +473,8 @@ class ModList {
 
           if (newScore > bestScore) {
             bestScore = newScore;
-            loadOrder.loadOrder.clear();
+            remainingMods.clear();
-            loadOrder.loadOrder.addAll(newOrder);
+            remainingMods.addAll(newOrder);
             improved = true;
             break; // Break inner loop, move to next mod
           }
@@ -438,19 +486,27 @@ class ModList {
       if (!improved) break; // If no improvements in this pass, stop
     }
 
-    // After optimizing for soft constraints, sort by size where possible
+    // Sort by size where possible for the remaining mods
-    return _sortSizeWithinConstraints(loadOrder: loadOrder);
+    _sortSizeWithinConstraints(loadOrder: loadOrder, modList: remainingMods);
+    loadOrder.loadOrder.addAll(remainingMods);
+
+    return loadOrder;
   }
 
   /// Sort mods by size within compatible groups
-  LoadOrder _sortSizeWithinConstraints({LoadOrder? loadOrder}) {
+  LoadOrder _sortSizeWithinConstraints({
+    LoadOrder? loadOrder,
+    List<String>? modList,
+  }) {
     loadOrder ??= LoadOrder();
+    List<String> modsToSort = modList ?? loadOrder.loadOrder;
+
     // Find groups of mods that can be reordered without breaking constraints
     List<List<String>> groups = [];
     List<String> currentGroup = [];
 
-    for (int i = 0; i < loadOrder.loadOrder.length; i++) {
+    for (int i = 0; i < modsToSort.length; i++) {
-      String modId = loadOrder.loadOrder[i];
+      String modId = modsToSort[i];
       Mod mod = mods[modId]!;
 
       if (currentGroup.isEmpty) {
@@ -504,9 +560,15 @@ class ModList {
     }
 
     // Reconstruct the order
-    loadOrder.loadOrder.clear();
+    modsToSort.clear();
     for (List<String> group in groups) {
-      loadOrder.loadOrder.addAll(group);
+      modsToSort.addAll(group);
+    }
+
+    // If we were given the loadOrder directly, update it
+    if (modList == null) {
+      loadOrder.loadOrder.clear();
+      loadOrder.loadOrder.addAll(modsToSort);
     }
 
     return loadOrder;

test/mod_list_regressive_test.dart

@@ -0,0 +1,314 @@
+import 'package:rimworld_modman/mod.dart';
+import 'package:rimworld_modman/mod_list.dart';
+import 'package:test/test.dart';
+
+Mod makeDummy() {
+  return Mod(
+    name: 'Dummy Mod',
+    id: 'dummy',
+    path: '',
+    versions: ["1.5"],
+    description: '',
+    dependencies: [],
+    loadAfter: [],
+    loadBefore: [],
+    incompatibilities: [],
+    size: 0,
+    isBaseGame: false,
+    isExpansion: false,
+    enabled: false,
+  );
+}
+
+void main() {
+  test('Mods should be sorted by size while respecting constraints', () {
+    final list = ModList();
+    list.mods = {
+      'dubwise.rimatomics': makeDummy().copyWith(
+        id: 'dubwise.rimatomics',
+        name: 'Dubs Rimatomics',
+        enabled: true,
+        size: 1563,
+        dependencies: [],
+        loadAfter: [],
+        loadBefore: [],
+        incompatibilities: [],
+      ),
+      'brrainz.justignoremepassing': makeDummy().copyWith(
+        id: 'brrainz.justignoremepassing',
+        name: 'Just Ignore Me Passing',
+        enabled: true,
+        size: 28,
+        dependencies: [],
+        loadAfter: [],
+        loadBefore: [],
+        incompatibilities: [],
+      ),
+      'brrainz.harmony': makeDummy().copyWith(
+        id: 'brrainz.harmony',
+        name: 'Harmony',
+        enabled: true,
+        size: 17,
+        dependencies: [],
+        loadAfter: [],
+        loadBefore: ['ludeon.rimworld'],
+        incompatibilities: [],
+      ),
+      'jecrell.doorsexpanded': makeDummy().copyWith(
+        id: 'jecrell.doorsexpanded',
+        name: 'Doors Expanded',
+        enabled: true,
+        size: 765,
+        dependencies: [],
+        loadAfter: [],
+        loadBefore: [],
+        incompatibilities: [],
+      ),
+      'dubwise.rimefeller': makeDummy().copyWith(
+        id: 'dubwise.rimefeller',
+        name: 'Rimefeller',
+        enabled: true,
+        size: 744,
+        dependencies: [],
+        loadAfter: [],
+        loadBefore: [],
+        incompatibilities: [],
+      ),
+      'neronix17.toolbox': makeDummy().copyWith(
+        id: 'neronix17.toolbox',
+        name: 'Tabula Rasa',
+        enabled: true,
+        size: 415,
+        dependencies: [],
+        loadAfter: [
+          'brrainz.harmony',
+          'ludeon.rimworld',
+          'ludeon.rimworld.royalty',
+          'ludeon.rimworld.ideology',
+          'ludeon.rimworld.biotech',
+          'ludeon.rimworld.anomaly',
+          'unlimitedhugs.hugslib',
+          'erdelf.humanoidalienraces',
+        ],
+        loadBefore: [],
+        incompatibilities: [],
+      ),
+      'automatic.bionicicons': makeDummy().copyWith(
+        id: 'automatic.bionicicons',
+        name: 'Bionic icons',
+        enabled: true,
+        size: 365,
+        dependencies: [],
+        loadAfter: [],
+        loadBefore: [],
+        incompatibilities: [],
+      ),
+      'lwm.deepstorage': makeDummy().copyWith(
+        id: 'lwm.deepstorage',
+        name: "LWM's Deep Storage",
+        enabled: true,
+        size: 256,
+        dependencies: [],
+        loadAfter: [
+          'brrainz.harmony',
+          'ludeon.rimworld.core',
+          'rimfridge.kv.rw',
+          'mlie.cannibalmeals',
+        ],
+        loadBefore: ['com.github.alandariva.moreplanning'],
+        incompatibilities: [],
+      ),
+      'dubwise.dubsmintmenus': makeDummy().copyWith(
+        id: 'dubwise.dubsmintmenus',
+        name: 'Dubs Mint Menus',
+        enabled: true,
+        size: 190,
+        dependencies: [],
+        loadAfter: [],
+        loadBefore: [],
+        incompatibilities: [],
+      ),
+      'dubwise.dubsmintminimap': makeDummy().copyWith(
+        id: 'dubwise.dubsmintminimap',
+        name: 'Dubs Mint Minimap',
+        enabled: true,
+        size: 190,
+        dependencies: [],
+        loadAfter: [],
+        loadBefore: [],
+        incompatibilities: [],
+      ),
+      'ludeon.rimworld': makeDummy().copyWith(
+        id: 'ludeon.rimworld',
+        name: 'RimWorld',
+        enabled: true,
+        size: 0,
+        dependencies: [],
+        loadAfter: [],
+        loadBefore: [],
+        incompatibilities: [],
+        isBaseGame: true,
+      ),
+      'ludeon.rimworld.royalty': makeDummy().copyWith(
+        id: 'ludeon.rimworld.royalty',
+        name: 'RimWorld Royalty',
+        enabled: true,
+        size: 0,
+        dependencies: [],
+        loadAfter: ['ludeon.rimworld'],
+        loadBefore: [],
+        incompatibilities: [],
+        isExpansion: true,
+      ),
+      'ludeon.rimworld.ideology': makeDummy().copyWith(
+        id: 'ludeon.rimworld.ideology',
+        name: 'RimWorld Ideology',
+        enabled: true,
+        size: 0,
+        dependencies: [],
+        loadAfter: ['ludeon.rimworld'],
+        loadBefore: [],
+        incompatibilities: [],
+        isExpansion: true,
+      ),
+      'ludeon.rimworld.biotech': makeDummy().copyWith(
+        id: 'ludeon.rimworld.biotech',
+        name: 'RimWorld Biotech',
+        enabled: true,
+        size: 0,
+        dependencies: [],
+        loadAfter: ['ludeon.rimworld'],
+        loadBefore: [],
+        incompatibilities: [],
+        isExpansion: true,
+      ),
+      'ludeon.rimworld.anomaly': makeDummy().copyWith(
+        id: 'ludeon.rimworld.anomaly',
+        name: 'RimWorld Anomaly',
+        enabled: true,
+        size: 0,
+        dependencies: [],
+        loadAfter: ['ludeon.rimworld'],
+        loadBefore: [],
+        incompatibilities: [],
+        isExpansion: true,
+      ),
+    };
+    list.enableAll();
+    final result = list.generateLoadOrder();
+
+    final expected = [
+      'brrainz.harmony',
+      'ludeon.rimworld',
+      'ludeon.rimworld.anomaly',
+      'ludeon.rimworld.biotech',
+      'ludeon.rimworld.ideology',
+      'ludeon.rimworld.royalty',
+      'dubwise.rimatomics',
+      'jecrell.doorsexpanded',
+      'dubwise.rimefeller',
+      'neronix17.toolbox',
+      'automatic.bionicicons',
+      'lwm.deepstorage',
+      'dubwise.dubsmintmenus',
+      'dubwise.dubsmintminimap',
+      'brrainz.justignoremepassing',
+    ];
+    expect(result.errors, isEmpty);
+    expect(result.loadOrder, equals(expected));
+  });
+  test('Prepatcher should load before Harmony', () {
+    final list = ModList();
+    list.mods = {
+      'bs.betterlog': makeDummy().copyWith(
+        id: 'bs.betterlog',
+        name: 'Better Log - Fix your errors',
+        enabled: true,
+        size: 69,
+        dependencies: [],
+        loadAfter: [
+          'brrainz.harmony',
+          'me.samboycoding.betterloading',
+          'zetrith.prepatcher',
+          'ludeon.rimworld',
+        ],
+        loadBefore: [
+          'ludeon.rimworld.royalty',
+          'ludeon.rimworld.ideology',
+          'ludeon.rimworld.biotech',
+          'ludeon.rimworld.anomaly',
+          'bs.performance',
+          'unlimitedhugs.hugslib',
+        ],
+        incompatibilities: [],
+      ),
+      'zetrith.prepatcher': makeDummy().copyWith(
+        id: 'zetrith.prepatcher',
+        name: 'Prepatcher',
+        enabled: true,
+        size: 21,
+        loadBefore: ['ludeon.rimworld', 'brrainz.harmony'],
+      ),
+      'brrainz.harmony': makeDummy().copyWith(
+        id: 'brrainz.harmony',
+        name: 'Harmony',
+        enabled: true,
+        size: 17,
+        loadBefore: ['ludeon.rimworld'],
+      ),
+      'ludeon.rimworld': makeDummy().copyWith(
+        id: 'ludeon.rimworld',
+        name: 'RimWorld',
+        enabled: true,
+        size: 0,
+        isBaseGame: true,
+      ),
+      'ludeon.rimworld.anomaly': makeDummy().copyWith(
+        id: 'ludeon.rimworld.anomaly',
+        name: 'RimWorld Anomaly',
+        enabled: true,
+        size: 0,
+        loadAfter: ['ludeon.rimworld'],
+        isExpansion: true,
+      ),
+      'ludeon.rimworld.biotech': makeDummy().copyWith(
+        id: 'ludeon.rimworld.biotech',
+        name: 'RimWorld Biotech',
+        enabled: true,
+        size: 0,
+        loadAfter: ['ludeon.rimworld'],
+        isExpansion: true,
+      ),
+      'ludeon.rimworld.ideology': makeDummy().copyWith(
+        id: 'ludeon.rimworld.ideology',
+        name: 'RimWorld Ideology',
+        enabled: true,
+        size: 0,
+        loadAfter: ['ludeon.rimworld'],
+        isExpansion: true,
+      ),
+      'ludeon.rimworld.royalty': makeDummy().copyWith(
+        id: 'ludeon.rimworld.royalty',
+        name: 'RimWorld Royalty',
+        enabled: true,
+        size: 0,
+        loadAfter: ['ludeon.rimworld'],
+        isExpansion: true,
+      ),
+    };
+    list.enableAll();
+    final order = list.generateLoadOrder();
+
+    final expected = [
+      'zetrith.prepatcher',
+      'brrainz.harmony',
+      'ludeon.rimworld',
+      'bs.betterlog',
+      'ludeon.rimworld.anomaly',
+      'ludeon.rimworld.biotech',
+      'ludeon.rimworld.ideology',
+      'ludeon.rimworld.royalty',
+    ];
+    expect(order.loadOrder, equals(expected));
+  });
+}

test/mod_list_test.dart

@@ -1,3 +1,4 @@
+import 'package:rimworld_modman/logger.dart';
 import 'package:rimworld_modman/mod.dart';
 import 'package:rimworld_modman/mod_list.dart';
 import 'package:test/test.dart';
@@ -147,6 +148,46 @@ void main() {
       expect(result.errors.any((e) => e.contains('incompatible')), isTrue);
       expect(result.errors.any((e) => e.contains('harmony')), isTrue);
     });
+    test('Base game should load before other mods', () {
+      final list = ModList();
+      list.mods = {
+        'dummy': makeDummy().copyWith(size: 10000),
+        'ludeon.rimworld': makeDummy().copyWith(
+          name: 'RimWorld',
+          id: 'ludeon.rimworld',
+          isBaseGame: true,
+        ),
+      };
+      list.enableAll();
+      final result = list.generateLoadOrder();
+
+      final expected = ['ludeon.rimworld', 'dummy'];
+      expect(result.errors, isEmpty);
+      expect(result.loadOrder, equals(expected));
+    });
+    test('Base game and expansions should load before other mods', () {
+      final list = ModList();
+      list.mods = {
+        'dummy': makeDummy().copyWith(size: 10000),
+        'ludeon.rimworld': makeDummy().copyWith(
+          name: 'RimWorld',
+          id: 'ludeon.rimworld',
+          isBaseGame: true,
+        ),
+        'ludeon.rimworld.anomaly': makeDummy().copyWith(
+          name: 'RimWorld Anomaly',
+          id: 'ludeon.rimworld.anomaly',
+          dependencies: ['ludeon.rimworld'],
+          isExpansion: true,
+        ),
+      };
+      list.enableAll();
+      final result = list.generateLoadOrder();
+
+      final expected = ['ludeon.rimworld', 'ludeon.rimworld.anomaly', 'dummy'];
+      expect(result.errors, isEmpty);
+      expect(result.loadOrder, equals(expected));
+    });
   });
 
   group('Test loadRequired', () {