DestinyItemManager / DIM

Destiny Item Manager
https://destinyitemmanager.com
MIT License
2.08k stars 641 forks source link

Unable to select more than 2 raid mods at a time in the ModPicker sheet #6402

Closed chrisdouglass closed 3 years ago

chrisdouglass commented 3 years ago

What version of DIM are you using? Repros on live and beta

What Browser and OS are you using? Repros on Chrome 87 & 88

Describe the bug and how to make it happen After selecting raid mods, the ModPicker sheet incorrectly limits raid mods to only 2 at a time regardless of the raid.

Repro steps: 1) In game, equip 5 pieces of raid gear from the garden of salvation raid. 1) In game, apply the Relay Defender mod to all 5 to prove it's legal to have 5 of that mod in a loadout. 1) In DIM, open Loadout Manager, then tap Add Mod. 1) Attempt to add Relay Defender five times.

Expected Results: I can select up to 5 relay defenders.

Actual Results: I can select only 2.

chrisdouglass commented 3 years ago

So part of the issue is this line:

https://github.com/DestinyItemManager/DIM/blob/486b95ebdb06cabec9978e3f14e4838517f5d81b/src/app/loadout-builder/filter/ModPicker.tsx#L326

Which will always limit all raid mods to 2 max. Changing that to be 5 for all cases allowed me to select more than 2, but I believe the calculation for what was legal was still wrong. It's kind of hard to tell without going in game and equipping things to verify mod combinations should be available to DIM.

ryan-rushton commented 3 years ago

@chrisdouglass we need some better logic around choosing the maximum selectable. For slot specific it is 2. For general it is 5. Things get a bit wild when we deal with the other categories as we need to deal with legacy mod slots and armour with combat/raid slots. For now extending the IsGeneralOrSeasonal to include raid mods will kinda fix things but there will be cases when users can get into trouble and not realise they have too many mods selected.

I am planning on rewriting a lot of this stuff to be driven off the plug category hashes but its a decent size job and I have just had a little one, so its a bit hard to get more than 30mins where I can focus on something.

ryan-rushton commented 3 years ago

I know you were looking at it but I will raise a PR now as I have that 30 mins spare. If you have something already just let me know and I will delete mine.