odota / core

Open source Dota 2 data platform
https://www.opendota.com
MIT License
1.52k stars 303 forks source link

why disallow /players in robots.txt? #625

Closed TrentBartlem closed 9 years ago

TrentBartlem commented 9 years ago

When I go to friends' places to show them my profile, I have to log in via steam instead of just googling my name + yasp :(

howardchung commented 9 years ago

Player pages are relatively expensive to compute, so we didn't want search engines spamming those pages. On Sep 11, 2015 2:16 AM, "Trent Bartlem" notifications@github.com wrote:

When I go to friends' places to show them my profile, I have to log in via steam instead of just googling my name + yasp :(

— Reply to this email directly or view it on GitHub https://github.com/yasp-dota/yasp/issues/625.

lomash commented 9 years ago

Allowing users to sign up for an alias might be a good idea. That way they can access their profiles using a custom URL of the format http://yasp.co/players/*alias which is way easier to remember than a random player ID http://yasp.co/players/player id*

lomash commented 9 years ago

I'm not sure how this works but URLs of this form direct you to a default profile right now. http://yasp.co/players/SomeRandomDude http://yasp.co/players/AnotherRandomDude

howardchung commented 9 years ago

This is actually a pretty good idea, although I dislike having to store custom user-supplied information.

We'd need to supply a POST endpoint, and create a unique index for the alias to prevent conflicts and for efficient search. On Sep 11, 2015 12:28 PM, "Lomash" notifications@github.com wrote:

Allowing users to sign up for an alias might be a good idea. That way they can access their profiles using a custom URL of the format http://yasp.co/players/*alias which is way easier to remember than a random player ID http://yasp.co/players/player id*

— Reply to this email directly or view it on GitHub https://github.com/yasp-dota/yasp/issues/625#issuecomment-139640977.

howardchung commented 9 years ago

I think what's happening is that we try to convert the string to number, get NaN, and search the db with it. Since there is no player criteria we restrict results to 2000. On Sep 11, 2015 12:32 PM, "Howard Chung" howardc93@gmail.com wrote:

This is actually a pretty good idea, although I dislike having to store custom user-supplied information.

We'd need to supply a POST endpoint, and create a unique index for the alias to prevent conflicts and for efficient search. On Sep 11, 2015 12:28 PM, "Lomash" notifications@github.com wrote:

Allowing users to sign up for an alias might be a good idea. That way they can access their profiles using a custom URL of the format http://yasp.co/players/*alias which is way easier to remember than a random player ID http://yasp.co/players/player id*

— Reply to this email directly or view it on GitHub https://github.com/yasp-dota/yasp/issues/625#issuecomment-139640977.

howardchung commented 9 years ago

Also some potential for abuse. Since the system would presumably be first-come first-served random players could grab aliases like "dendi" and confuse people.

ghost commented 9 years ago

@TrentBartlem your profile URL stays the same per account. You could bookmark it, or make a memorable bit.ly link even.

howardchung commented 9 years ago

Yes, a memorable URL-shortened link would also be a viable solution :)

TrentBartlem commented 9 years ago

Well you could allow them but set a cache-control header so they only check back once a month or so?

howardchung commented 9 years ago

To be more precise, since we now cache player pages on load, that would mean we have to store a lot more profiles, which costs a lot of space.

ahockersten commented 9 years ago

Since we know steam-id of the player, it might be possible to have a player url that matches the steam profile url of the player. For example, my steam profile page is http://steamcommunity.com/id/mcglenn/, so my human-readable yasp url could be http://yasp.co/players/mcglenn/

That way we avoid users having yasp-specific aliases but can still give them more memorable profile URLs.

The steam profile url can probably be found by just everything after the final / in GetPlayerSummaries().profileurl https://developer.valvesoftware.com/wiki/Steam_Web_API#GetPlayerSummaries_.28v0002.29

This has one potential issue: I think Steam allows you to setup this URL by yourself (and change it?), so it would have to be kept up to date if that is indeed the case.

howardchung commented 9 years ago

Requires an API call. https://wiki.teamfortress.com/wiki/WebAPI/ResolveVanityURL

On Thu, Oct 8, 2015 at 4:41 AM, Anders Höckersten notifications@github.com wrote:

Since we know steam-id of the player, it might be possible to have a player url that matches the steam profile url of the player. For example, my steam profile page is http://steamcommunity.com/id/mcglenn/, so my human-readable yasp url could be http://yasp.co/players/mcglenn/

That way we avoid users having yasp-specific aliases but can still give them more memorable profile URLs.

The steam profile url can probably be found by just everything after the final / in GetPlayerSummaries().profileurl https://developer.valvesoftware.com/wiki/Steam_Web_API#GetPlayerSummaries_.28v0002.29

This has one potential issue: I think Steam allows you to setup this URL by yourself (and change it?), so it would have to be kept up to date if that is indeed the case.

— Reply to this email directly or view it on GitHub https://github.com/yasp-dota/yasp/issues/625#issuecomment-146511468.

howardchung commented 9 years ago

I don't know if I want to have to make an API call that scales with visits to YASP. Sure, we could optimize it a few ways:

It means somebody could kind of DoS us by spamming visits to non-numeric player profiles, eating up our rate limit. We'd have to implement some kind of safety check for that.

howardchung commented 9 years ago

Actually it looks like we can do this. @ahockersten is right, we can extract the vanity name from the profileurl property.

We could then store a redis mapping of this vanity name to account_id, and thus resolve it ourselves.

Update would occur whenever a player logs in. So if they changed their vanity URL on steam side, they'd have to relog into YASP to get it to work with their new name.

We should build this on SQL branch, not master.

Sample response:

{
response: {
players: [
{
steamid: "76561198048632981",
communityvisibilitystate: 1,
profilestate: 1,
personaname: "Snifflehopper",
lastlogoff: 1444441482,
commentpermission: 2,
profileurl: "http://steamcommunity.com/id/snifflehopper/",
avatar: "https://steamcdn-a.akamaihd.net/steamcommunity/public/images/avatars/fe/fef49e7fa7e1997310d705b2a6158ff8dc1cdfeb.jpg",
avatarmedium: "https://steamcdn-a.akamaihd.net/steamcommunity/public/images/avatars/fe/fef49e7fa7e1997310d705b2a6158ff8dc1cdfeb_medium.jpg",
avatarfull: "https://steamcdn-a.akamaihd.net/steamcommunity/public/images/avatars/fe/fef49e7fa7e1997310d705b2a6158ff8dc1cdfeb_full.jpg",
personastate: 0
}
]
}
}
howardchung commented 9 years ago

Implemented but not deployed. To use:

howardchung commented 9 years ago

An alternative approach that avoids some of the hacks I'm doing now. . . what if we namespace vanity urls under a separate path?

For example:

http://yasp.co/vanity/snifflehopper would redirect to: http://yasp.co/players/88367253

The word "vanity" could be replaced with something else :P

TrentBartlem commented 9 years ago

http://yasp.co/players/named/snifflehopper ?

howardchung commented 9 years ago

I implemented it as /names/XXX.

I'd prefer not to mess with the players/ namespace.

ahockersten commented 9 years ago

Steam uses https://steamcommunity.com/profiles/76561198253981591 for profile and http://steamcommunity.com/id/mcglenn/ so "id" is a good candidate for vanity urls. I don't particularly like "id" but I think there's a case for being consistent with what steam does.

I don't think "vanity" is a good choice at all. It's only named a vanity url in steam's api, it's presented as a "custom url" to users but maybe I'm biased against vanity due to being a non-native English speaker and that word sounding very uncommon to me.

If you go to any numbered profile url on steam it will redirect to the vanity URL if there is one rather than the other way around. Since the vanity url is generally nicer, maybe we should redirect the other way around instead of what the patch currently does? There is at least one issue with this, if the vanity url changes then old bookmarks etc get broken.

On steam, apparently https://steamcommunity.com/my redirects to the currently logged in user's profile. Might be nice to have.

howardchung commented 9 years ago

Yeah, are you okay with it as /names?

Since the name-based URL is mutable it should definitely be a secondary option. The old account_id is canonical and will always work.

yasp.co itself will redirect to profile if the user is logged in, so I don't think /my is necessary.

ahockersten commented 9 years ago

@howardchung are you asking me about /names or @TrentBartlem? I'm ok with it.

howardchung commented 9 years ago

Anyone who cares about the feature, really :P I personally probably won't ever use it since I just go to yasp.co on my own laptop all the time.

ahockersten commented 9 years ago

I didn't think about this before, but how are users who want it supposed to discover that this feature exists? Obviously anyone reading the blog post mentioning it or this issue will know about it, but what about users coming along say, 6 months from now?

One way to solve it could be to add it to the faq, although I don't particularly like the idea.

howardchung commented 9 years ago

Usually, we'd just blog about it or announce it on the banner. I'm not sure how many people want this, but it was pretty easy to do so I added it

TrentBartlem commented 9 years ago

On the player page, there's the Steam player icon (which links to their steam profile), then their vanity name next to it that is not a link. Perhaps make that a link to the vanity URL in Yasp?

Alternatively, make the vanity url small, and on the next line?

snifflehopper

howardchung commented 9 years ago

I am not sure I want to do that because it implies that the vanity URL is "primary", while really it's just a redirect to the id-based profile. The vanity URL could break at any time if the player changes their Steam vanity URL.