Skip to content

Added Skyblock Functions for Profile and Auction House - #15

Open
fivepoint-0 wants to merge 2 commits into
Kanin:masterfrom
fivepoint-0:master
Open

fivepoint-0 wants to merge 2 commits into
Kanin:masterfrom
fivepoint-0:master

Conversation

@fivepoint-0

Copy link
Copy Markdown

I will be looking at this more. I think you got something pretty cool here! I want to see if I can help build the library if possible. Cheers! I didn't add definitions/docs to the code but I'll do it soon.

@destruc7i0n

Copy link
Copy Markdown
Collaborator

I haven’t worked with the API since around 2016, so I can’t really comment too much on the functions themselves. A few thoughts though:

Why does setMainSkyblockProfile exist? I don’t see it being used in the lib anywhere.
I notice that getSkyblockAuctions uses pages, can the user specify which page they want to go to in the parameters?
Also, it would be nice to have the inline docs as well.

@destruc7i0n
destruc7i0n removed the request for review from yrsegal February 24, 2020 21:45
@fivepoint-0

fivepoint-0 commented Feb 26, 2020

Copy link
Copy Markdown
Author

I haven’t worked with the API since around 2016, so I can’t really comment too much on the functions themselves. A few thoughts though:

Why does setMainSkyblockProfile exist? I don’t see it being used in the lib anywhere.
I notice that getSkyblockAuctions uses pages, can the user specify which page they want to go to in the parameters?
Also, it would be nice to have the inline docs as well.

setMainSkyblockProfile was just a function I implemented because grabbing your Skyblock profile returned more than one result. I will look at the docs, refactor the code, add internal docs, and resubmit. Thanks!

Also, they cannot choose the page yet but I will parameter-ize that!

@Kanin
Kanin removed the request for review from destruc7i0n September 8, 2020 01:39
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

Successfully merging this pull request may close these issues.

2 participants