GeekyDeaks / discord-destinybot

Discord Destiny Bot
MIT License
8 stars 3 forks source link

Add Command to Pull Daily and Weekly Activities from Destiny #16

Closed unisys12 closed 7 years ago

unisys12 commented 7 years ago

Create a cmd that the user can use to pull in daily and weekly Destiny activities, such as Daily bounties, Daily story missions, Daily crucible bounties and playlists.

GeekyDeaks commented 7 years ago

Just had a look under the debugger - try changing the loop in advisors.js as follows:

    var i;
    for (i in activities) {
        console.log(i);
    }

The let was failing on my version of node for some reason and the .display key is actually under each key in advisor.data.activities

unisys12 commented 7 years ago

Ok. If I have time during lunch, I will play with it. If not, when I get off work then. Thanks.

On Wed, Aug 24, 2016, 9:25 AM Scott Deakin notifications@github.com wrote:

Just had a look under the debugger - try changing the loop in advisors.js as follows:

var i;
for (i in activities) {
    console.log(i);
}

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/GeekyDeaks/discord-destinybot/issues/16#issuecomment-242083629, or mute the thread https://github.com/notifications/unsubscribe-auth/AB_vg_ZEZKM3YMFz8BK9n3FlJAYROTFiks5qjFRqgaJpZM4Jr_LW .

unisys12 commented 7 years ago

Alright, had a some time to work on this at work and here's where I'm at.

To access the other properties of the advisors object, you have to explicitly call them. Ex: Response.data.activities.srl.status.active. This example has a value of false.

Now, on to the next frustrating part...

If you use the iteration I used this morning, you would think that you should be able to do the following:

// I captured an example API response to a JSON file
var res = require('./activities.json');
var activities = res.Response.data.activities;

for (let activity in activities) {
    /** activity should contain the property title of each activity property
      *  held in the API response object. 
      *  prisonofelders, elderchallenge, trials, armsday, weeklycrucible, etc...
      */
    console.log(activities + '.' + activity + '.' + status.active);
}

But this merely returns the following error:

c:\sites\objects\main.js:33
        console.log(activities + '.' + activity + '.' + status.active);
                                                        ^

ReferenceError: status is not defined

I am sure that I am doing something stupid and elementary wrong, but like you... still pretty new to JavaScript and this is only maybe my third all out JavaScript project. This is one of those moments when you say to yourself, "Self! This would be so much easier in the language I am used to!" But sticking it out and working through this is how we learn I guess. LOL. I will get it.

GeekyDeaks commented 7 years ago

From reading the structure above, I think you may have wanted something like this:

    console.log('activities.' + activity + '.status.active: ' + activity.status.active);
unisys12 commented 7 years ago

Ok! I see what's going on... Not all the properties have the status.active that I was testing for. Weird. Now I know why there the following comment in the API docs: V2 features contracts that are 40% less shitty.

The following still fails though...

'use strict'

var res = require('./response.json');
var activities = res.Response.data.activities;

/**
 * Returns Prison of Elders
 */
console.log(activities.prisonofelders.display.advisorTypeCategory);

for (let activity in activities) {

    console.log(activities.activity.display.advisorTypeCategory);

}

Error:

$ node index.js
Prison of Elders
C:\sites\objects\index.js:12
    console.log(activities.activity.display.advisorTypeCategory);
                                   ^

TypeError: Cannot read property 'display' of undefined

Which basically means that activity is undefined or NULL. But, if you console.log(activity) inside the for...in loop, it outputs:

$ node index.js
prisonofelders
elderchallenge
trials
armsday
weeklycrucible
kingsfall
vaultofglass
crota
nightfall
heroicstrike
dailychapter
dailycrucible
prisonofelders-playlist
ironbanner
xur
srl

So, it works. At this point I think you can see my confusion as to why the var activity created in the for...in loop returns something accessing it alone, but not when using it to access properties inside the object. Just weird.

unisys12 commented 7 years ago

Playing with this before work, I tried the following:

'use strict'

var res = require('./response.json');
var activities = res.Response.data.activities;
var list = [];

//console.log(activities.prisonofelders.display.advisorTypeCategory);

for (let activity in activities) {

    list.push(activity);

}

for (var i = 0; i < list.length; i++) {
    //console.log(list[i]); spits my list back out, as before
    console.log(activities.list[i].display.advisorTypeCategory);
}

And it fails with:

C:\sites\objects\index.js:16
    console.log(activities.list[i].display.advisorTypeCategory);
                               ^

TypeError: Cannot read property '0' of undefined

At this point, I am going to separate this out into independent methods based on user input. So! The user will enter /d advisor PoE or /d advisor trials or /d advisor daily crucible || dc, like that. This way, I can make it work. There is a guy in the FFC community that has made what he refers to as a Warmind. It is a email newsletter that pulls the weekly reset info and emails it to you in the form of what looks like a Warmind terminal, using Python. I have talked with him a few times and I know he had issues with this new V2 of the advisor api. I will touch base with him and see if he can give some insight. Until then, I will move forward as I described above.

GeekyDeaks commented 7 years ago

hmm... in the above code you are pushing onto list and checking it's length in the for loop, but you are then accessing activities.list - did you mean to just use list[i].display.advisorTypeCategory?

unisys12 commented 7 years ago

Sorry for the delayed response. Internet is down again, but that's not stopping me from working on this.

Yes, that's what I meant to write, since activities contains the first part of the object structure. It's sorta mute now though. As I mentioned earlier, I am going to split this up into separate cmds. No biggie.

unisys12 commented 7 years ago

Ok... got it sorted this morning and found the error of my ways. The variable that I want to place with the string of a property has to use "bracket notation" and not "dot notation".

So, instead of this:

for (let activity in activities) {

    console.log(activities.activity.display.advisorTypeCategory);

}

I should've been doing this:

for (let activity in activities) {

    console.log(activities[activity].display.advisorTypeCategory);

}

Anyway, I will never forget this lesson! EVER!

Moving forward. Now that I have proper internet again, I will perform a pull (to bring in any updates and changes to the project) . Shouldn't affect me since most everything I am working with is being created from scratch, aside from the DestinyAPI and modules/destiny/manifest.js.

Still going looking at breaking out each activity into it's own separate response. Some will be easier to put together than others, but in the end, These can be really long and the 2000 character limit in Discord might cause multiple DM's being sent. That would get messy to say the least.

GeekyDeaks commented 7 years ago

Looking good! Regarding the 2000 character limit, take a look in bot/message.js. There are two functions exposed:

send(msg, content, isPublic, expire)
update(prevMsg, content, expire)

If you look at some of the other commands that I have updated to use the above, you will see how they are used. In brief:

unisys12 commented 7 years ago

Trials and Daily Chapter are done. Was going to submit a pull request, for nothing else but to document progress, but apparently something has gone out of sync between the Master branch and my Destiny_Advisor branch. Will investigate that tonight before moving forward. Not worth the hassle at this time. Simple enough for me to delete my branch and start a new one if need be.

GeekyDeaks commented 7 years ago

happy to figure out the delta's if you want to submit a pull request

GeekyDeaks commented 7 years ago

just merged the latest advisor code into master as it seems stable

unisys12 commented 7 years ago

Should've done this long ago... 💩