Spells with multiple damage types aren't represented

[Colorblindninja]

Taking the spell Ice Storm for example:

Ice Storm is an AoE spell that deals (at base level) 2d8 bludgeoning damage + 4d6 cold damage. The API lists the spell damage as follows:

   "damage": {
      "damage_type": {
        "index": "bludgeoning",
        "name": "Bludgeoning",
        "url": "/api/damage-types/bludgeoning"
      },
      "damage_at_slot_level": {
        "4": "2d8 + 4d6",
        "5": "3d8 + 4d6",
        "6": "4d8 + 4d6",
        "7": "5d8 + 4d6",
        "8": "6d8 + 4d6",
        "9": "7d8 + 4d6"
      }
    },

It correctly captures that two different damage types are used for the respective d8 and d6 rolls. But does not identify the damage type and also missed the cold damage type altogether.

Not sure the optimal representation for this, but I suggest the keys for "damage_at_slot_level" and "damage_at_character_level" to be a subtype of "damage_type". Additionally "damage_type" would replace the current "damage" key, as "damage" seems redundant since I believe there are no times where a damage value exists without belonging to a damage_type Resulting in this instance for the below fix for Ice Storm

"damage_type": [
      {
        "index": "bludgeoning",
        "name": "Bludgeoning",
        "url": "/api/damage-types/bludgeoning",
        "damage_at_slot_level": {
        "4": "2d8",
        "5": "3d8",
        "6": "4d8",
        "7": "5d8",
        "8": "6d8",
        "9": "7d8 "
        }
      },
      {
        "index": "cold",
        "name": "Cold",
        "url": "/api/damage-types/cold",
        "damage_at_slot_level": {
        "4": "4d6",
        "5": "4d6",
        "6": "4d6",
        "7": "4d6",
        "8": "4d6",
        "9": "4d6"
        }
      }
    ],

[bagelbits]

We'd probably want to represent it similar to how we do multi damage types for Monster Attacks.

[benjaminapetersen]

I'd offer a fix here by using an LLM to rip through the 2014 PDF and update this data. As just data processing, I think it's a good use of a model and doesn't violate human rights and agency and artistry.

[JuCJeff]

We'd probably want to represent it similar to how we do multi damage types for Monster Attacks.

Would you mind pointing me to where the multi damage types for Monster Attacks is? I am thinking of putting up a PR to address this

[bagelbits]

@JuCJeff A good example of this is Bite attack for Adult Red Dragon:

"damage": [
          {
            "damage_type": {
              "index": "piercing",
              "name": "Piercing",
              "url": "/api/2014/damage-types/piercing"
            },
            "damage_dice": "2d10+8"
          },
          {
            "damage_type": {
              "index": "fire",
              "name": "Fire",
              "url": "/api/2014/damage-types/fire"
            },
            "damage_dice": "2d6"
          }
        ]

[JuCJeff]

Thank you for providing the schema. I noticed with this change we will need to modify the damage field in SpellSchema

5e-database/src/2014/schemas/5e-SRD-Spells.ts

Line 30 in b755925

damage: SpellDamageSchema.optional(),

We would want to change it to an array: damage: z.array(SpellDamageSchema).optional(), similar to the monster attack schema

5e-database/src/2014/schemas/5e-SRD-Monsters.ts

Line 57 in b755925

damage: z.array(DamageSchema).optional(),

This makes sense to me as it covers multiple damage types and makes it a unified array to consume. Are we okay with this schema change?

Thought I'd mention it as it might cause regressions for downstream consumers, since it will alawys be an array instead of an object if the field exist now after this change. With all spells conforming to this, it could be a breaking change for the consumers on the API side. I do believe it's the right direction to go towards though for how the spell damage should logically be structured.

Any thoughts on how we want to go about it? @bagelbits

[JuCJeff]

I will happily put up another PR in https://github.com/5e-bits/5e-srd-api to accommodate this change so we could have the bases covered, if we decide we are okay going with this schema change 👍🏼