Skip to content

Fix shard price in tooltips#2518

Closed
LJNeon wants to merge 3 commits into
SkyblockerMod:mainfrom
LJNeon:shard-price-fix
Closed

Fix shard price in tooltips#2518
LJNeon wants to merge 3 commits into
SkyblockerMod:mainfrom
LJNeon:shard-price-fix

Conversation

@LJNeon

@LJNeon LJNeon commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

fixes #2474

Tests

Option set to Buy Order image image image
Option set to Buy Instantly image image image

@LifeIsAParadox LifeIsAParadox added the reviews needed This PR needs reviews label Jul 7, 2026

@Alex33856 Alex33856 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be its own config option, right?

@LJNeon

LJNeon commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

To me that feels like unnecessary bloat, maybe renaming the option to a more generic name might be better?

@Alex33856

Copy link
Copy Markdown
Contributor

maybe renaming the option to a more generic name might be better?

yeah that's probably fine.

@LJNeon
LJNeon force-pushed the shard-price-fix branch 2 times, most recently from 532ec51 to 6f823da Compare July 11, 2026 18:20
@LJNeon
LJNeon requested a review from Alex33856 July 13, 2026 21:34
@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed reviews needed This PR needs reviews labels Jul 14, 2026
@AzureAaron AzureAaron modified the milestone: 6.8.0 Jul 15, 2026
@LJNeon
LJNeon force-pushed the shard-price-fix branch from 6f823da to 373ebc0 Compare July 17, 2026 04:08
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed merge me please Pull requests that are ready to merge labels Jul 17, 2026
@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed reviews needed This PR needs reviews labels Jul 17, 2026
"skyblocker.config.general.itemTooltip.avg.THREE_DAY": "3-day price",
"skyblocker.config.general.itemTooltip.craft": "Crafting Cost",
"skyblocker.config.general.itemTooltip.craft.@Tooltip": "You can choose which Bazaar order type to use in crafting calculation",
"skyblocker.config.general.itemTooltip.craft": "Calculated Costs",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought I don't like this. This rename obscures the tooltip option into something wildly different than it actually is (which is that it shows the raw crafting cost).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you rather I split crafting cost in tooltip into one option and whether calculations use buy order or buy instantly prices into a second option? I can do that if you want.

@LifeIsAParadox LifeIsAParadox added changes requested This PR need changes and removed merge me please Pull requests that are ready to merge labels Jul 17, 2026
@LJNeon LJNeon closed this Jul 19, 2026
@LifeIsAParadox LifeIsAParadox removed the changes requested This PR need changes label Jul 19, 2026
@LJNeon

LJNeon commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Decided it was better left unchanged.

@LJNeon
LJNeon deleted the shard-price-fix branch July 19, 2026 02:32
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 this pull request may close these issues.

Make shard sell price tooltip affected by tooltip crafting cost config option

4 participants