Wrong damage display for charged spells

[AmProsius]

Describe the bug The damage display of charged spells doesn't reflect the actual damage (see Mondgesaenge for correct values).

Expected behavior The damage display of charged spells now reflects the actual damage.

Additional context Bug provided by Blubbler.

@szapp Mondgesaenge also speaks of rechargeable spells.

[szapp]

@szapp Mondgesaenge also speaks of rechargeable spells.

Maybe we can agree to just referring to them as “charged spells” like in #130.

[szapp]

I just had a look at Spell_Thunderball aka. "Ball Lightning" to understand the problem. It seems there is a discrepancy in description, between "damage per spell level" and "damage per mama", which are not the same. Spell levels are the step-wise changes of the effects of a spell while charging it (e.g. fireball increases in the hand of the player character with each spell level which is increased after a certain time of charging). It also serves as a multiplier of the spell damage: spell.level * spell.damage_per_level.

An easy fix would be to change item text from showing "Damage (per mana)" to "Damage (per spell level)" https://github.com/AmProsius/gothic-1-community-patch/blob/56e45df879d2078a436491a8df7a7142f19788c9/scriptbase/_work/Data/Scripts/Content/Items/Artifacts.d#L912

However, that will have no meaning to the player. These spell levels are only internally represented. That will be as uninformative as it is currently.

The numbers we have at hand are:

• SPL_DAMAGE_THUNDERBALL / C_Spell.damage_per_level: The damage per level
• SPL_SENDCAST_THUNDERBALL: The maximum mana that can be invested
• C_Spell.time_per_mana: The time it takes to invest one mana point

The number of levels a spell can go through is not stored as a number. The spell logic function merely increases the spell level while the player casts the spell given certain conditions. It is impossible to determine the number of spell levels analytically (that would require a script that could understand arbitrarily written functions). Since we don't know the maximum number of levels, we cannot relate the damage-per-level to the damage-per-mana.

Unfortunately, I don't see a way of fixing this issue. We could hard-code the new damage-per-mana values into the item text. But that would make everything worse if a mod changed the number of spell levels (to what number we cannot determine). A compromise - that would still be a lot of work - would be to match the byte code of the spell logic function against how we expect it to be, i.e. as in the original scripts. Only in that case, we know the spell was not altered and we can place the pre-calculated damage-per-mana values into the item instance script. As an illustration for the "Ball Lightning" spell

1. Check if the constant SPL_DAMAGE_THUNDERBALL exists and is 20.
2. Check if the constant SPL_SENDCAST_THUNDERBALL exists and is 5.
3. Check if the latter matches the damage_per_level property of the spell instance Spell_Thunderball.
4. Check if the byte code of the item instance function ItArRuneThunderball shows the expected wrong values of damage-per-level = SPL_DAMAGE_THUNDERBALL.
5. Do the same for the scroll item ItArScrollThunderball. (What if one is wrong and the other is correct?)
6. Check if the byte code of the spell logic function Spell_Logic_Thunderball matches exactly the original scripts. (Special treatment for existent/non-existent calls to debugging functions, e.g. PrintDebugNpc.)
7. Only then update the item instance function to show the hard-coded damage-per-mana = 60 (3 level * 20 damage-per-level).

Seems like a ton of work - which will be different for each spell.

We can leave this here for a while. Maybe someone will come up with an alternative.

[szapp]

The best solution that comes to mind is to replace the item text

TEXT    [2]         =   ConcatStrings(NAME_Dam_Magic,NAME_PerMana); // "Magic Damage" + " (per mana)"

to

TEXT    [2]         =   ConcatStrings(NAME_Dam_Magic, " (per charge)"); // German: " (pro Aufladung)"

I think that will be most intuitive and understandable to the player, as the individual charge (i.e. level) is visible from the spell effect.

This is a draft for implementation. Some lines of code are marked with // ## TODO: ... ## that require changes.

Note that here, the fix will only be marked as applied if all instances have been updated. If not all of them were found or changed, the remaining will be still fixed, but the applied-status will be NOT APPLIED.

/*
 * #222 Wrong damage display for charged spells
 */
func int G1CP_222_ChargedSpellsText() {
    // Check necessary string symbols
    if (!G1CP_IsStringConst("NAME_Dam_Magic", 0))
    || (!G1CP_IsStringConst("NAME_PerMana", 0)) {
        return FALSE;
    };

    // Retrieve strings from symbols
    var string NAME_Dam_Magic; NAME_Dam_Magic = G1CP_GetStringConst("NAME_Dam_Magic", 0, NAME_Dam_Magic);
    var string NAME_PerMana;   NAME_PerMana   = G1CP_GetStringConst("NAME_PerMana",   0, NAME_PerMana);   
    var string curString; curString = ConcatStrings(NAME_Dam_Magic, NAME_PerMana);

    // Get new string by language
    var string perCharge;
    if (G1CP_Lang == G1CP_Lang_EN) {
        perCharge = " (per charge)";
    } else if (G1CP_Lang == G1CP_Lang_DE) {
        perCharge = " (per Aufladung)";
    } else if (G1CP_Lang == G1CP_Lang_PL) {
        perCharge = " (per charge)"; // ## TODO: Add Polish translation ##
    } else if (G1CP_Lang == G1CP_Lang_RU) {
        perCharge = " (per charge)"; // ## TODO: Add Russian translation ##
    } else {
        // Other languages not supported here
        return FALSE;
    };

    // Create replacement string
    const string newString = ""; // Must be constant
    newString = ConcatStrings(NAME_Dam_Magic, perCharge);

    // Apply to all instance functions
    var int symbId;
    var int numApplied; numApplied = 0;
    const int numInstances = 2; // ## TODO: Update with the number of instances ##

    // ThunderBall rune
    symbId = G1CP_GetItemInstID("ItArRuneThunderball");
    numApplied += (G1CP_ReplaceAssignStr(symbId, "C_ITEM.TEXT", 2, curString, newString) > 0);

    // ThunderBall scroll
    symbId = G1CP_GetItemInstID("ItArScrollThunderball");
    numApplied += (G1CP_ReplaceAssignStr(symbId, "C_ITEM.TEXT", 2, curString, newString) > 0);

    // ... 
    // ## TODO: Add all rune and scroll items of affected charged spells here ##
    // ...

    // Return success only if applied to all
    return (numApplied == numInstances); 
};

[szapp]

I see a critical compatibility issue with the proposed fix above. It does not consider that a mod may have changed the parameters of a spell, e.g. a spell may no longer be a charged spell. This seems like a very complicated problem.