Bungie-net / api

Resources for the Bungie.net API
Other
1.22k stars 92 forks source link

Deepsight weapons not using `ItemState.HighlightedObjective` #1837

Closed joaopmarquesini closed 1 year ago

joaopmarquesini commented 1 year ago

I just realized that Deepsight weapons doesn't return ItemState.HighlightedObjective anymore, is it intended behaviour ? Right now I'm using this snippet to find if weapons are deepsights, but it's a bit less performant than checking item.state.

final isDeepSight = itemInfo.sockets?.any((element) {
          final isEnabled = element.isEnabled ?? false;
          if (!isEnabled) return false;
          final isVisible = element.isVisible ?? false;
          if (!isVisible) return false;
          final socketDef = context.definition<DestinyInventoryItemDefinition>(element.plugHash);
          return socketDef?.plug?.plugCategoryIdentifier?.contains('crafting.plugs.weapons.mods.memories') ?? false;
        }) ??
        false;
jshaffstall-bng commented 1 year ago

Good catch, thanks for the heads up. It looks like something changed on the game content side for 'deepsight' weapons with the release of Season 21.

I don't have an ETA for an API fix, but we'll look into this soon.

joaopmarquesini commented 1 year ago

maybe something related to how deepsight weapons doesn't have an objective attached to it (as they can be "redeemed" immediately instead of needing to complete a progress bar as they were before)

jshaffstall-bng commented 1 year ago

It looks like the right way to do this now is by checking for an active tooltip notification with this display style string: ui_display_style_deepsight

"tooltipNotifications": [
  {
    "displayString": "This weapon's Pattern can be extracted.",
    "displayStyle": "ui_display_style_deepsight"
  }
]

We will also update the D2 API response to apply the ItemState.HighlightedObjective to items that have an active tooltip with this display style.

joaopmarquesini commented 1 year ago

On a side note: DestinyVendorSaleItemComponent doesn't have the tooltipNotificationIndexes property, right ?

jshaffstall-bng commented 1 year ago

...well maybe they should! 😹

jshaffstall-bng commented 1 year ago

This should be fixed now for non-Vendor inventory items. No ETA yet on adding tooltip indices to vendor sale item components.

joaopmarquesini commented 1 year ago

Nice! Btw ItemState was already right for vendor items