added filter to just sell to npc's #21

Merged

MrBiscuit921 merged 3 commits from master into master 2024-10-29 08:45:10 +00:00

Conversation 3 Commits 3 Files changed 2 +28 -7

MrBiscuit921 commented 2024-10-28 06:43:18 +00:00 (Migrated from github.com)

Copy link

No description provided.

ianrenton commented 2024-10-28 10:19:25 +00:00 (Migrated from github.com)

Copy link

Thanks for another contribution!

Unfortunately I think this one might have a bit of a logic bug, and could do with a couple of tweaks to neaten it up.

• At the moment, the NPC sales only appear if the NPC sale gives you more money than the Bazaar sale (~line 275). This is still true even with your new option selected. So if you can sell to an NPC, and make a profit that way, but you could have got more money from the Bazaar, the item just doesn't appear. Pushing to calcData at ~line 279 also means the other checks, such as for unprofitable/unaffordable/manipulated items, are skipped. I'm not sure if this is what you intended? If not, I think a better way would be to remove the calcData.push() at ~line 279, then further down where you have if (npcDataOnly == false) instead be if (npcDataOnly == false || npcSellPrices.has(id)). That will still ensure that only NPC-sellable items are included when npcDataOnly is true, but all the other checks still get applied as well, and you'll see items that could have been sold at the Bazaar for more.
• You can get an empty table by selecting "Show only items sold to NPCs" and deselecting "Indicate where items can be sold to NPCs". I think if "Show only items sold to NPCs" is selected it makes sense to auto-select the other checkbox, and disable it so it can't be turned off while "Show only items sold to NPCs" is true.
• As a minor thing I think the new option should move up above "Remove manipulated" in the HTML, to group it better with the other NPC-sale-related option.

Thanks for another contribution! Unfortunately I think this one might have a bit of a logic bug, and could do with a couple of tweaks to neaten it up. - At the moment, the NPC sales only appear if the NPC sale gives you more money than the Bazaar sale (~line 275). This is still true even with your new option selected. So if you *can* sell to an NPC, and make a profit that way, but you could have got more money from the Bazaar, the item just doesn't appear. Pushing to `calcData` at ~line 279 also means the other checks, such as for unprofitable/unaffordable/manipulated items, are skipped. I'm not sure if this is what you intended? If not, I think a better way would be to remove the `calcData.push()` at ~line 279, then further down where you have `if (npcDataOnly == false)` instead be `if (npcDataOnly == false || npcSellPrices.has(id))`. That will still ensure that only NPC-sellable items are included when `npcDataOnly` is true, but all the other checks still get applied as well, and you'll see items that could have been sold at the Bazaar for more. - You can get an empty table by selecting "Show only items sold to NPCs" and deselecting "Indicate where items can be sold to NPCs". I think if "Show only items sold to NPCs" is selected it makes sense to auto-select the other checkbox, and disable it so it can't be turned off while "Show only items sold to NPCs" is true. - As a minor thing I think the new option should move up above "Remove manipulated" in the HTML, to group it better with the other NPC-sale-related option.

MrBiscuit921 commented 2024-10-29 03:30:16 +00:00 (Migrated from github.com)

Copy link

i think its fixed now (hopefully) thanks for finding those errors
one thing i changed though was if (npcDataOnly == false || npcSellPrices.has(id)) will show items that are sellable to npcs but are better to sell to bazaar however i made it if (npcOnlyFilter == false || item.sellPrice == npcSellPrices.get(id)) that way it will only show items that are better to sell to npcs than bazaar

i think its fixed now (hopefully) thanks for finding those errors one thing i changed though was `if (npcDataOnly == false || npcSellPrices.has(id))` will show items that are sellable to npcs but are better to sell to bazaar however i made it `if (npcOnlyFilter == false || item.sellPrice == npcSellPrices.get(id))` that way it will only show items that are better to sell to npcs than bazaar

ianrenton commented 2024-10-29 08:44:20 +00:00 (Migrated from github.com)

Copy link

Looks good, thanks again!

Looks good, thanks again!

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

ian/Skyblock-Bazaar-Flipping-Calculator!21

No description provided.