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.

Sign up for GitHub

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

Jump to bottom

Get event results #165

Merged
merged 4 commits into from May 3, 2019
Merged

Get event results #165

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9ca53a3
+ Filtering by eventId for the getResults method
Chuli2k Apr 29, 2019
6f09603
Reset pages param to 1 if filtering by events
Chuli2k Apr 29, 2019
3b28e9e
Removed variable for eventName
Chuli2k May 3, 2019
be0cb7b
Removed default value from eventId parameter
Chuli2k May 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 31 additions & 5 deletions src/endpoints/getResults.ts
View file
Open in desktop
Expand Up @@ -6,19 +6,35 @@ import { popSlashSource } from '../utils/parsing'
import { HLTVConfig } from '../config'
import { fetchPage, toArray, getMatchFormatAndMap } from '../utils/mappers'

export const getResults = (config: HLTVConfig) => async ({ pages = 1 } = {}): Promise<
export const getResults = (config: HLTVConfig) => async ({ pages = 1, eventId = 0 } = {}): Promise<
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the default to 0 needed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's not... I'll remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean I should set it to undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cheated and looked at how emha did it for teamId in his PR and made the same changes :)
I'm still new to typescript and didn't know how to remove the default value...

MatchResult[]
> => {
if (pages < 1) {
console.error('getLatestResults: pages cannot be less than 1')
return []
}

// All results for events are always on one page
if(eventId) {
pages = 1
}

let matches = [] as MatchResult[]

for (let i = 0; i < pages; i++) {
const $ = await fetchPage(`${config.hltvUrl}/results?offset=${i * 100}`, config.loadPage)
let fullUrl = `${config.hltvUrl}/results?offset=${i * 100}`
let eventName: string

if(eventId) {
fullUrl += `&event=${eventId}`
}

const $ = await fetchPage(fullUrl, config.loadPage)

if(eventId) {
eventName = $('.eventname').text()
}

matches = matches.concat(
toArray($('.results-holder > .results-all > .results-sublist .result-con .a-reset')).map(
matchEl => {
Expand Down Expand Up @@ -48,11 +64,21 @@ export const getResults = (config: HLTVConfig) => async ({ pages = 1 } = {}): Pr
}

const event: Event = {
name: matchEl.find('.event-logo').attr('alt'),
id: Number(popSlashSource(matchEl.find('.event-logo'))!.split('.')[0])
name: eventId ? eventName : matchEl.find('.event-logo').attr('alt'),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just put the implementation of eventName here? Instead of:

let eventName: string

if(eventId) {
  eventName = $('.eventname').text()
}

name: eventId ? eventName : matchEl.find('.event-logo').attr('alt')

just

name: eventId ? $('.eventname').text() : matchEl.find('.event-logo').attr('alt')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want it inside the for loop. This way if you filter by eventid it is resolved only once.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so I was on my phone and missed, that I had it insode the for loop. I wanted to set it outside, but for the events it will only get one page, so setting it outside the loop is irrelevant :P
I did it as you suggested.

id: eventId
? eventId
: Number(popSlashSource(matchEl.find('.event-logo'))!.split('.')[0])
}

const date = Number(matchEl.parent().attr('data-zonedgrouping-entry-unix'))
const date = Number(
eventId
? matchEl
.find('.date-cell')
.children()
.first()
.attr('data-unix')
: matchEl.parent().attr('data-zonedgrouping-entry-unix')
)

return { id, team1, team2, result, event, map, format, stars, date }
}
Expand Down
10 changes: 7 additions & 3 deletions src/playground.ts
View file
Open in desktop
@@ -1,7 +1,7 @@
import HLTV from './index'
HLTV.getMatch({ id: 2332676 })
.then(res => console.dir(res, { depth: null }))
.catch(err => console.log(err))
// HLTV.getMatch({ id: 2332676 })
// .then(res => console.dir(res, { depth: null }))
// .catch(err => console.log(err))
// HLTV.getMatches().then(res => console.log(res))
// HLTV.getResults({pages: 1}).then(res => console.log(res))
// HLTV.getStreams({ loadLinks: true }).then(res => console.log(res))
Expand All @@ -28,3 +28,7 @@ HLTV.getMatch({ id: 2332676 })
// HLTV.getTeamStats({id: 6669}).then(res => console.dir(res, { depth: null })).catch(err => console.log(err))
// HLTV.getPlayer({id: 9216}).then(res => console.dir(res, { depth: null })).catch(err => console.log(err))
// HLTV.getEvent({id: 3773}).then(res => console.dir(res, { depth: null })).catch(err => console.log(err))


HLTV.getResults({pages: 1, eventId: 3883}).then(res => console.log(res))
HLTV.getResults({pages: 1, eventId: 3047}).then(res => console.log(3047 + ':' + res.length))