Support for unidentified and corrupted items #1063
Conversation
| @@ -26,6 +26,8 @@ public class ItemHoverViewModel | |||
| public string DescriptionText { get; private set; } | |||
| public string SecondaryDescriptionText { get; private set; } | |||
| public bool IsCorrupted { get; private set; } | |||
| public bool IsUnidentified { get; private set; } | |||
| public bool IsCorruptedOrUnidentified { get; private set; } | |||
There was a problem hiding this comment.
Remove the setter, and have the getter look directly at this.IsCorrupted and this.IsUnidentified, so we don't have to duplicate the logic (and remember to keep multiple things in sync when changing any of them).
|
|
||
| public bool Applicable(Item item) | ||
| { | ||
| if (item.TypeLine.Contains(" Flask") || item.TypeLine.Contains("Sacrifice at ") || item.TypeLine.Contains("Mortal ") || item.TypeLine.Contains(" Key") || item.TypeLine.Contains("Fragment of the ") || item.TypeLine.Contains(" Breachstone")) |
There was a problem hiding this comment.
We should prioritize quicker checks first. The first thing is to check if item.Corrupted is true. (And we shouldn't have to make that check multiple times in the function.)
These type line checks are for various kinds of fragments, right? We should limit this, then, to when the item is not a Gear object. There's no reason to run these [relatively] expensive string comparisons when the object is a Gear. Also we can use StartsWith or EndsWith for some of these, instead of the more expensive Contains.
Also, MaxStackSize is a property of Item, so we can check that before casting to a Gear object.
There was a problem hiding this comment.
MaxStackSize is just to exclude divination cards from gears, others are map fragments and flasks, there may be better ways to exclude those
There was a problem hiding this comment.
Flasks, map fragments and breachstones come up as part of Gear, that's why I excluded them first
There was a problem hiding this comment.
please check the new commit
|
|
||
| public bool Applicable(Item item) | ||
| { | ||
| if (item.TypeLine.Contains(" Flask") || item.TypeLine.Contains("Sacrifice at ") || item.TypeLine.Contains("Mortal ") || item.TypeLine.Contains(" Key") || item.TypeLine.Contains("Fragment of the ") || item.TypeLine.Contains(" Breachstone")) |
There was a problem hiding this comment.
Split this line across multiple lines.
|
|
||
| public bool Applicable(Item item) | ||
| { | ||
| return !item.Identified; |
There was a problem hiding this comment.
For items that can't be unidentified, is item.Identified always true? If so, add a comment explaining this, and thus why we don't need a more complicated check (like we do for the IdentifiedItemFilter).
There was a problem hiding this comment.
yes that appears to be the case, haven't seen any issues with the way these filters work
|
Reverted itemhover changes since other pull request now contains bigger changes including these |
Added "Unidentified" text for unidentified items, added filters for unidentified and corrupted items and ensured "unidentified corrupted" maps look same as in-game. closes #995
screenshot:
Added identified and uncorrupted item filters which only return maps and gear that can be identified or corrupted