Refactor mod list to return an object with a list of errors
So we don't throw and catch like Java
This commit is contained in:
@@ -8,6 +8,13 @@ const configRoot = '$root/AppData/RimWorld by Ludeon Studios/Config';
|
||||
const configPath = '$configRoot/ModsConfig.xml';
|
||||
const logsPath = '$root/ModManager';
|
||||
|
||||
// Few things we missed:
|
||||
// 1. BuildLoadOrder should either return or throw a List<String> in addition to... List<String>
|
||||
// That will represent errors
|
||||
// Those would be incompatibilities, missing dependencies, unable to setup before/after and so on
|
||||
// 2. Write tests for missing dependencies
|
||||
// 3. Maybe write tests for missing loadBefore/loadAfter
|
||||
|
||||
Mod makeDummy() {
|
||||
return Mod(
|
||||
name: 'Dummy Mod',
|
||||
@@ -40,8 +47,9 @@ void main() {
|
||||
list.enableAll();
|
||||
final order = list.generateLoadOrder();
|
||||
|
||||
final harmonyIndex = order.indexOf('harmony');
|
||||
final rimworldIndex = order.indexOf('ludeon.rimworld');
|
||||
final harmonyIndex = order.loadOrder.indexOf('harmony');
|
||||
final rimworldIndex = order.loadOrder.indexOf('ludeon.rimworld');
|
||||
expect(order.errors, isEmpty);
|
||||
expect(harmonyIndex, lessThan(rimworldIndex));
|
||||
});
|
||||
|
||||
@@ -67,9 +75,10 @@ void main() {
|
||||
list.enableAll();
|
||||
final order = list.generateLoadOrder();
|
||||
|
||||
final prepatcherIndex = order.indexOf('prepatcher');
|
||||
final harmonyIndex = order.indexOf('harmony');
|
||||
final rimworldIndex = order.indexOf('ludeon.rimworld');
|
||||
final prepatcherIndex = order.loadOrder.indexOf('prepatcher');
|
||||
final harmonyIndex = order.loadOrder.indexOf('harmony');
|
||||
final rimworldIndex = order.loadOrder.indexOf('ludeon.rimworld');
|
||||
expect(order.errors, isEmpty);
|
||||
expect(prepatcherIndex, greaterThan(harmonyIndex));
|
||||
expect(prepatcherIndex, greaterThan(rimworldIndex));
|
||||
});
|
||||
@@ -89,8 +98,9 @@ void main() {
|
||||
list.enableAll();
|
||||
final order = list.generateLoadOrder();
|
||||
|
||||
final rimworldIndex = order.indexOf('ludeon.rimworld');
|
||||
final anomalyIndex = order.indexOf('ludeon.rimworld.anomaly');
|
||||
final rimworldIndex = order.loadOrder.indexOf('ludeon.rimworld');
|
||||
final anomalyIndex = order.loadOrder.indexOf('ludeon.rimworld.anomaly');
|
||||
expect(order.errors, isEmpty);
|
||||
expect(rimworldIndex, lessThan(anomalyIndex));
|
||||
});
|
||||
|
||||
@@ -105,7 +115,7 @@ void main() {
|
||||
list.disableAll();
|
||||
final order = list.generateLoadOrder();
|
||||
|
||||
final disabledIndex = order.indexOf('disabledDummy');
|
||||
final disabledIndex = order.loadOrder.indexOf('disabledDummy');
|
||||
expect(disabledIndex, isNegative);
|
||||
});
|
||||
|
||||
@@ -122,12 +132,13 @@ void main() {
|
||||
list.enableAll();
|
||||
final order = list.generateLoadOrder();
|
||||
|
||||
final smolIndex = order.indexOf('smol');
|
||||
final yuuugeIndex = order.indexOf('yuuuge');
|
||||
final smolIndex = order.loadOrder.indexOf('smol');
|
||||
final yuuugeIndex = order.loadOrder.indexOf('yuuuge');
|
||||
expect(order.errors, isEmpty);
|
||||
expect(yuuugeIndex, lessThan(smolIndex));
|
||||
});
|
||||
|
||||
test('Incompatible mods should throw exception', () {
|
||||
test('Incompatible mods should return errors', () {
|
||||
final list = ModList();
|
||||
list.mods = {
|
||||
'incompatible': makeDummy().copyWith(
|
||||
@@ -138,7 +149,10 @@ void main() {
|
||||
'harmony': makeDummy().copyWith(name: 'Harmony', id: 'harmony'),
|
||||
};
|
||||
list.enableAll();
|
||||
expect(() => list.generateLoadOrder(), throwsException);
|
||||
final result = list.generateLoadOrder();
|
||||
expect(result.errors, isNotEmpty);
|
||||
expect(result.errors.any((e) => e.contains('incompatible')), isTrue);
|
||||
expect(result.errors.any((e) => e.contains('harmony')), isTrue);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -156,7 +170,8 @@ void main() {
|
||||
list.disableAll();
|
||||
list.setEnabled('prepatcher', true);
|
||||
final order = list.loadRequired();
|
||||
expect(order.indexOf('harmony'), isNot(-1));
|
||||
expect(order.errors, isEmpty);
|
||||
expect(order.loadOrder.indexOf('harmony'), isNot(-1));
|
||||
});
|
||||
|
||||
test('Only required mods should be enabled', () {
|
||||
@@ -173,11 +188,12 @@ void main() {
|
||||
list.disableAll();
|
||||
list.setEnabled('prepatcher', true);
|
||||
final order = list.loadRequired();
|
||||
expect(order.indexOf('harmony'), isNot(-1));
|
||||
expect(order.indexOf('dummy'), -1);
|
||||
expect(order.errors, isEmpty);
|
||||
expect(order.loadOrder.indexOf('harmony'), isNot(-1));
|
||||
expect(order.loadOrder.indexOf('dummy'), -1);
|
||||
});
|
||||
|
||||
test('Incompatible mods should throw exception', () {
|
||||
test('Incompatible mods should return errors', () {
|
||||
final list = ModList();
|
||||
list.mods = {
|
||||
'incompatible': makeDummy().copyWith(
|
||||
@@ -195,7 +211,10 @@ void main() {
|
||||
list.disableAll();
|
||||
list.setEnabled('incompatible', true);
|
||||
list.setEnabled('prepatcher', true);
|
||||
expect(() => list.loadRequired(), throwsException);
|
||||
final result = list.loadRequired();
|
||||
expect(result.errors, isNotEmpty);
|
||||
expect(result.errors.any((e) => e.contains('incompatible')), isTrue);
|
||||
expect(result.errors.any((e) => e.contains('harmony')), isTrue);
|
||||
});
|
||||
test('Dependencies of dependencies should be loaded', () {
|
||||
final list = ModList();
|
||||
@@ -215,14 +234,15 @@ void main() {
|
||||
list.disableAll();
|
||||
list.setEnabled('modA', true);
|
||||
final order = list.loadRequired();
|
||||
expect(order.indexOf('modA'), isNot(-1));
|
||||
expect(order.indexOf('modB'), isNot(-1));
|
||||
expect(order.indexOf('modC'), isNot(-1));
|
||||
expect(order.errors, isEmpty);
|
||||
expect(order.loadOrder.indexOf('modA'), isNot(-1));
|
||||
expect(order.loadOrder.indexOf('modB'), isNot(-1));
|
||||
expect(order.loadOrder.indexOf('modC'), isNot(-1));
|
||||
});
|
||||
});
|
||||
|
||||
group('Test cyclic dependencies', () {
|
||||
test('Cyclic dependencies should throw exception', () {
|
||||
test('Cyclic dependencies should return errors', () {
|
||||
final list = ModList();
|
||||
list.mods = {
|
||||
'modA': makeDummy().copyWith(
|
||||
@@ -243,7 +263,11 @@ void main() {
|
||||
};
|
||||
list.disableAll();
|
||||
list.setEnabled('modA', true);
|
||||
expect(() => list.loadRequired(), throwsException);
|
||||
final result = list.loadRequired();
|
||||
expect(result.errors, isNotEmpty);
|
||||
expect(result.errors.any((e) => e.contains('modA')), isTrue);
|
||||
expect(result.errors.any((e) => e.contains('modB')), isTrue);
|
||||
expect(result.errors.any((e) => e.contains('modC')), isTrue);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -264,10 +288,11 @@ void main() {
|
||||
list.enableAll();
|
||||
final order = list.generateLoadOrder();
|
||||
|
||||
final aIndex = order.indexOf('modA');
|
||||
final bIndex = order.indexOf('modB');
|
||||
final cIndex = order.indexOf('modC');
|
||||
final aIndex = order.loadOrder.indexOf('modA');
|
||||
final bIndex = order.loadOrder.indexOf('modB');
|
||||
final cIndex = order.loadOrder.indexOf('modC');
|
||||
|
||||
expect(order.errors, isEmpty);
|
||||
expect(aIndex, greaterThan(bIndex));
|
||||
expect(aIndex, lessThan(cIndex));
|
||||
});
|
||||
@@ -286,7 +311,9 @@ void main() {
|
||||
'modC': makeDummy().copyWith(name: 'Mod C', id: 'modC'),
|
||||
};
|
||||
list.enableAll();
|
||||
final conflicts = list.checkIncompatibilities();
|
||||
final conflicts = list.checkIncompatibilities(
|
||||
list.activeMods.keys.toList(),
|
||||
);
|
||||
expect(conflicts.length, equals(2));
|
||||
|
||||
// Check if conflicts contain these pairs (order doesn't matter)
|
||||
@@ -349,8 +376,9 @@ void main() {
|
||||
final order = list.generateLoadOrder();
|
||||
|
||||
// Base game should load before any expansions
|
||||
final baseGameIndex = order.indexOf('ludeon.rimworld');
|
||||
final expansionIndex = order.indexOf('ludeon.rimworld.anomaly');
|
||||
final baseGameIndex = order.loadOrder.indexOf('ludeon.rimworld');
|
||||
final expansionIndex = order.loadOrder.indexOf('ludeon.rimworld.anomaly');
|
||||
expect(order.errors, isEmpty);
|
||||
expect(baseGameIndex, lessThan(expansionIndex));
|
||||
});
|
||||
});
|
||||
@@ -381,16 +409,268 @@ void main() {
|
||||
|
||||
final result = list.loadRequired();
|
||||
|
||||
expect(result.errors, isEmpty);
|
||||
// All mods in the chain should be enabled
|
||||
expect(result.contains('modA'), isTrue);
|
||||
expect(result.contains('modB'), isTrue);
|
||||
expect(result.contains('modC'), isTrue);
|
||||
expect(result.contains('modD'), isTrue);
|
||||
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.indexOf('modD'), lessThan(result.indexOf('modC')));
|
||||
expect(result.indexOf('modC'), lessThan(result.indexOf('modB')));
|
||||
expect(result.indexOf('modB'), lessThan(result.indexOf('modA')));
|
||||
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')),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
group('Test missing dependencies', () {
|
||||
test('Should detect missing dependencies and return errors', () {
|
||||
final list = ModList();
|
||||
list.mods = {
|
||||
'modA': makeDummy().copyWith(
|
||||
name: 'Mod A',
|
||||
id: 'modA',
|
||||
dependencies: ['modB', 'nonExistentMod'],
|
||||
),
|
||||
'modB': makeDummy().copyWith(name: 'Mod B', id: 'modB'),
|
||||
};
|
||||
list.enableAll();
|
||||
|
||||
// This should throw an exception because the dependency doesn't exist
|
||||
final result = list.generateLoadOrder();
|
||||
expect(result.errors, isNotEmpty);
|
||||
expect(result.errors.any((e) => e.contains('nonExistentMod')), isTrue);
|
||||
});
|
||||
|
||||
test('Should handle multiple missing dependencies correctly', () {
|
||||
final list = ModList();
|
||||
list.mods = {
|
||||
'modA': makeDummy().copyWith(
|
||||
name: 'Mod A',
|
||||
id: 'modA',
|
||||
dependencies: ['missing1'],
|
||||
),
|
||||
'modB': makeDummy().copyWith(
|
||||
name: 'Mod B',
|
||||
id: 'modB',
|
||||
dependencies: ['missing2'],
|
||||
),
|
||||
};
|
||||
list.enableAll();
|
||||
|
||||
// Should return errors with both missing dependencies mentioned
|
||||
final result = list.generateLoadOrder();
|
||||
expect(result.errors, isNotEmpty);
|
||||
expect(result.errors.any((e) => e.contains('missing1')), isTrue);
|
||||
expect(result.errors.any((e) => e.contains('missing2')), isTrue);
|
||||
});
|
||||
});
|
||||
|
||||
group('Test missing loadBefore/loadAfter relationships', () {
|
||||
test('Should handle missing loadBefore relationships', () {
|
||||
final list = ModList();
|
||||
list.mods = {
|
||||
'modA': makeDummy().copyWith(
|
||||
name: 'Mod A',
|
||||
id: 'modA',
|
||||
loadBefore: ['nonExistentMod'],
|
||||
),
|
||||
};
|
||||
list.enableAll();
|
||||
|
||||
// Should not throw exception for soft constraints
|
||||
// But might generate a warning that we could check for
|
||||
final order = list.generateLoadOrder();
|
||||
expect(order.errors, isEmpty);
|
||||
expect(order.loadOrder.contains('modA'), isTrue);
|
||||
});
|
||||
|
||||
test('Should handle missing loadAfter relationships', () {
|
||||
final list = ModList();
|
||||
list.mods = {
|
||||
'modA': makeDummy().copyWith(
|
||||
name: 'Mod A',
|
||||
id: 'modA',
|
||||
loadAfter: ['nonExistentMod'],
|
||||
),
|
||||
};
|
||||
list.enableAll();
|
||||
|
||||
// Should not throw exception for soft constraints
|
||||
final order = list.generateLoadOrder();
|
||||
expect(order.errors, isEmpty);
|
||||
expect(order.loadOrder.contains('modA'), isTrue);
|
||||
});
|
||||
});
|
||||
|
||||
group('Test BuildLoadOrder error handling', () {
|
||||
test('Should return errors for incompatibilities', () {
|
||||
final list = ModList();
|
||||
list.mods = {
|
||||
'modA': makeDummy().copyWith(
|
||||
name: 'Mod A',
|
||||
id: 'modA',
|
||||
incompatibilities: ['modB'],
|
||||
),
|
||||
'modB': makeDummy().copyWith(name: 'Mod B', id: 'modB'),
|
||||
};
|
||||
list.enableAll();
|
||||
|
||||
final result = list.generateLoadOrder();
|
||||
expect(result.errors, isNotEmpty);
|
||||
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);
|
||||
});
|
||||
|
||||
test('Should handle a combination of errors simultaneously', () {
|
||||
final list = ModList();
|
||||
list.mods = {
|
||||
'modA': makeDummy().copyWith(
|
||||
name: 'Mod A',
|
||||
id: 'modA',
|
||||
dependencies: ['missingDep'],
|
||||
incompatibilities: ['modB'],
|
||||
),
|
||||
'modB': makeDummy().copyWith(name: 'Mod B', id: 'modB'),
|
||||
};
|
||||
list.enableAll();
|
||||
|
||||
final result = list.generateLoadOrder();
|
||||
expect(result.errors, isNotEmpty);
|
||||
expect(result.errors.any((e) => e.contains('missingDep')), isTrue);
|
||||
expect(result.errors.any((e) => e.contains('Incompatible')), isTrue);
|
||||
});
|
||||
});
|
||||
|
||||
group('Test dependency resolution with constraints', () {
|
||||
test(
|
||||
'Should resolve dependencies while respecting load order constraints',
|
||||
() {
|
||||
final list = ModList();
|
||||
list.mods = {
|
||||
'modA': makeDummy().copyWith(
|
||||
name: 'Mod A',
|
||||
id: 'modA',
|
||||
dependencies: ['modB'],
|
||||
loadAfter: ['modC'],
|
||||
),
|
||||
'modB': makeDummy().copyWith(name: 'Mod B', id: 'modB'),
|
||||
'modC': makeDummy().copyWith(name: 'Mod C', id: 'modC'),
|
||||
};
|
||||
list.enableAll();
|
||||
|
||||
final order = list.generateLoadOrder();
|
||||
expect(order.errors, isEmpty);
|
||||
|
||||
// modB should load before modA due to dependency
|
||||
expect(
|
||||
order.loadOrder.indexOf('modB'),
|
||||
lessThan(order.loadOrder.indexOf('modA')),
|
||||
);
|
||||
|
||||
// modC should load before modA due to loadAfter constraint
|
||||
expect(
|
||||
order.loadOrder.indexOf('modC'),
|
||||
lessThan(order.loadOrder.indexOf('modA')),
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
test('Should detect and report conflicting constraints', () {
|
||||
final list = ModList();
|
||||
list.mods = {
|
||||
'modA': makeDummy().copyWith(
|
||||
name: 'Mod A',
|
||||
id: 'modA',
|
||||
loadBefore: ['modB'],
|
||||
),
|
||||
'modB': makeDummy().copyWith(
|
||||
name: 'Mod B',
|
||||
id: 'modB',
|
||||
loadBefore: ['modA'],
|
||||
),
|
||||
};
|
||||
list.enableAll();
|
||||
|
||||
// These constraints create a circular dependency which should cause an error
|
||||
try {
|
||||
list.generateLoadOrder();
|
||||
fail('Expected an exception to be thrown due to circular constraints');
|
||||
} catch (e) {
|
||||
// Verify error is about circular dependencies or conflicting constraints
|
||||
expect(
|
||||
e.toString().toLowerCase().contains('conflict') ||
|
||||
e.toString().toLowerCase().contains('circular'),
|
||||
isTrue,
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
group('Test BuildLoadOrder with result object', () {
|
||||
test('Should return successful load order with no errors', () {
|
||||
final list = ModList();
|
||||
list.mods = {
|
||||
'modA': makeDummy().copyWith(name: 'Mod A', id: 'modA'),
|
||||
'modB': makeDummy().copyWith(name: 'Mod B', id: 'modB'),
|
||||
};
|
||||
list.enableAll();
|
||||
|
||||
final result = list.generateLoadOrder();
|
||||
|
||||
expect(result.errors, isEmpty);
|
||||
expect(result.loadOrder.length, equals(2));
|
||||
expect(result.loadOrder.contains('modA'), isTrue);
|
||||
expect(result.loadOrder.contains('modB'), isTrue);
|
||||
});
|
||||
|
||||
test('Should return errors for missing dependencies', () {
|
||||
final list = ModList();
|
||||
list.mods = {
|
||||
'modA': makeDummy().copyWith(
|
||||
name: 'Mod A',
|
||||
id: 'modA',
|
||||
dependencies: ['nonExistentMod'],
|
||||
),
|
||||
};
|
||||
list.enableAll();
|
||||
|
||||
final result = list.generateLoadOrder();
|
||||
|
||||
expect(result.errors, isNotEmpty);
|
||||
expect(result.errors.any((e) => e.contains('nonExistentMod')), isTrue);
|
||||
// The load order might be empty or contain valid mods only
|
||||
});
|
||||
|
||||
test('Should return both valid load order and errors', () {
|
||||
final list = ModList();
|
||||
list.mods = {
|
||||
'modA': makeDummy().copyWith(name: 'Mod A', id: 'modA'),
|
||||
'modB': makeDummy().copyWith(
|
||||
name: 'Mod B',
|
||||
id: 'modB',
|
||||
dependencies: ['nonExistentMod'],
|
||||
),
|
||||
};
|
||||
list.enableAll();
|
||||
|
||||
final result = list.generateLoadOrder();
|
||||
|
||||
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
|
||||
});
|
||||
});
|
||||
}
|
||||
|
Reference in New Issue
Block a user