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

[Loot] Dynamically set NPC IDs from LR response #243

Open
Kwack-Kwack opened this issue Mar 25, 2024 · 5 comments
Open

[Loot] Dynamically set NPC IDs from LR response #243

Kwack-Kwack opened this issue Mar 25, 2024 · 5 comments

Comments

@Kwack-Kwack
Copy link
Collaborator

Consider all keys in the npcs object (from the response of https://api.lzpt.io/loot) as active NPCs and disregard others (LZPT will handle seasonal NPCs for us)
Example response from LZPT

@Lazerpent
Copy link

Rather than using the keys of the npcs, it would be best to rely on the order array. This is more just standardization - they will always match.

@Manuito83
Copy link
Owner

I think we would need to completely disassociate what comes from LR and Firebase (while still allowing PDA to solely rely on Firebase if the user chooses to deactivate LR or LR is down), as they seem to be a bit mixed up at the moment. For example, in here we still use the information that comes from Firebase (_mainLootInfo) and compare it to LR before we build the final list.

And then here we add NPCs that might be missing in LootRangers. I can't remember if I added this because something happened... or just in case?

We might want to decide very early if we are going to use one or the other, and just rely on that information. I think LR has all the information we need to build the rest of the page (e.g.: hospital times so that we can build the lines for each level)...?

Rather than using the keys of the npcs, it would be best to rely on the order array. This is more just standardization - they will always match.

I think we are already doing this here:

imagen

@Kwack-Kwack
Copy link
Collaborator Author

Kwack-Kwack commented Mar 25, 2024

This (edit: using the order array) wouldn't make much sense to me (unless I'm missing something)?

Primarily during the seasonal NPCs (where there is desync between attack times) but also just when Leslie isn't planned for an attack, I would've thought it made more sense to show all currently active NPCs than just the ones specified in the next attacks.

Both times there are people who may want to [a] see the info of the unplanned NPC (to plan ahead), or [b] set a timer for a specific NPC's level; neither of which can be done without the card there.

I'm on my phone currently so I can't check, but from memory it currently sorts the card depending on that order array (and adds any extras on the end), which makes sense to me. I wouldn't be against adding some indication that an NPC isn't planned for the next clear, similar to how you do in the Discord.

Let me know if I'm missing something @Lazerpent

@Lazerpent
Copy link

The order array always includes all NPCs, regardless of if they are "next" or not. It's just the key set of npcs except in order. They should always be the same, but just for consistency it's best to use order since that's the one I guarantee will have the correct listing.

My post condition is basically "all active NPCs are listed in order, and all items in order exist in npcs"

@Lazerpent
Copy link

Lazerpent commented Mar 25, 2024

while still allowing PDA to solely rely on Firebase if the user chooses to deactivate LR or LR is down

If LR is down, it won't work at all right? Even if firebase reported correct values there is no loot timers to pull. Is disabling LR even an option right now?

I think we are already doing this here:

Yes you are for displaying the order but not for which NPCs to show. Until firebase was updated today, I still had Scrooge listed on my page. That said, I think the solution you touched at is better than the "hacky" "replace firebase with the order array" I had thought up

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

No branches or pull requests

3 participants