search

w35l3y / userscripts

Public userscripts to be used with Greasemonkey
http://gm.wesley.eti.br
GNU General Public License v3.0
43 stars 21 forks source link

Include: Stock Market #37

Closed jacobkossman closed 8 years ago

jacobkossman commented 8 years ago

Added the preliminary functionality of a stock market include since I see you didn't start one. Haven't gotten to buy / sell, you may have more expertise in regards to these in terms of needing referrers etc.

w35l3y commented 8 years ago

I will check it as soon as I get home. Thanks @jacobkossman

jacobkossman commented 8 years ago

@w35l3y how do i not add my new commits to this pull request lol.

w35l3y commented 8 years ago

This is a very good question. Did you pull request again? Don't worry. I will check the whole code.

jacobkossman commented 8 years ago

nope. no new pull requests, just commit and pushed to my fork.

i've been updating the food club include with a bunch of new functions and editing the training school one because you couldn't choose a pet (as it would default to active). feel free to take or not take whatever you choose!

w35l3y commented 8 years ago

It will consider the active pet unless you specify anything different as 2nd parameter :) var TrainingSchool = function (page, pet) {

jacobkossman commented 8 years ago

yeah but that means you have to make an instance for each pet, no? is that a "better" option?

w35l3y commented 8 years ago

Yes, that's right. I think it's better to create a instance for each pet then passing the same parameter every time. What do you think? You have very good questions.

jacobkossman commented 8 years ago

i totally didn't think about making multiple instances. i like the idea of using the parameter because you can then more easily loop to finish / start all courses regardless of the pet / name of the instance. aka grab the pets that are on a course from the DOM and pass that to the function in a loop.

also based on my changes it's still an optional parameter, you can leave it empty and it'll default to the active petname.

w35l3y commented 8 years ago

You are thinking most likely as having a singleton. This is one of the problems with my old libs, they couldn't keep custom states. It's very difficult to maintain custom states for each case when using singleton (single instance to everything)

jacobkossman commented 8 years ago

Fair enough. I don't have any problem making an instance for each pet just didn't realize that was the way to do it. Feel free to disregard those commits and i'll update my scripts accordingly. I'm about to push a inventory include as well, just fyi :)