foundryvtt / dnd5e

An implementation of the 5th Edition game system for Foundry Virtual Tabletop (http://foundryvtt.com).
MIT License
333 stars 222 forks source link

Some Spells needs more complicated Scaling formula #313

Closed aaclayton closed 3 weeks ago

aaclayton commented 4 years ago

Originally in GitLab by @ariakas81

Spells like Spiritual Weapon, Flame Blade scale by 1dX for every two slot levels above their basic level. Right now they can't scale properly with the available choices.image

aaclayton commented 3 years ago

Originally in GitLab by @tpnils

I do believe she is correct that this is a bug and a dnd5e specific issue which occurs when trying to simplify the roll. It currently tries to simply group rolls which is, as you said, a complex factor. The sollution to this is, do not simplify it (insert 4head meme here). I will try to go over my findings to explain this solution.

First off, I have only encountered a few damage modifying options in vanilla dnd5e, it is probably limited to 1 time rerolls, min rolls ({group}kh) and recursive rerolls (1d8x1kh) with the help of feats and/or magical items. Next, I would always expect that the scaling is {base formula} + {scaling formula} where each level of scaling adds another + {scaling formula}. Obviously simplifing the formula is nice but the priority of the correct calculation should be higher than simplifiying it.

So when/how can you simplify? Basically anything that doen't group rolls, cause each scaling "level up" should be considered it's own group. So here is a list of the current roll modifiers which affect each die individually so these could be simplified in any combination. Or said differently, the roll can't be simplified if there is a modifier that isn't one of these

Additionally, is there any reason why the numerical values don't get scaled up as this breaks the 1d4+1 scaling of magic missile

aaclayton commented 3 years ago

I think the term "broken" is a bit harsh, but you are correct that automated scaling doesn't work properly in that case. Implementing the logic to understand how to (and when to) scale {1d6,2}kh by a constant factor is complex, and a core Foundry VTT issue.

aaclayton commented 3 years ago

Originally in GitLab by @Depili

Spell level scaling is also broken for features that allow treating a rolled number as some other number, for example when you can treat any rolls of 1 as 2.

Currently only way to specify such rolls is {1d6, 2}kh per dice and when adding two or more scaled levels this gets turned into {2d6, 2}kh by the dice multiplication call, instead of {1d6,2}kh + {1d6,2}kh which would be correct.

aaclayton commented 3 years ago

Originally in GitLab by @tpnils

Why do you consider the duration scaling valuable? There are 2 spells that scale the duration with a linear path and then there are a couple more variants which all scale differently. I believe this may cause more confusion than that it would help.

As for the custom functions, manual overrides seem the best option to me. Any other idea that sprung into my mind was over engineerd + the simplest solution is sometimes also the easiest to use from the user persective. As for how it is technically solved, I am sure that can be improved.

aaclayton commented 3 years ago

Thank you @tpnils for the information that you compiled, it's very helpful. My interpretation of the data is:

  1. A data-based way to provide spell scaling which alters the targets of the spell would be valuable.
  2. A similar data-driven way to provide spell scaling which alters the duration of the spell would be valuable.
  3. A way to provide a custom function that defines the scaling method for cases where level-based scaling is non-linear would be valuable.

The next step is a design discussion about the alternative ways of accomplishing these goals. One would be an array of override data objects as the initial issue suggests, but alternative solutions may also work.

aaclayton commented 3 years ago

Originally in GitLab by @tpnils

The only reasonable standard upcasting I see is the increment the targets and maybe the area. There are about 30 spells and 1 cantrip which increase the target amount with a linear pattern and 3 spells which increase the area. But other than that, I don't see a standard sollutions for all the other scaling options

There are

I was also thinking about a way where you could supply a "hook" to be able to add custom scaling options, but that would mean the SRD material already needs about 10+ custom scaling options and any homebrewer would need to be able to code.

I am interested to see what you think, but I still believe in the "enter every upcast manually" idea, though in combination with the increase of targets since it is a common occurance.

aaclayton commented 3 years ago

Originally in GitLab by @tpnils

Here you have a sheet with all the spell scaling options. This does not include anything else like the life cleric feature

https://docs.google.com/spreadsheets/d/1m8NSId3QeTK7RZQnR47tUUpS2TQXKsRh1e0PzA5b3a4/edit?usp=sharing

aaclayton commented 3 years ago

We can certainly look at the books when making this decision, we just can't publish complete or exactly as-written spell details of non-SRD content.

aaclayton commented 3 years ago

Originally in GitLab by @tpnils

I understand due to copyright we can't look at the books, but homebrew content may not match any predefined scaling. But in any case, I will get working on a list.

aaclayton commented 3 years ago

Originally in GitLab by @Depili

I don't think that SRD covers all of the scaling types that are in the official books, most notably in cantrips. The said cantrips would be handled by having two independent fields that can both scale and be displayed independently.

aaclayton commented 3 years ago

If we are going to move forward with this, I strongly believe we need to organize the work as follows:

  1. Establish consensus around the set of scaling types that we need the system to support
  2. Build the right underlying data structures in order to accomodate our solution
  3. Build user-facing interfaces for applying configuration rules
  4. Modify existing compendium packs to apply more advanced scaling systems.
aaclayton commented 3 years ago

Thanks for your thought and effort on this @tpnils. Before moving forward with any changes I have a few questions/comments that are needed in order to establish consensus and move forward.

  1. Can we please create a list the spells in the SRD which cannot scale "correctly" under the current system? If it requires an active effect in order to scale properly, let's include it in the list of spells that won't work properly without further work. I want to do this so that we have a clear understanding of what the different scaling modes that require accomodation by an improved system are. If we are going to make the scaling system substantially more complex, let's make sure the solution handles all known cases (within reason).

  2. Once (if) we have agreed to move forward, we should start with the underlying data model changes to support a custom scaling configuration. Only once the backend data structures are in place should we start thinking about the user interface.

  3. Once the backend support for a more advanced scaling system is in place, we can start to think about what is the best way to present that to the user in way that does not make the UI for spell configuration overly obtrusive and complex.

aaclayton commented 3 years ago

Originally in GitLab by @tpnils

I would like to summarize this issue and include some solutions which have been provided by some modules.

I have been working on the manual scaling in a fork, not really sure it Andrew will approve as I had not contacted him. The normal scaling options obviously remain but have been expanded with a manual scaling option based on spell level. The gray background is just a placeholder indicator so it's clear what the scaling part of the page is. The indicator will definitely be improved once I also get it to functionally work. image

aaclayton commented 4 years ago

I think there are too many edge case examples in this issue, each of which may need a different solution in terms of how it scales. I'm not sure there is a "one size fits all" solution - or if there is, it's too complicated a solution to be worth doing.

In order to make headway on this issue we need to simplify it and identify some lower-hanging fruit that can be changed without a comprehensive refactor.

aaclayton commented 4 years ago

Originally in GitLab by @gatesvp

For spells that deal more than one type of damage, Flamestrike is the canonical example here.

A level 7 Flamestrike could be: 4d6 (radiant) + 6d6 (fire) OR 5d6 (radiant) + 5d6 (fire) OR 6d6 (radiant) + 4d6 (fire).

Ice Storm has a similar issue where only the bludgeoning scales. So you can add the value in the "scaling formula", but there's no automation to type the damage.

However, there are some challenges here:

aaclayton commented 4 years ago

Originally in GitLab by @marcoseither

That'd be great. Sure, those Cantrips are not part of SRD, but being able to add them is great if you have bought the book. Another thing that should be considered are spells that deal more than one type of damage. If i understand your proposal, that would also be solved?

aaclayton commented 4 years ago

Originally in GitLab by @gatesvp

Hey @marcoseither , the two cantrips you mention are not SRD material, so they won't be part of the official content. But they would be solved with my proposal above.

That stated, even among the SRD content, Eldritch Blast currently doesn't scale targets correctly.

aaclayton commented 4 years ago

Originally in GitLab by @marcoseither

Other spells I currently do not know how to implement correctly are Booming Blade and Green-Flame Blade, both Cantrips with two separate level-based damage calculations.

aaclayton commented 4 years ago

Originally in GitLab by @Depili

Also current system fails with life domain clerics, which receive +1 healing per spell level, if you add 1d8+1 for level scaling for cure wounds the dice amount scales, but the +1 doesn't get multiplied. This can be worked around by using 1d8+1d1 but that is a ugly hack.

aaclayton commented 4 years ago

Originally in GitLab by @gatesvp

but from a UI/UX perspective setting up that data and entering it for a custom spell or item would be very painful

Totally agreed the UI input for "lay users" would not be trivial.

aaclayton commented 4 years ago

Fair points all - that being said I'd like to wait to see what problems still exist once nested dice expressions are supported.

I see the appeal of @gatesvp suggestion from a pure data modeling perspective, but from a UI/UX perspective setting up that data and entering it for a custom spell or item would be very painful. I'd like to find a better compromise between accuracy/depth of the data model and ease of user experience for homebrew or DMs who want to create their own spell entries.

aaclayton commented 4 years ago

Originally in GitLab by @gatesvp

I think the challenge with the current implementation is two-fold.

The current sheet has broken out damage scaling into "cantrip" or "spell level". This works for things like "Fireball", but it falls down for things like "Flame Blade".

In theory "nested roll expressions" can fix the Flame Blade example. But Flame Blade is just a more complex "damage scaling model". But "nested rolls" won't help with Flame Strike which actually involves decision and dice.

Nested rolls also don't fix spells that have different scaling mechanics without rolls.

For example:

If we want to really cover these cases (and allow for automation), we may need a different model. I'm proposing it here, because I think it also fixes "Flame Blade" without the need for "nested roll expressions".


Proposal:

Programatically, we take a spell + a casting level and then effectively re-write the spell before it gets passed to another part of the system.

Right now Flame Blade has:

"scaling": {
  "mode": "level",
  "formula": "1d6"
}

Under my proposal this would change to:

"scaling": {
  "level3": {"damage": { "parts": [["3d6","fire"]] },
  "level4": {"damage": { "parts": [["4d6","fire"]] },
  "level5": {"damage": { "parts": [["4d6","fire"]] },
  "level6": {"damage": { "parts": [["5d6","fire"]] },
  "level7": {"damage": { "parts": [["5d6","fire"]] },
  "level8": {"damage": { "parts": [["6d6","fire"]] },
  "level9": {"damage": { "parts": [["6d6","fire"]] },
}

The benefit of this system is that it can handle things like Hold Person:

"scaling": {
  "level3": { "target": { "value": 1, "units": "", "type": "creature" } },
  "level4": { "target": { "value": 2, "units": "", "type": "creature" } },
  "level5": { "target": { "value": 3, "units": "", "type": "creature" } },
  "level6": { "target": { "value": 4, "units": "", "type": "creature" } },
  "level7": { "target": { "value": 5, "units": "", "type": "creature" } },
  "level8": { "target": { "value": 6, "units": "", "type": "creature" } },
  "level8": { "target": { "value": 7, "units": "", "type": "creature" } },
}

With a slight modification it could also handle something like Flame Strike.

aaclayton commented 4 years ago

Thanks for making sure an issue exists about this. It may be a little while until it is fixed though, as the "right way" to fix it is to wait until nested roll expressions are supported in the core VTT and then I can make the number of dice to roll for level scaling a function of the upcast amount.

Fyorl commented 3 weeks ago

A lot of these use cases are handled by per-damage-part scaling, as well as the improved scaling functionality available in activities, so I am marking this as done for now to allow for more targeted issues to be raised for any remaining use cases.