CheckUnique function always generates the same unique items

[peter-lang]

Operating System

Mac

DevilutionX version

1.4.1 (latest release)

Describe

It seems that if there are multiple unique items available for the same base-type, the game always spawns the last from the UniqueItems list.

Although I got about 5 unique great swords, 10 unique rings and 10 unique amulets through Normal Lazarus runs, they always were "Shirotachi", "Gladiator's Ring" and "Acolyte's Amulet".

In the CheckUnique function the code, I can confirm, that it does take the last applicable element, see:
https://github.com/diasurgical/devilutionX/blob/master/Source/items.cpp#L1402
https://github.com/diasurgical/devilutionX/blob/master/Source/itemdat.cpp#L429

I would expect to have the same random selection logic as in:
https://github.com/diasurgical/devilutionX/blob/master/Source/items.cpp#L1309

To Reproduce

1. Farm unique items in Lazarus Runs in Hellfire
2. Wait until unique "Great Sword", "Ring" or "Amulet" drops
3. Great sword always will be "Shirotachi", ring is "Gladiator's Ring" and amulet is "Acolyte's Amulet"

Expected Behavior

When multiple unique would be generated of the same type, there is equal chance to drop any of them.

Additional context

No response

[qndel]

#628
Is this the bug where if items have same lvl range, it always picks the last (vanilla one) or something new?

[peter-lang]

#628 Is this the bug where if items have same lvl range, it always picks the last (vanilla one) or something new?

It is happening in Hellfire, but it seems the same. It does not need to be the same lvl range, it's enough that we are in both level ranges, then the later item in the UniqueItemList would be picked. I've checked the proposed fix, it seems to fix this issue as well. A randomization of the picked item, when multiple uniques suffice the prerequisites, would solve this issue.

[DakkJaniels]

This is how the game worked, look at Jarulf's Guide.

[peter-lang]

You are right, Jarulf's guide chapter 3.8.3 describes this logic and 3.5.2 describes the uniques that are not findable at all because of this. Closing this issue.

[qndel]

@peter-lang I mean the issue is valid if we don't have one for it already - we surely have a PR with fix

[Sduibek]

@DakkJaniels @peter-lang Wait so if the behavior is clearly a bug / unintended behavior, has been reported and documented for many years, is causing a negative gameplay experience which also completely prevents access to game content, why are we specifically choosing to not fix it?

Why even make a project like this if we're going to go out of our way to not fix bugs? I'm struggling to understand the logic here.

To phrase it another way, just because a bug is documented in Jarulf's Guide doesn't mean it's now a thing that shouldn't be fixed; that makes no sense. Being in his guide or not is entirely irrelevant here.

As evidence of this: many other bugs he documented in the guide have already been fixed in/by DevilutionX...

[qndel]

@Sduibek don't comment in old&closed issues, there's discord for discussing things

[DakkJaniels]

Why even make a project like this if we're going to go out of our way to not fix bugs? I'm struggling to understand the logic here.

@Sduibek because you don't fully understand the issue. I was just pointing out that this was the original behavior. The reason why it is has not been fixed has been documented in the other issues that duplicate this one.

[AJenbo]

why are we specifically choosing to not fix it?

We aren't