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

Feature Request - Specify the epic rapport gift being sold #36

Closed
AlantaRian opened this issue Apr 16, 2022 · 8 comments · Fixed by #47
Closed

Feature Request - Specify the epic rapport gift being sold #36

AlantaRian opened this issue Apr 16, 2022 · 8 comments · Fixed by #47

Comments

@AlantaRian
Copy link

In terms of silver costs, it's cheaper to use the 450 rapport gifts than legendary gifts (the same amount of silver gets you 80% more rapport). Thus it would be nice to have an ability to track specific epic gifts and get alerts about them.

@Lunariz
Copy link

Lunariz commented Apr 17, 2022

+1 for this, exactly what I'm missing. I've finished 2 of the 3 rapport NPCs in Tortoyk, so right now only 1 in 3 visits actually gives me the item I want. Would save me a lot of time to know which item is there. Same thing for a lot of other continents.

@Sedro01
Copy link
Contributor

Sedro01 commented Apr 20, 2022

@Xeio would you accept a PR for this?

@Xeio
Copy link
Owner

Xeio commented Apr 21, 2022

Yeah, honestly biggest headache is mostly the data entry in https://github.com/Xeio/WanderLost/blob/main/WanderLost/WanderLost/Client/wwwroot/data/merchants.json (Should be structured similar to cards, with a rarity + name), even if that's all you want to provide. Technically if you want to go further you could even add an "AdditionalText" field or something to use as a tooltip for which NPCs the Epic rapports are for though I'd need to make some small tweaks to the UI code to display it.

As far as code I could do most of that if you don't want to/can't. It's basically the merchant validation, grid display, update merchant page, and the notification settings page. I'll probably try to put in a compatibility shim for the notifications settings too so it defaults anyone that had "Legendary" to select all the legendary items.

Also it's probably a client-breaking change, but I have the ability to force client page reloads so hopefully that part at least will go smoothly...

@Sedro01
Copy link
Contributor

Sedro01 commented Apr 21, 2022

I'll probably try to put in a compatibility shim for the notifications settings too so it defaults anyone that had "Legendary" to select all the legendary items.

Maybe I'll add notification support for individual rapport items in a separate PR

@Xeio
Copy link
Owner

Xeio commented Apr 21, 2022

Thanks for the PR, didn't mean to close this issue immediately...

I did add the notification logic, and it should migrate the old settings to the new as well. Will deploy it probably some time tonight.

@Xeio Xeio reopened this Apr 21, 2022
@Xeio
Copy link
Owner

Xeio commented Apr 21, 2022

Now live, client should automatically refresh to latest.

@Xeio Xeio closed this as completed Apr 21, 2022
@Cassock
Copy link

Cassock commented Apr 22, 2022

Excuse the necro and tell me if this should be a new issue or not, but appending the NPC that this epic item gives 450 rep for would be spectacular (so I don't have to memorize which gift goes to who). Maybe an icon or NPC name to the right of the item name, or icon/picture/name on mouseover?

@Xeio
Copy link
Owner

Xeio commented Apr 22, 2022

You would probably want to open a separate issue to track. It's possible, mostly the work of entering all the data (displaying it would be fairly simple).

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

Successfully merging a pull request may close this issue.

5 participants