SpaceTradersAPI / issues-and-suggestions

16 stars 1 forks source link

Add ship endpoint for single ship #13

Closed jadlers closed 3 years ago

ramonGonzEdu commented 3 years ago

Should contain movement object is ship is moving, rather than just not having location property

HamishWHC commented 3 years ago

What form and key would this movement/location property take?

jadlers commented 3 years ago

At the moment you can deduce the ship being in flight from it not having a location property. See example below where ships[0] has a location while ships[1] doesn't.

But inFlight: true is a suggestion of mine should it be added.

{
  "ships": [
      {
          "cargo": [
              {
                  "good": "FUEL",
                  "quantity": 16
              }
          ],
          "class": "MK-I",
          "id": "ckluvaajr415740c8898hhtxc99",
          "location": "OE-D2",
          "manufacturer": "Gravager",
          "maxCargo": 300,
          "plating": 10,
          "spaceAvailable": 284,
          "speed": 1,
          "type": "GR-MK-I",
          "weapons": 5
      },
      {
          "cargo": [
              {
                  "good": "FUEL",
                  "quantity": 43
              }
          ],
          "class": "MK-I",
          "id": "ckluukpdv402033c889uqfe9s6q",
          "manufacturer": "Gravager",
          "maxCargo": 300,
          "plating": 10,
          "spaceAvailable": 257,
          "speed": 1,
          "type": "GR-MK-I",
          "weapons": 5
      }
  ]
}
HamishWHC commented 3 years ago

I feel like it should provide the flight plan, not just that it is in flight.

jadlers commented 3 years ago

Adding the flightPlanId to the ship might be something worth adding. Issue #30 is accepted and would allow you to get the flight plan. You could just hit that endpoint instead if you know it's in flight. Note that this is just me surfacing my thoughts!

HamishWHC commented 3 years ago

Why shouldn't the entire flight plan be provided?

ramonGonzEdu commented 3 years ago

Why shouldn't the entire flight plan be provided?

Flight plan id should be provided so that the server doesn't have to send massive amounts of data when you only need the available property. If you do need the flight Plan, then you can request that separately

HamishWHC commented 3 years ago

Massive amounts of data? It's maybe an extra byte of data per ship! The ships themselves count for a lot more, and including an extra byte of data per ship (only if they are in flight) will be a lot more performant than another query (for the server and the client). All you have to do is add a JOIN to the current ship SQL query and then nicely JSON format it.

ramonGonzEdu commented 3 years ago

A byte is one letter or character so the flight plan (https://spacetraders.io/#create-flight-plan) is around 287 bytes rather than the much smaller id. This will also set a precedent, so if more data is added to ships, the ship endpoint will also include that data which will slow down clients on bad internet. And massive amounts is relative to the necessary amount for most ship endpoint requests, 0 bytes or 16 bytes for the id.

HamishWHC commented 3 years ago

Sorry, meant KB. Still shouldn't make a difference to most. IMO as much data as possible should be returned, as that reduces the amount of requests the client has to send to the server. Another request is 99% of the time going to be more processing than a SQL JOIN. If people playing an API game are concerned about the time to download a few KB of data, then they should move their code to somewhere with better internet.