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

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

Closed
unisys12 opened this issue Aug 24, 2016 · 14 comments
Closed

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

unisys12 opened this issue Aug 24, 2016 · 14 comments
Assignees

Comments

@unisys12
Copy link
Collaborator

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.

@unisys12 unisys12 self-assigned this Aug 24, 2016
@GeekyDeaks
Copy link
Owner

GeekyDeaks commented Aug 24, 2016

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
Copy link
Collaborator Author

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
#16 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB_vg_ZEZKM3YMFz8BK9n3FlJAYROTFiks5qjFRqgaJpZM4Jr_LW
.

@unisys12
Copy link
Collaborator Author

unisys12 commented Aug 24, 2016

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
Copy link
Owner

GeekyDeaks commented Aug 24, 2016

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

    console.log('activities.' + activity + '.status.active: ' + activity.status.active);

@unisys12
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Owner

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Owner

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:

  • msg - referrers to any message you are responding to and is used to either get the channel (public) or author (private)
  • content - easy one, but there is a catch - if this is an array, then it will build up the message by joining the array with \n until it reaches the limit of 1930 (configurable in config.discord.maxMessageSize), making sure the message consists of whole array elements. The process is then repeated for any remaining array elements until all have been processed. The idea is you build up what you want to send in discrete chunks, putting each chunk into an array.
  • isPublic - flag to indicate if the response should be on a server channel (public) or PM. If the original command was in a PM, then this is effectively ignored. This only applies to send() as update() will simply update the already existing message so it does not have to work out if it needs to be on a server channel or PM.
  • expire - if this is defined and the message is sent on a public channel, then it will automatically be deleted after expire ms. This is useful to prevent error messages cluttering the channel.

@unisys12
Copy link
Collaborator Author

unisys12 commented Sep 1, 2016

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
Copy link
Owner

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

@unisys12 unisys12 mentioned this issue Sep 2, 2016
@GeekyDeaks
Copy link
Owner

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

@unisys12
Copy link
Collaborator Author

Should've done this long ago... 💩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants