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.

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

loot system fixes and tweaks #2930

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

qndel
Copy link
Member

@qndel qndel commented Sep 23, 2021

Part 2 of the "oneliners that flip the game upside down"
Now required because of #2763
Turns out the previous loot fix actually makes some uniques unfindable as they were only findable because of that bug which we fixed. This will apply to all uniques dropped in devilutionx and they might morph into a different unique of the same base in vanilla - In my opinion that's acceptable as there's literally no way we can keep it compatible so there's no reason to even attempt to.
I think that along with #2763 this puts Diablo's loot in a nice state and I'd even consider the game almost playable ;)

  • Needs a fat changelog entry!

#edit
Added 1 way compatibility - uniques from vanilla won't morph, but devx uniques will morph in vanilla - there's simply no way around that :P

It's worth considering if we wouldn't want to improve the rng for regular magic items as well, since this PR is introducing a way to tell the difference between vanilla items and devilutionx ones.

@qndel qndel added enhancement New feature or request vanilla Bugs found also in the original Diablo 1.09b release fix fix for a bug critical A bug that is super serious labels Sep 23, 2021
@qndel qndel added this to the 1.3.0 milestone Sep 23, 2021
julealgon
julealgon previously approved these changes Sep 23, 2021
Copy link
Contributor

@julealgon julealgon left a comment

Choose a reason for hiding this comment

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

Scary. I like it.

galaxyhaxz
galaxyhaxz previously approved these changes Sep 23, 2021
Copy link
Member

@galaxyhaxz galaxyhaxz left a comment

Choose a reason for hiding this comment

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

LGTM. I tested against the PR I made 2 years ago (#540) and they both generate the exact same results. I still prefer my cleanup however as it makes the logic a bit more understandable.

@qndel qndel dismissed stale reviews from galaxyhaxz and julealgon via 01e9ae4 September 24, 2021 19:26
@qndel qndel force-pushed the fix_unique_drop branch 4 times, most recently from 265e1a3 to f8a5b11 Compare September 25, 2021 03:18
Source/inv.cpp Show resolved Hide resolved
Source/items.h Show resolved Hide resolved
@AJenbo AJenbo modified the milestones: 1.3.0, 1.4.0 Oct 7, 2021
@qndel
Copy link
Member Author

qndel commented Oct 9, 2021

@AJenbo if this goes into 1.4.0, I believe #2763 should be reverted for 1.3.0. They really need to go together imo.

@AJenbo
Copy link
Member

AJenbo commented Oct 9, 2021

Could you do so and then add it to this PR?

@qndel qndel changed the title allow all uniques to drop loot system fixes and tweaks Oct 9, 2021
@agris-codes
Copy link

agris-codes commented Oct 12, 2021

This is going to be a rehash of my feedback contained in another issue, but I think this is the correct location for it.

In fixing this issue, I think the problem of best-of-class items being unable to drop in higher difficulties due to the current qlvl minimum defined as mlvl / 2 should be fixed as well. I propose that the qlvl floor be decreased to mlvl / 3, which means items such as Obsidian Tower Shield of the Ages (qlvl 25), Full Plate Mail of Sorcery (qlvl 25), and "Weapon of Haste" derivatives (qlvl 27) can all be found as drops in higher difficulties, while also extending the period over which the player can discover very good items but not best-of-class (Weapon of Speed, for example). Plus add in all the uniques at qlvl 15 to 25 that are filtered by the current qlvl floor (http://www.bigd-online.com/JG/Body/JG3-5.html). It lets the loot system's depth really shine outside of the vendor system.

It also resolves another peculiarity of the current system, wherein some base item types are are over-represented in drops on Nightmare / Hell due to how they satisfy min(qlvl) requirements.

I certainly do not understand the nuances of how base item is decided and then affixes applied as well as others, like @qndel, but my suggestion of decreasing the floor as described above would go a long way to incentivizing playing at higher difficulties, to hunt for rare items.

The more monster slaying and less vendor shopping, the better.

@obligaron
Copy link
Contributor

Additional Question: if we change the item generation in a 1 way compatible way. Should we also use a better random number generator for the new item generator? It's not backwards compatible anyway.

@qndel
Copy link
Member Author

qndel commented Jan 22, 2022

Additional Question: if we change the item generation in a 1 way compatible way. Should we also use a better random number generator for the new item generator? It's not backwards compatible anyway.

Can't see why not

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
critical A bug that is super serious enhancement New feature or request fix fix for a bug vanilla Bugs found also in the original Diablo 1.09b release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Issue Report]: Unique rings and amulets drop are always the same.
6 participants