DestinyItemManager / DIM

Destiny Item Manager
https://destinyitemmanager.com
MIT License
2.07k stars 641 forks source link

Decouple Vendors from character loading. #939

Closed SunburnedGoose closed 8 years ago

SunburnedGoose commented 8 years ago

During the ROI maintenance window, the API was available for items, but DIM couldn't load due to Vendors being in maintenance still.

We need to decouple vendors and make that feature one that has it's own loading mechanism to avoid this blocking.

screen shot 2016-09-19 at 1 44 42 pm

bhollis commented 8 years ago

Linking #665.

bhollis commented 8 years ago

I just noticed that we now loop over every entry in the DestinyVendorsDefinitions table and make a request for each vendor, for each character. With the latest manifest, this is 77 vendors 𝗑 3 characters = 231 separate requests - all for data that isn't even shown on the main page!

Most of these vendors aren't included in our UI anywhere, either. Here's the full list: "Rise of Iron Book Rewards", "Titan Vanguard", "Vehicles", "Guardian Outfitter", "Tribute for Crota's Bane", "Taken Sterling Treasure", "Eris Morn", "Iron Banner", "Vanguard Package", "Queen's Blessing", "Radiant Treasure", "", "Shipwright", "New Monarchy Package", "Gunsmith", "Emote Collection", "Rise of Iron Book Rewards", "Dead Orbit Weapons", "Crucible Package", "Vault", "Spring 2016 Emotes", "Rise of Iron Book Rewards", "Crucible Package", "Rise of Iron Book Rewards", "Cryptarch", "Sparrow Toolkit", "Vanguard Package", "Queen's Wrath", "Exotic Weapon Blueprints", "Bounty Tracker", "Warlock Vanguard", "Fall 2015 Emote", "Future War Cult Package", "Rise of Iron Book Rewards", "House of Judgment Package", "Dead Orbit Package", "New Monarchy", "Future War Cult", "Disciple of Osiris", "Crucible Package", "House of Judgment", "Winter 2015 Emote", "Postmaster", "New Monarchy Package", "Vanguard Scout", "Cryptarch", "Ship Collection", "Dead Orbit Chroma", "Dead Orbit Armor", "Shader Collection", "Crucible Package", "Future War Cult Package", "Iron Banner", "Iron Lord", "Vanguard Quartermaster", "The Speaker", "Custom Horn Kit", "Future War Cult Package", "Postmaster", "New Monarchy Package", "Agent of the Nine", "Hunter Vanguard", "Cryptarch Package", "Future War Cult Package", "Vanguard Package", "Vanguard Package", "Emblem Collection", "New Monarchy Package", "Dead Orbit", "Crucible Quartermaster", "Crucible Handler", "Tower Map", "Exotic Armor Blueprints", "Eververse", "Rise of Iron Book Rewards", "Cryptarch", "Sterling Treasure".

/cc @ericnelson0

kyleshay commented 8 years ago

What do we want to do -- call the vendor endpoints only if they click the vendor tab? The t12 loadout builder also needs vendor info.

maybe when the app loads the first time and they don't have it cached, we can give them an option to load them? still doesn't get around making the 231 requests.

maybe we could organize those requests... some of them we don't need for the loadout builder. some we don't need until we click on that specific tab in the vendors view.

bhollis commented 8 years ago

And many of them we don't need at all - we don't display them anywhere.

I'm also not sure we need to show the emblems, shaders, ships, etc. I think those should go in a separate section that shows what parts of collections you've unlocked. Putting them in with vendors just shows which ones you have right now.

On Thursday, September 22, 2016, kyle shay notifications@github.com wrote:

What do we want to do -- call the vendor endpoints only if they click the vendor tab? The t12 loadout builder also needs vendor info.

maybe when the app loads the first time and they don't have it cached, we can give them an option to load them? still doesn't get around making the 231 requests.

maybe we could organize those requests... some of them we don't need for the loadout builder. some we don't need until we click on that specific tab in the vendors view.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/DestinyItemManager/DIM/issues/939#issuecomment-248923345, or mute the thread https://github.com/notifications/unsubscribe-auth/AATHePM00udmscl79XsHom7AEwNct2xUks5qspM9gaJpZM4KA1GF .

SunburnedGoose commented 8 years ago

If the data is stable and has a known expiration, why don't we save the payload in localstorage and grab our cached version after we've downloaded it. This in addition to the rest of the suggestions of course.

screen shot 2016-09-22 at 12 09 40 pm

ericnelson0 commented 8 years ago

I'll be back after the 30th and I should be able to work on this again, but if someone wants to work on this before then by all means.

bhollis commented 8 years ago

Just a heads up, I'm pretty far along with this, and it's going great. My ETA for having it done is Tuesday October 18th.