Simplify/Rework drop rules

[Chicken-Bones]

Do you intend to personally contribute/program this feature?

Yes

I would like to see this change made to improve my experience with

Mod capability as a Modder

Description

The current drop rules system is really hard to modify and work with. This is due to a couple factors:

• every rule gets its own 'wrapper chance', even if it's a 'selection' rule (like OneFromRulesRule or FewFromRulesRule etc
  • There only needs to be one rule which wraps another rule in a 'chance'
  • Stuff like OneFromRulesRule and ItemDropWithConditionRule don't need their own chance.
  • Luck scaling could just be one parameter of a 'chance' rule rather than luck and noluck variants for every rule
• there's no way to inspect the actual chance of a rule returning 'success' (and thus triggering a chain). If it is a 'wrapper rule'
  • Eg, one-from-rules with chance 1 always succeeds, but it the wrapped rule has its own chance, then it that may fail
• 'drop style' rules like DropLocalPerClientAndResetsMoneyTo0 and DropOneByOne and DropPerPlayerOnThePlayer have nothing todo with the chance, but require yet another rule type, making pattern matching harder for modders
• DropBasedOnExpert/MasterMode can't be generalised to modded difficulties. They should just be conditional rules.
• There's no need for ItemDropWithConditionRule it could just be a chained rule on LeadingConditionRule

All this makes rules a pain to modify by modders. We should provide some helper methods at least. Though finding the right rule-chain to modify is also likely to be a problem, potentially there should be a more compatible version of modify loot hooks where modders can modify the rules by adding to tables or altering rates, but not change the overall structure, and then a second pass where the structure can be changed (to add more conditions or such)

What does this proposal attempt to solve or improve?

No response

Which (other) solutions should be considered?

No response

[ThomasThePencil]

I like what you're goin' for here, but as someone who's had to mess with vanilla rules extensively for DD, I...can't help but feel that the initial pitch misses the mark a little bit on why so many, myself included, have found this new system to be a pain despite its many superiorities over the pre-1.4 approach. as such, I'd like to throw my hat into the ring here

one of the most major problems I've encountered --- and one I'm sure many others who've messed with vanilla rules in the past can attest to --- is that tryin' to actually get a hold of and either modify (in the case of e.g. mods like Calamity, which modify vanilla rates) or remove (in the case of yours truly) a rule that drops a specific item is, pardon my French, a pain in the ass. doin' so can often require a recursive call of a helper method which itself has to be extremely explicitly comprehensive, with little way to organically extend that helper method or generally make it not look like shit thanks to the interface-based nature of drop rules (a sentiment I have also found to be applicable to entity sources, though that's a much less solvable problem) the "simplest" solution would be to effectively write that recursive helper method into the drop table extensions normally, which I'm fine with, except it'd still look ugly because you're workin' with something that can't be extended in an easy or...robust? is that the word? whatever. fashion something which would take a lot more work, but would solve this problem much more effectively, would be to make drop rules extend from an abstract class as opposed to an interface, with a few core fields stored in that abstract class --- this would not only make writin' a few helper methods about handlin' vanilla drop rules better into tML itself much easier, but it'd also greatly improve accessibility for modders new and old alike in terms of modifyin' rules without havin' to check against each rule type individually

in any event, that's the main thing I wanted to bring up. essentially, what I'm after boils down to the followin' current deficiencies:

• a way to easily give any rules that contain a given item the axe (e.g. if I want to keep a given enemy from droppin' the Megaphone at all due to makin' it obtained elsehow, I'd like a way to easily remove all instances of that drop from the enemy)
• a way to tell what mod (if any) added a drop rule, as well as if mods have changed existin' drop rules at all (see below). establishes consistency with our recent push towards transparency on when mods change things, and allows the Bestiary to be even more useful than it already is
• a way to possibly change either the condition or the chance for an existin' drop rule without havin' to cut it and make a new one (as doin' so can be a huge chore in cases like boss drops, where 90% of the items come down to chained rules)

[Chicken-Bones]

1.4.4 milestone removed as we don't have a good plan of attack for the timeline

[Jacklz]

I have a couple ideas I'd like to share here when I get around to finishing them. I've never made a PR before and I wouldn't even know where to start when it comes to replacing the current loot system with some of the ideas I've got. I may post some code/class snippets here when I finish it to see what people think.

Currently my idea would be to use a single class that you can either inherit from or create new instances of. The class contains multiple constructors for a more flexible way to manage creating new drop rules. Its current structure allows you to disable any item drop within the rule while also being able to change any aspect of the drop such as the item, minimum stack, maximum stack, drop rate, etc. It also gives more customization for an item drop such as being able to define an actual Item rather than using item.type to declare a new item. However you can still use the item.type to create a default new item if you want.

Another change I'm working on is how a drop is determined. I've switched to just using a ListCondition to determine if a drop is eligible. A CanDrop() method checks if any condition is false and will prevent the item from dropping if so. Drops will still have a drop chance, the chance is only calculated if all the conditions are met. The current DropAttemptInfo struct includes a player and an npc variable both of which can have additional logic checked for by overriding two virtual methods I've declared in the new drop rule class.

Lastly I'm not too familiar with how the current OnFailedRoll and OnSuccessRoll system works but I've designed the drop method to return the reason why the drop failed or succeeded along with being able to add new rules for both the failure and success routes if a user so chooses.

I don't know if CB has done any work on this rework and I may be out of my league here but let me know what you think.