Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add breakermod to match weapons with seasonal artifact breaker mods #8914

Merged
merged 7 commits into from
Sep 3, 2024
Merged

Add breakermod to match weapons with seasonal artifact breaker mods #8914

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
34fa5a5
Add breakermod to match weapons with seasonal artifact breaker mods
robojumper Dec 31, 2022
d71ced8
fixes
delphiactual Jun 17, 2024
92bf020
Merge branch 'master' into breakermod-artifact-breakers
delphiactual Jun 19, 2024
48f7473
.
delphiactual Jun 19, 2024
bca2cbc
update snapshot
delphiactual Jun 19, 2024
cea58d1
Consolidate breakermod/breaker searches, show effective breaker on
bhollis Sep 3, 2024
94401d9
Merge remote-tracking branch 'origin/master' into breakermod-artifact…
bhollis Sep 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitmodules
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
[submodule "destiny-icons"]
path = destiny-icons
url = https://github.com/justrealmilk/destiny-icons
branch = master
1 change: 1 addition & 0 deletions config/i18n.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@
"Ascended": "Shows items that have an ascend node which have been ascended.",
"Unascended": "Shows items that have an ascend node which have not been ascended.",
"Breaker": "Filter by breaker type or corresponding champion type.",
"BreakerMod": "Filter by available anti-champion artifact mods for this breaker type or corresponding champion type.",
"BulkClear": "Removed tag from 1 item.",
"BulkClear_plural": "Removed tags from {{count}} items.",
"BulkRevert": "Reverted tag on 1 item.",
Expand Down
2 changes: 1 addition & 1 deletion destiny-icons
Open in desktop
Submodule destiny-icons updated 1 files
+1 −0 general/shaped.svg
1 change: 1 addition & 0 deletions src/app/search/__snapshots__/search-config.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ exports[`buildSearchConfig generates a reasonable filter map: key-value filters
"armorintrinsic",
"basestat",
"breaker",
"breakermod",
"catalyst",
"count",
"deepsight",
Expand Down
20 changes: 20 additions & 0 deletions src/app/search/items/search-filters/known-values.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
DestinyClass,
DestinyRecordState,
} from 'bungie-api-ts/destiny2';
import artifactBreakerMods from 'data/d2/artifact-breaker-weapon-types.json';
import { D2EventEnum, D2EventInfo } from 'data/d2/d2-event-info-v2';
import focusingOutputs from 'data/d2/focusing-item-outputs.json';
import { BreakerTypeHashes, ItemCategoryHashes } from 'data/d2/generated-enums';
Expand Down Expand Up @@ -228,6 +229,25 @@ const knownValuesFilters: ItemFilterDefinition[] = [
return (item) => breakerType.includes(item.breakerType?.hash as BreakerTypeHashes);
},
},
{
keywords: 'breakermod',
description: tl('Filter.BreakerMod'),
format: 'query',
suggestions: Object.keys(breakerTypes),
destinyVersion: 2,
filter: ({ filterValue }) => {
const breakerType: BreakerTypeHashes[] | undefined =
breakerTypes[filterValue as keyof typeof breakerTypes];
if (!breakerType) {
throw new Error(`Unknown breaker type ${breakerType as string}`);
}
const breakingIchs = breakerType.flatMap((ty) => artifactBreakerMods[ty] || []);
Copy link
Contributor

Choose a reason for hiding this comment

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

The tricky thing here is that not all of the artifact breaker mods may be active. Do we want to care about that? I assume most folks enable all the breaker mods, at least the basic ones, but then there's always some later column one that I ignore or haven't unlocked...

The tricky part of course being that each character has their own artifact.

Copy link
Contributor

@DHager DHager Aug 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

All the options have problems, but I find thinking "the principle of least surprise": Which model is easiest for users to recognize and understand?

To me, that's the one where the filter is showing consistent breaker potential via mods that may-or-may-not be unlocked yet, as opposed to "works on this character, but not on that character", or "worked yesterday, but not today", etc. In other words, filter results should only change when Bungie alters the seasonal-artifact.

return (item) =>
Boolean(
!item.breakerType && item.itemCategoryHashes.some((ich) => breakingIchs.includes(ich)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this really be a new search, or should the breaker search just take into account unlocked artifact mods? What's the use case for finding items that are only granted breaker by the artifact?

And then we'd probably want to think about whether to show the breaker icon on the item tooltip.

Copy link
Contributor

@DHager DHager Aug 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Should this really be a new search, or should the breaker search just take into account unlocked artifact mods?

I'm in favor of combining the logic into breaker, provided it handles cases like "Wish-Ender has intrinsic anti-barrier therefore it cannot get anti-overload from Overload Bow."

I can only think of one scenario where I might want to filter for the difference, and that's where I'm desperately trying to figure out if I can disable/avoid a champion mod and unlock something else without ruining a build... But in practice I tend to treat those seasonal mod choices as mandatory.

That said, it may be valuable to expose the breaker-"reason" as something that can be spot-checked on a particular item, such as an icon/tooltip indicating that Le Monarque has Anti-Overload intrinsically, while Hush is getting Anti-Overload from a seasonal mod.

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case, would you expect the breaker icon to also display on the item tile if an artifact mod could grant it breaker?

Copy link
Contributor

@DHager DHager Aug 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

would you expect the breaker icon to also display on the item tile if an artifact mod could grant it breaker?

Yeah, as someone who plays Too Damn Much, all the exotic weapons with breaker-status have permanent names/effects, distinct icons, and tend to get memorized and I don't even look for any icons. So showing permanent and seasonally-conditional results together is more helpful.

);
},
},
{
keywords: 'foundry',
description: tl('Filter.Foundry'),
Expand Down
1 change: 1 addition & 0 deletions src/locale/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@
"ArmorIntrinsic": "Shows legendary armor which has an intrinsic perk, such as Artifice Armor.",
"Ascended": "Shows items that have an ascend node which have been ascended.",
"Breaker": "Filter by breaker type or corresponding champion type.",
"BreakerMod": "Filter by available anti-champion artifact mods for this breaker type or corresponding champion type.",
"BulkClear": "Removed tag from 1 item.",
"BulkClear_plural": "Removed tags from {{count}} items.",
"BulkRevert": "Reverted tag on 1 item.",
Expand Down
Loading