Replace classic rewards with "dynamic rewards"

[ScuffedNewt]

Similar to dynamic limits, dynamic rewards are designed to be a "drop-in" replacement for rewards

There are options for both recipient type (which can be hidden for player-facing UI like submission) and extra fields (can be used for weights, percentages, min/max... whatever someone wants to add)

also fixes a bug with character rewards, adds new feature of having character default rewards on prompts

there is also the ability to have the form as either a part of a higher workflow (ex. prompts) or similar to dynamic limits where it can be edited with its own form independently of other form submission

Why does this not have a grant function or builder for assetsArray?

I thought about this when creating it and decided against it to instead allow more freedom of how the arrays are created and distributed in a way that fits the traditional Lorekeeper sense, but I could include a simple builder if it's a want

[CH3RVB]

i'm just wondering what we should do about this PR: #1194 (originally was a full PR, currently it's in draft since i was suggested to do so since i haven't had the time/energy as of late to keep up with code much)

it was opened earlier and the ext has already been floating around for longer, i hate to cause friction but i know that i'd have to comment eventually... a good few other exts already rely on it and having to adapt those existing parts seems less ideal since it's already established... i would be willing to combine the two/use parts of this PR on mine if needed

[itinerare]

So, I understand that absent a solution to a given problem being merged into core, multiple solutions are liable to arise via extensions or otherwise, and "standardizing" one of them (i.e. merging it into core) is likely to cause issues for the others. We generally try to avoid breaking extensions and other modifications as a consequence of changes to core, or offer direction in the event that it's unavoidable, but ultimately it's a matter of "we'll do our best" and not a guarantee for simple practical reasons (there is too much going on downstream for that to be feasible).

I'm also not too fussed about which PR was opened first. In the event of multiple "competing" PRs, what's most relevant (from my perspective) is which one actually reaches a state that's fit to merge (complete, with multiple approving reviews and no outstanding changes requested) first, barring a compelling reason to hold off otherwise. Ultimately, the important part is getting something merged; even if we do not all agree on the precise details of what the solution to a problem should be, merging something into core provides a base on which to make additional changes (via subsequent PRs), adapt within extensions, and so on. There will, as mentioned, necessarily be some friction, changes required downstream, etc. as a consequence of this. This is unfortunate but I don't think worth avoiding for its own sake.

[ScuffedNewt]

added the ability to have weight extra field with same functionality as loot tables (displaying chance)

example usage:

        'extra_fields' => [
            'weight' => [
                'type' => 'number',
                'label' => 'Weight',
                'class' => 'loot-weight',
                'attributes' => ['step' => '0.01', 'min' => '0', 'max' => '100'],
                'default' => 100,
                'tooltip' => 'Weights are relative to other rewards. For example, a reward with weight 2 is twice as likely to be given as a reward with weight 1.',
            ],
        ]

in action on prompts:

I've chosen not to replace the Loot model with dynamic rewards since I think that would balloon this PR, also, I don't find it strictly neccessary since Loot is actually an assetType itself vs an object with rewards

[itinerare]

Yeah, I think it's probably best to keep Loot as-is.

[ScuffedNewt]

Yeah, I think it's probably best to keep Loot as-is.

yeah thinking it over, definitely needs to be untouched to remain atomic for assetType

[SpeedyD]

Out of curiosity, and this is actually an old question.. what is again the difference between _loot_select_row and _loot_select?...

and why do we duplicate code between them instead of just adding an include to a single "loot" file in both..

[ScuffedNewt]

So that the clone row was outside of the form and wasn't adding blank / null values in the submission

[itinerare]

Yeah, all of the clone forms have a split setup-- one version of the cloned setup for existing entries, one for new ones so that there's a blank to copy in. _loot_select_row, iirc, fills the latter role, while _loot_select supplies the whole rest of the setup. Since loot select as a whole is used multiple places, they both need to be available to be included separately (since as mentioned, the blank copy needs to be outside the form).

[SpeedyD]

Ah, I see. I never actually looked into it in-depth..

But I still have to wonder why we can't just.. make ANOTHER file and add the repeating text in there, and then include it in both files? 🤔

[Draconizations]

Had looked over the actual code a while ago and didn't notice anything that stuck out to me there, but I wanted to test it first. Didn't particularly notice any issues so this seems good to me!