Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Waypoint/SystemWaypoint schema inconsistency #45

Open
yamin8000 opened this issue May 18, 2023 · 2 comments
Open

Waypoint/SystemWaypoint schema inconsistency #45

yamin8000 opened this issue May 18, 2023 · 2 comments

Comments

@yamin8000
Copy link

Get System endpoint https://api.spacetraders.io/v2 /systems/{systemSymbol} response includes a waypoints object which is a list of Waypoint.

image

However, Get Waypoint endpoint https://api.spacetraders.io/v2 /systems/{systemSymbol}/waypoints/{waypointSymbol} returns a more detailed waypoint with more parameters. Why?
Why not return a detailed Waypoint in both endpoints?

image

I mean client could define two classes one named Waypoint with more detailed information and another compact called SystemWaypoint but they are fundamentally the same entity so it's better to treat them as such.

@tyrope
Copy link

tyrope commented May 28, 2023

Same with ShipNavRouteWaypoint.

They could all be collapsed into the Waypoint schema with some removal of required flags.

Edit: The same situation can be applied for ScannedShip vs Ship

@space-admiral
Copy link
Contributor

@yamin8000 ya I agree, I think originally this was set up to make it simpler to plot a system and all the waypoints without needing a beginner to send multiple requests to piece things together, but I don't think it makes a lot of sense in hindsight. I'm thinking we might remove the waypoints property entirely from System and just require the call to /systems/{systemSymbol}/waypoints.

As for ScannedShip vs Ship, that one is a bit more interesting. I can share that the intention here is that a "scanned ship" can only show you details of what you might see externally, and so the properties are limited relative to a ship you own. I agree you could accomplish this by leveraging the Ship schema and just making many of the internal properties optional, but that would be a bit cumbersome on clients to now have to do fairly constant property checks on ship properties, when the use cases are quite different. I think I prefer in this case to keep the two models separate.

I think our intention is that on any endpoint where the use cases or perspectives are largely different, the preference is to keep the types separate even if they are similar views of the same underlying object. Having said that, it's definitely a tricky area we're still thinking about - if anyone has some suggestions, we would love to consider them. Some of the design goals include:

  • don't burden clients with constant optional property checks
  • cleanly separate static data that can be cached from dynamic data at the route level to leverage native HTTP caching
  • limit the complexity of how many requests you have to send to pull data about a single object, such as a ship, to make the game more approachable when you are first getting started
  • try to limit how many near-identical representations exist of the same schema across different endpoints (this is your point above)

You can see some of these goals are in conflict, and thats what makes it so tricky. Thanks for reporting this and I hope to get some more thoughts if you have some design suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants