GeoguessrTools / calico-cut-pants

This is the API for serving up country tips
MIT License
0 stars 0 forks source link

Support mutliple maps #4

Open TheConcierge opened 2 years ago

TheConcierge commented 2 years ago

Currently, only one map is supported, intended to be used as just a general country map. However, other maps are sometimes needed, such as meta driving maps.

TheConcierge commented 2 years ago

Currently, only one map is supported, intended to be used as just a general country map. However, other maps are sometimes needed, such as meta driving maps.

Yeah, clearly there is a need for multiple maps...It's easy enough to just make that a list of ImageWithDescription types so you have labels associated with each as well. That being said, does it make sense to reserve a top level spot for the broad county map? Or do you not plan on giving that particular map preferential placement?

TheConcierge commented 2 years ago

Currently, only one map is supported, intended to be used as just a general country map. However, other maps are sometimes needed, such as meta driving maps.

Yeah, clearly there is a need for multiple maps...It's easy enough to just make that a list of ImageWithDescription types so you have labels associated with each as well. That being said, does it make sense to reserve a top level spot for the broad county map? Or do you not plan on giving that particular map preferential placement?

Thinking about it, this might also be a good time to add enum identifiers to the ImageWithDescription struct. So for maps we could have something like:

{
   image_url: "https://this.is.an.example/country-map",
   description: "The country of germany is not super well covered. It's mainly large cities",
   identifier: COUNTRY_MAP
}
daseinkapital commented 2 years ago

Thinking about it, this might also be a good time to add enum identifiers to the ImageWithDescription struct. So for maps we could have something like:


{
  image_url: "https://this.is.an.example/country-map",
  description: "The country of germany is not super well covered. It's mainly large cities",
  identifier: COUNTRY_MAP
}

Yeah I think that map can go in the map section (theoretically they are already looking at the map on Geoguessr. We could also just have that map in particular in two places, one just showing it, and the general info section highlighting major cities in the country (maybe a top 5 big cities?)

TheConcierge commented 2 years ago

i'd rather not store it in two places because that might be annoying if we need to update them.

I'll probably just include it in the map list, with an identifier. It's a liiiiitle bit more work on your part as you'll have to iterate looking for it to display it front and center if that's what you want.

TheConcierge commented 2 years ago

unless you have any objections

daseinkapital commented 2 years ago

You can just leave it as is and close the issue

On Wed, Sep 28, 2022, 10:00 PM Ryan Grant @.***> wrote:

unless you have any objections

— Reply to this email directly, view it on GitHub https://github.com/GeoguessrTools/calico-cut-pants/issues/4#issuecomment-1261648337, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADXGV37NR5X4YOANAMEE4JLWATZ25ANCNFSM6AAAAAAQXGYYFM . You are receiving this because you commented.Message ID: @.***>

daseinkapital commented 2 years ago

Wait sorry I was in the wrong issue thread. Don't reply to issue threads through email lol. I think having to pass one extra map into general isn't that bad, but I'd rather group all the maps so if you go to our API and call maps, you get all maps for a country.

On Wed, Sep 28, 2022, 10:04 PM Andrew Samuelson @.***> wrote:

You can just leave it as is and close the issue

On Wed, Sep 28, 2022, 10:00 PM Ryan Grant @.***> wrote:

unless you have any objections

— Reply to this email directly, view it on GitHub https://github.com/GeoguessrTools/calico-cut-pants/issues/4#issuecomment-1261648337, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADXGV37NR5X4YOANAMEE4JLWATZ25ANCNFSM6AAAAAAQXGYYFM . You are receiving this because you commented.Message ID: @.***>

TheConcierge commented 2 years ago

Oh, had a thought. Instead of an identifier, what if we did this:

maps: {
    country_map: {
          image_url: this.is.my.country.map/map
          description: lmao
    }. 
    city_maps: {
          {
             image_url: city.maps/carlsbad,
             description: Carlsbad
          }. 
         {
             image_url: city.maps/oceanside,
             description: Oceanside
         }
    }
}

or something like that? I am starting to feel like hardcoded categorical identifiers are a little bit of overhead when we can structure the data to represent that a little better?

daseinkapital commented 2 years ago

I personally don't see a need for maps of specific cities. When I was thinking of maps, I was thinking of everything at the country level, just the information it would give you would be different (one map has major cities listed, one map has the topology, one map shows average humidity, one map has vegetation, etc.). Also, maybe I don't understand the data type, but is that supposed to be a list of cities in the city map because it looks like an object of objects so it doesn't have a key

TheConcierge commented 2 years ago

yeah it's supposed to be a list, i forgot to add the brackets

daseinkapital commented 2 years ago

Okay yeah I'm fine with that being more explicit like that.

TheConcierge commented 2 years ago

If we are going to have pretty strict category of maps, we could even just list the map types as top level keys?

maps: {
    major_cities: {
        <map info>
    }, 
    topological: {
       <map_info>
    }
    ...
}

easier to identify, parse, sort, etc. but also more rigid.

If we don't have a type of map, I can just have the api not return it so you can iterate over keys and display if exists or something like that.

daseinkapital commented 2 years ago

Iterating over an object is always easy because in Javascript you can do Object.keys(obj).map(key => {*do stuff}) so I don't mind iterative over keys. Maybe each map has an optional description though in case it's something that's not as easy to understand without the context (or even possibly just a title is fine). I like this most recent way you've laid out though

If we are going to have pretty strict category of maps, we could even just list the map types as top level keys?

maps: {
    major_cities: {
        <map info>
    }, 
    topological: {
       <map_info>
    }
    ...
}

easier to identify, parse, sort, etc. but also more rigid.

If we don't have a type of map, I can just have the api not return it so you can iterate over keys and display if exists or something like that.