Fix: Item Value Estimation Coloring #3061
Conversation
|
in the tooltip i dont think the "base item" should reflect recomb |
|
either add reforge name to the item name, then we can simply use the real item name again, or dont show recomb. |
|
unrelated: the data in the repo pr is used to move existing logic outside of live code. this is fine. but this has noting to do with this pr's title or description |
There was a problem hiding this comment.
Im not very happy with this design change. I agree, before it was missing the recomb color entirely, and this was confusing as well, but at least the design was clear.
how about we add the display name of the item at the first line, then a empty line, then the base item + everything else? just for the estimated chest value, not touching the actual estiamted item value logic here.
additionallly, the changelog wording is not precise. this affects only the estimated chest value, not the estimated item value.
| /** | ||
| * Returns the item name with its full color, i.e. after recombobulation | ||
| */ | ||
| val ItemStack.itemNameFullyRarityAware: String |
There was a problem hiding this comment.
what is the difference between this and the display name?
There was a problem hiding this comment.
if the purpose is to remove the reforge name, maybe its better to have the function be called nameWithoutReforge instead?
| @@ -700,7 +700,7 @@ object EstimatedItemValueCalculator { | |||
| } | |||
| } | |||
|
|
|||
| val name = internalName.itemName | |||
| val name = stack.itemNameBaseRarityAware | |||
There was a problem hiding this comment.
we dont want to show the recomb color here, and since the reforge name is also not part of the clean item name, i dont see why this line change is necessary
| @@ -49,6 +50,9 @@ object EstimatedItemValue { | |||
| var bookBundleAmount = mapOf<String, Int>() | |||
| private var currentlyShowing = false | |||
|
|
|||
| lateinit var itemValueCalculationData: ItemValueCalculationDataJson | |||
There was a problem hiding this comment.
this should be null, not lateinit. starting with a outdated repo will cause ugly errors instead of clean repo errors
Dependencies
What
Fixes an issue with items not showing their correct colors, both due to varying name, and recombobulation not being accounted for. If this was intentional let me know and I'll make it a config option.
Images
Before
After
Changelog Fixes