Skip to content

Commit

Permalink
refactor: Use our own Waypoint, System and SystemWaypoint types
Browse files Browse the repository at this point in the history
Previously we used the OpenAPI types directly, now we wrap them.

This was initially done for a performance win (so we could cache *Symbol objects),
but as I did it, it became clear it's also a huge
code cleanup win.
  • Loading branch information
eseidel committed Dec 27, 2023
1 parent b52492a commit fed0c51
Show file tree
Hide file tree
Showing 45 changed files with 558 additions and 527 deletions.
4 changes: 2 additions & 2 deletions packages/cli/bin/deals_nearby.dart
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Future<void> cliMain(FileSystem fs, ArgResults argResults) async {
: systemsCache.waypointFromString(startArg)!;

final jumpGate = systemsCache.jumpGateWaypointForSystem(start.systemSymbol)!;
final construction = constructionSnapshot[jumpGate.waypointSymbol];
final construction = constructionSnapshot[jumpGate.symbol];
centralCommand.activeConstruction = construction;

final extraSellOpps = <SellOpp>[];
Expand Down Expand Up @@ -88,7 +88,7 @@ Future<void> cliMain(FileSystem fs, ArgResults argResults) async {
cargoCapacity: cargoCapacity,
fuelCapacity: fuelCapacity,
shipSpeed: shipSpeed,
startSymbol: start.waypointSymbol,
startSymbol: start.symbol,
extraSellOpps: extraSellOpps,
);

Expand Down
2 changes: 1 addition & 1 deletion packages/cli/bin/exports_supply_chain.dart
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ WaypointSymbol? nearestExtractionSiteFor(
..sort(
(a, b) => a.distanceTo(destination).compareTo(b.distanceTo(destination)),
);
return candidates.firstOrNull?.waypointSymbol;
return candidates.firstOrNull?.symbol;
}

MarketListing? nearestListingWithExport(
Expand Down
11 changes: 4 additions & 7 deletions packages/cli/bin/routing_tests.dart
Original file line number Diff line number Diff line change
Expand Up @@ -238,21 +238,18 @@ void runTests(TestSuite suite, String path) {
);
waypoints.add(
SystemWaypoint(
symbol: waypointSymbol.waypoint,
symbol: waypointSymbol,
type: WaypointType.ASTEROID,
x: waypoint.x,
y: waypoint.y,
position: WaypointPosition(waypoint.x, waypoint.y, systemSymbol),
),
);
}
systems.add(
System(
symbol: systemSymbol.system,
sectorSymbol: systemSymbol.sector,
symbol: systemSymbol,
waypoints: waypoints,
type: SystemType.NEBULA,
x: 0,
y: 0,
position: const SystemPosition(0, 0),
),
);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/bin/waypoint_reachability.dart
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ Future<void> command(FileSystem fs, ArgResults argResults) async {
final waypoint = systemsCache.waypoint(waypointSymbol);
logger.info(
' ${waypointSymbol.waypoint.padRight(11)} '
'@ ${waypoint.x},${waypoint.y}',
'@ ${waypoint.position}',
);
}
}
Expand Down
5 changes: 2 additions & 3 deletions packages/cli/lib/api.dart
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import 'package:cli/net/counts.dart';
import 'package:http/http.dart' as http;
import 'package:openapi/api.dart';

export 'package:openapi/api.dart';
import 'package:openapi/api.dart'
show AgentsApi, ContractsApi, DefaultApi, FactionsApi, FleetApi, SystemsApi;

/// The default http get function.
const defaultHttpGet = http.get;
Expand Down
1 change: 1 addition & 0 deletions packages/cli/lib/cache/caches.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import 'package:db/db.dart';
import 'package:file/file.dart';
import 'package:http/http.dart' as http;
import 'package:meta/meta.dart';
import 'package:types/types.dart';

export 'package:cli/api.dart';
export 'package:cli/cache/agent_cache.dart';
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/lib/cache/static_cache.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import 'package:cli/cache/caches.dart';
import 'package:cli/cache/json_list_store.dart';
import 'package:cli/compare.dart';
import 'package:collection/collection.dart';
import 'package:types/export.dart';
import 'package:types/types.dart';

// Not using named parameters to save repetition at call sites.
List<Value> _loadJsonNullable<Value>(
Expand Down
7 changes: 3 additions & 4 deletions packages/cli/lib/cache/systems_cache.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class SystemsCache extends JsonListStore<System> {
static List<System> _parseSystems(String systemsString) {
final parsed = jsonDecode(systemsString) as List<dynamic>;
return parsed
.map<System>((e) => System.fromJson(e as Map<String, dynamic>)!)
.map<System>((e) => System.fromJson(e as Map<String, dynamic>))
.toList();
}

Expand All @@ -48,7 +48,7 @@ class SystemsCache extends JsonListStore<System> {
final systems = JsonListStore.loadRecords<System>(
fs,
path,
(json) => System.fromJson(json)!,
System.fromJson,
);
if (systems == null) {
return null;
Expand Down Expand Up @@ -103,8 +103,7 @@ class SystemsCache extends JsonListStore<System> {
/// Fetch the waypoint with the given symbol, or null if it does not exist.
SystemWaypoint? waypointOrNull(WaypointSymbol waypointSymbol) {
final waypoints = waypointsInSystem(waypointSymbol.systemSymbol);
return waypoints
.firstWhereOrNull((w) => w.symbol == waypointSymbol.waypoint);
return waypoints.firstWhereOrNull((w) => w.symbol == waypointSymbol);
}

/// Return the SystemWaypoint for the given [symbol].
Expand Down
9 changes: 3 additions & 6 deletions packages/cli/lib/cache/waypoint_cache.dart
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,9 @@ class WaypointCache {
}

return Waypoint(
symbol: systemWaypoint.symbol,
symbol: systemWaypoint.waypointSymbol,
type: systemWaypoint.type,
systemSymbol: systemWaypoint.systemSymbol.system,
x: systemWaypoint.x,
y: systemWaypoint.y,
position: systemWaypoint.position,
chart: values.chart,
faction: values.faction,
orbitals: systemWaypoint.orbitals,
Expand Down Expand Up @@ -183,8 +181,7 @@ class WaypointCache {
return cachedWaypoint;
}
final waypoints = await waypointsInSystem(systemSymbol);
return waypoints
.firstWhereOrNull((w) => w.symbol == waypointSymbol.waypoint);
return waypoints.firstWhereOrNull((w) => w.symbol == waypointSymbol);
}

/// Returns true if the given waypoint is known to be charted, will
Expand Down
1 change: 1 addition & 0 deletions packages/cli/lib/cli.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import 'package:cli/logger.dart';
import 'package:file/local.dart';
import 'package:meta/meta.dart';
import 'package:scoped/scoped.dart';
import 'package:types/types.dart';

export 'package:args/args.dart';
export 'package:cli/logger.dart';
Expand Down
1 change: 1 addition & 0 deletions packages/cli/lib/logic.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'package:cli/net/exceptions.dart';
import 'package:cli/printing.dart';
import 'package:cli/ship_waiter.dart';
import 'package:db/db.dart';
import 'package:types/types.dart';

// Pulled in to a separate function to help make sure we don't confuse
// the wait we needed for this ship with the next wait.
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/lib/nav/route.dart
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ RoutePlan routePlanFromJumpPlan(
final startWaypoint = systemsCache.waypoint(start);
final startJumpGate =
systemsCache.jumpGateWaypointForSystem(start.systemSymbol)!;
if (startJumpGate.symbol != start.waypoint) {
if (startJumpGate.symbol != start) {
_addSubPlanWithinSystem(
systemsCache,
actions,
Expand Down Expand Up @@ -197,7 +197,7 @@ RoutePlan routePlanFromJumpPlan(
}
final endWaypoint = systemsCache.waypoint(end);
final endJumpGate = systemsCache.jumpGateWaypointForSystem(end.systemSymbol)!;
if (endJumpGate.symbol != end.waypoint) {
if (endJumpGate.symbol != end) {
_addSubPlanWithinSystem(
systemsCache,
actions,
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/lib/net/actions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,8 @@ Future<void> chartWaypointAndLog(
) async {
try {
final response = await api.fleet.createChart(ship.symbol);
final waypoint = response!.data.waypoint;
final openapiWaypoint = response!.data.waypoint;
final waypoint = Waypoint.fromOpenApi(openapiWaypoint);
await chartingCache.addWaypoint(waypoint);
waypointTraitCache.addAll(waypoint.traits);
// Powershell needs the space after the emoji.
Expand Down
1 change: 1 addition & 0 deletions packages/cli/lib/net/auth.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import 'package:cli/net/counts.dart';
import 'package:cli/net/queue.dart';
import 'package:db/db.dart';
import 'package:file/file.dart';
import 'package:types/types.dart';

export 'package:cli/net/queue.dart'
show networkPriorityDefault, networkPriorityLow;
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/lib/net/counts.dart
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import 'dart:async';

import 'package:cli/api.dart';
import 'package:cli/logger.dart';
import 'package:http/http.dart';
import 'package:types/types.dart';

/// RequestCounts tracks the number of requests made to each path.
class RequestCounts {
Expand Down
23 changes: 22 additions & 1 deletion packages/cli/lib/net/exceptions.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,28 @@
import 'dart:convert';

import 'package:cli/api.dart';
import 'package:cli/logger.dart';
import 'package:types/types.dart';

/// Returns a valid [T] value found at the specified Map [key], null otherwise.
T? mapValueOfType<T>(dynamic map, String key) {
final dynamic value = map is Map ? map[key] : null;
return value is T ? value : null;
}

/// Returns a valid Map<K, V> found at the specified Map [key], null otherwise.
Map<K, V>? mapCastOfType<K, V>(dynamic map, String key) {
final dynamic value = map is Map ? map[key] : null;
return value is Map ? value.cast<K, V>() : null;
}

/// Returns a valid [DateTime] found at the specified Map [key], null otherwise.
DateTime? mapDateTime(dynamic map, String key) {
final dynamic value = map is Map ? map[key] : null;
if (value is String) {
return DateTime.tryParse(value);
}
return null;
}

/// Error 4224 is a survey exhausted error.
bool isSurveyExhaustedException(ApiException e) {
Expand Down
6 changes: 4 additions & 2 deletions packages/cli/lib/net/queries.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,17 @@ Stream<T> fetchAllPages<T, A>(
Future<Waypoint> fetchWaypoint(Api api, WaypointSymbol waypointSymbol) async {
final response = await api.systems
.getWaypoint(waypointSymbol.system, waypointSymbol.waypoint);
return response!.data;
final openApiWaypoint = response!.data;
return Waypoint.fromOpenApi(openApiWaypoint);
}

/// Fetches all waypoints in a system. Handles pagination from the server.
Stream<Waypoint> allWaypointsInSystem(Api api, SystemSymbol system) {
return fetchAllPages(api, (api, page) async {
final response = await api.systems
.getSystemWaypoints(system.system, page: page, limit: pageLimit);
return (response!.data, response.meta);
final waypoints = response!.data.map(Waypoint.fromOpenApi).toList();
return (waypoints, response.meta);
});
}

Expand Down
1 change: 1 addition & 0 deletions packages/cli/lib/net/register.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import 'package:cli/logger.dart';
import 'package:cli/net/auth.dart';
import 'package:cli/net/exceptions.dart';
import 'package:db/db.dart';
import 'package:types/types.dart';

/// loadAuthTokenOrRegister loads the auth token from the given file system
/// or registers a new user and returns the auth token.
Expand Down
13 changes: 7 additions & 6 deletions packages/cli/lib/ships.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import 'package:cli/cache/static_cache.dart';
import 'package:collection/collection.dart';
import 'package:meta/meta.dart';
import 'package:types/api.dart';
import 'package:types/types.dart';

/// Map from ship type to ship frame symbol.
extension ShipTypeToFrame on ShipyardShipCache {
Expand Down Expand Up @@ -39,12 +39,13 @@ extension ShipTypeToFrame on ShipyardShipCache {
}) {
final symbolString = shipSymbol?.symbol ?? 'S-1';
final factionString = factionSymbol?.value ?? 'COSMIC';
final waypointSymbol =
origin?.waypointSymbol ?? WaypointSymbol.fromString('A-B-C');
final waypoint = origin ??
SystemWaypoint(
symbol: 'A-B-C',
symbol: waypointSymbol,
type: WaypointType.PLANET,
x: 0,
y: 0,
position: WaypointPosition(0, 0, waypointSymbol.systemSymbol),
);
final arrival = now ?? DateTime.utc(2021);

Expand Down Expand Up @@ -122,8 +123,8 @@ ShipNav _makeShipNav({required SystemWaypoint origin, required DateTime now}) {
symbol: originSymbol.waypoint,
systemSymbol: originSymbol.system,
type: origin.type,
x: origin.x,
y: origin.y,
x: origin.position.x,
y: origin.position.y,
);

return ShipNav(
Expand Down
18 changes: 5 additions & 13 deletions packages/cli/test/behavior/central_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -514,13 +514,10 @@ void main() {
.thenReturn(hqSystemSymbol);
when(() => caches.waypoints.waypointsInSystem(hqSystemSymbol)).thenAnswer(
(_) => Future.value([
Waypoint(
symbol: hqSymbol.waypoint,
Waypoint.test(
hqSymbol,
type: WaypointType.PLANET,
systemSymbol: hqSymbol.system,
x: 0,
y: 0,
isUnderConstruction: false,
position: WaypointPosition(0, 0, hqSystemSymbol),
traits: [
WaypointTrait(
symbol: WaypointTraitSymbol.SHIPYARD,
Expand Down Expand Up @@ -550,17 +547,12 @@ void main() {
),
).thenReturn(0);
when(() => caches.agent.headquarters(caches.systems)).thenReturn(
SystemWaypoint(
symbol: 'W-A-Y',
type: WaypointType.ASTEROID,
x: 0,
y: 0,
),
SystemWaypoint.test(hqSymbol),
);
when(() => caches.agent.agent).thenReturn(
Agent(
symbol: shipSymbol.agentName,
headquarters: 'W-A-Y',
headquarters: hqSymbol.waypoint,
credits: 100000,
shipCount: 1,
startingFaction: faction,
Expand Down
Loading

0 comments on commit fed0c51

Please sign in to comment.