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

Adding features to the "matchups" endpoint #2570

Open
booshack opened this issue Jul 1, 2022 · 6 comments
Open

Adding features to the "matchups" endpoint #2570

booshack opened this issue Jul 1, 2022 · 6 comments

Comments

@booshack
Copy link

booshack commented Jul 1, 2022

Hey there! Thought I would try my hand at adding some functionality to "matchups" today - specifically I planned on adding responses "games_played_with" and "wins_with" (the pre-existing ones becoming "_against"). But I've just seen that there was some attempt made to implement this earlier in the code, which has now been commented (endpoint with description: "Get hero matchups (teammates and opponents)").

Is there a reason this was abandoned that would cause it to be rejected in the future? As in...should I bother? These will probably be quite substantial projects for me as I am very green.

Also hoped to add parameters "time_period", "position"* (for both heroes), "same_lane"**. Same question about these? Would you consider implementing them?

*Accepts an integer 1-5 that represents the hero's rank among its teammates with respect to lasthits
**Accepts True/False based on whether or not to look at matches where the two heroes laned in the same lane.

@howardchung
Copy link
Member

Can you post a pointer to that code? Maybe there were performance issues or the feature was no longer needed

@booshack
Copy link
Author

booshack commented Jul 2, 2022

Line 3380 in /routes/spec.js.

Will revisit this in a little while btw!

@howardchung
Copy link
Member

howardchung commented Jul 5, 2022

f0a0245

Looks like I removed it six years ago but I don't remember why--I think it added a lot of Redis memory usage since it creates many many keys due to the number of possible hero combinations

@howardchung
Copy link
Member

It is possibly more feasible if we keep it pairs of heroes only and don't consider triads+

@booshack
Copy link
Author

booshack commented Jul 5, 2022

Oh I see I see thanks, yeah duos is enough I think.

Have also just learnt that the current matchups endpoint (spec.js line 3679) only looks at pro matches - would an attempt to make it consider all matches (and then filter with parameter time_period) be rejected?

Also, on the point that I'm a newbie, I don't understand what in the code makes it only look at pro matches...it seems to manipulate data from tables matches and player_matches, which include all matches no? Still making sense of it all to be honest haha

@howardchung
Copy link
Member

the SQL tables matches/player_matches only include pro matches due to capacity issues.

Pub matches are stored in Cassandra which has much more limited querying abilities.

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

2 participants