NickMcConnell / NarSil

Unofficial rewrite of Sil 1.3.0
10 stars 2 forks source link

Timed effects which use change-grade #547

Open backwardsEric opened 2 days ago

backwardsEric commented 2 days ago

These are some observations from looking at the code. I haven't verified if they impact actual gameplay:

  1. The logic in player_set_timed() to determine which of the change-inc messages to use looks like it could trigger a NULL pointer dereference if the increase is greater than any level set by the change-inc directives for the effect. Instead of the "while (change > inc->max) { inc = inc->next; }" loop currently used, I'd use "while (change > inc->max && inc->next) { inc = inc->next; }".

  2. How effects with change-grade are displayed in the status line seems strange. In player_timed.txt, those effects do not specify any grades, so timed_effects[i].grade will be NULL in ui-display.c's prt_tmd(). Because of that, nothing is ever displayed for the effect and the contents of c_grade for the effect are not used outside of the initialization in parse_player_timed_change_grade(), cleanup in cleanup_player_timed(), and an assertion in player_set_timed(). To match what Sil does, it looks like inner part of the loop over timed effects in prt_tmd() should be:

if (player->timed[i]) {
    if (timed_effects[i].grade) {
        struct timed_grade *grade = timed_effects[i].grade;

        while (player->timed[i] > grade->max) {
            grade = grade->next;
        }
        if (!grade->name) continue;
        c_put_str(grade->color, grade->name, row, col + len);
        len += strlen(grade->name) + 1;
    } else if (timed_effects[i].c_grade) {
        struct timed_change_grade *cg = timed_effects[i].c_grade;

        while (player->timed[i] > cg->max && cg->next) {
            cg = cg->next;
        }
        if (cg->name) {
            char *meter = format("%s %-3d", cg->name, player->timed[i]);

            c_put_str(cg->color, meter, row, col + len);
            len += strlen(meter) + 1;
        }
    }
backwardsEric commented 2 days ago

On looking at Sil some more, Sil displays "Bleeding %-2d" for cuts when the value is less than 100 and simply prints "Mortal wound" without a value if the value is greater than 100. So it looks like the change-grade directive in player_timed.txt should have a third parameter specifying the number of digits to use for the value and if that number of digits is not positive, the name is used without a value.