Implementing the restructure changes

[PowerWyrm]

Of course in V there is no spells that use a random component with a bonus. But if one is added, better get something that works immediately instead of having to wonder why it doesnt work no?

[PowerWyrm]

So the plan is you can have an expression with actual dice (not just a base), and then still have an m_bonus? And in V where there are no &'s, the only change is the extra H in the G row, which has no effect because there are no expressions of that type (but presumably we could use those now).

I kind of suspect the whole thing was just cooked up by you and molybdenum to confuse me, but you have failed

Nope, you need an extra operator because operator + on L line returns C, not G. So operator + after base component flushes base and operator & after random component flushes sides.

[Nick]

I hope I got it right this time...

So the plan is you can have an expression with actual dice (not just a base), and then still have an m_bonus? And in V where there are no &'s, the only change is the extra H in the G row, which has no effect because there are no expressions of that type (but presumably we could use those now).

I kind of suspect the whole thing was just cooked up by you and molybdenum to confuse me, but you have failed

[Carnivean]

I hated programming in C, so I don't remember even the tiny bits I learnt, but this thread is amazing. It's like watching a mad scientist in action.

[PowerWyrm]

I've changed my mind and simply added another operator for the case to be able to use regular variables in expressions. So the use is "dice:base+bonus" for the normal case and "dice:base+random&bonus" for the case of spells with a random component.

/*
 * Input types for the parser state table.
 */
typedef enum dice_input_e
{
    [COLOR="Red"]DICE_INPUT_AMP,[/COLOR]
    DICE_INPUT_MINUS,
    DICE_INPUT_BASE,
    DICE_INPUT_DICE,
    DICE_INPUT_BONUS,
    DICE_INPUT_VAR,
    DICE_INPUT_DIGIT,
    DICE_INPUT_UPPER,
    DICE_INPUT_NULL,
    DICE_INPUT_MAX
} dice_input_t;

/* Catch specific characters before checking bigger char categories. */
    switch (c)
    {
       [COLOR="Red"] case '&':
            return DICE_INPUT_AMP;[/COLOR]
        case '-':
            return DICE_INPUT_MINUS;
        case '+':
            return DICE_INPUT_BASE;
        case 'd':
            return DICE_INPUT_DICE;
        case 'M':
        case 'm':
            return DICE_INPUT_BONUS;
        case '$':
            return DICE_INPUT_VAR;
        case '\0':
            return DICE_INPUT_NULL;
        default:
            break;
    }

/*
         * Get the next state, based on the type of input char. If it's a possible
         * number or varible name, we'll store the character in the token buffer.
         */
        switch (input_type)
        {
            [COLOR="Red"]case DICE_INPUT_AMP:[/COLOR]
            case DICE_INPUT_BASE:
            case DICE_INPUT_DICE:
            case DICE_INPUT_VAR:
            case DICE_INPUT_NULL:
                state = dice_parse_state_transition(state, input_type);
                break;
...

static unsigned char state_table[DICE_STATE_MAX][DICE_INPUT_MAX] =
    {
        /* Input:                                &-+dm$DU0 */
        /*[DICE_STATE_START] = */       /* A */ ".B.EHKB..",
        /*[DICE_STATE_BASE_DIGIT] = */  /* B */ "..CE..B.C",
        /*[DICE_STATE_FLUSH_BASE] = */  /* C */ "...EHKD..",
        /*[DICE_STATE_DICE_DIGIT] = */  /* D */ "...E..D..",
        /*[DICE_STATE_FLUSH_DICE] = */  /* E */ ".....KF..",
        /*[DICE_STATE_SIDE_DIGIT] = */  /* F */ "G...H.F.G",
        /*[DICE_STATE_FLUSH_SIDE] = */  /* G */ "....H....",
        /*[DICE_STATE_BONUS] = */       /* H */ ".....KI..",
        /*[DICE_STATE_BONUS_DIGIT] = */ /* I */ "......I.J",
        /*[DICE_STATE_FLUSH_BONUS] = */ /* J */ ".........",
        /*[DICE_STATE_VAR] = */         /* K */ ".......L.",
        /*[DICE_STATE_VAR_CHAR] = */    /* L */ "G.CEH..LM",
        /*[DICE_STATE_FLUSH_ALL] = */   /* M */ "........."
    };

I hope I got it right this time...

[Nick]

And it doesn't work...

Once you have it working, I'm happy to just slot it right in - part of the point of all this as the z-* files at least should not need to be changed for variants.

[PowerWyrm]

And it doesn't work...

I also need to declare the expression as "S-" instead of "S" because the parser considers the minus sign as a digit and not an operator.

[PowerWyrm]

Bah, there's also a Thunderstorm spell that uses the format "dice:$B+$Dd$S+m8" to display something like "dam 100+5d6x8" and that still doesn't work because the dice parser doesn't expect a "+" after anything else than the "base" value. For PWMAngband, I've simple hacked the parser to be able to use "-" as separator between dice and bonus so that the "dice" component is properly flushed.

The state table becomes:

static unsigned char state_table[DICE_STATE_MAX][DICE_INPUT_MAX] =
    {
        /* Input:                                -+dm$DU0 */
        /*[DICE_STATE_START] = */       /* A */ "B.EHKB..",
        /*[DICE_STATE_BASE_DIGIT] = */  /* B */ ".CE..B.C",
        /*[DICE_STATE_FLUSH_BASE] = */  /* C */ "..EHKD..",
        /*[DICE_STATE_DICE_DIGIT] = */  /* D */ "..E..D..",
        /*[DICE_STATE_FLUSH_DICE] = */  /* E */ "....KF..",
        /*[DICE_STATE_SIDE_DIGIT] = */  /* F */ "G..H.F.G",
        /*[DICE_STATE_FLUSH_SIDE] = */  /* G */ "...H....",
        /*[DICE_STATE_BONUS] = */       /* H */ "....KI..",
        /*[DICE_STATE_BONUS_DIGIT] = */ /* I */ ".....I.J",
        /*[DICE_STATE_FLUSH_BONUS] = */ /* J */ "........",
        /*[DICE_STATE_VAR] = */         /* K */ "......L.",
        /*[DICE_STATE_VAR_CHAR] = */    /* L */ "GCEH..LM",
        /*[DICE_STATE_FLUSH_ALL] = */   /* M */ "........"
    };

And I can use "dice:$Dd8-m4" for Elemental Bolt and "dice:$B+$Dd$S-m8" for Thunderstorm.

[PowerWyrm]

In dice_parse_state_transition(), there's no way to add a "bonus" part when the spell has a random component. For example, PWMAngband has an elemental bolt spell which fires four bolts, one of each element. I've tried "dice:$Dd8+m4" to be able to display "dam 5d8x4", but the state_table doesn't allow that (only "base" + "bonus" is allowed, like for Meteor Swarm).

Here's an updated, more complete state_table:

static unsigned char state_table[DICE_STATE_MAX][DICE_INPUT_MAX] =
    {
        /* Input:                                -+dm$DU0 */
        /*[DICE_STATE_START] = */       /* A */ "B.EHKB..",
        /*[DICE_STATE_BASE_DIGIT] = */  /* B */ ".CE..B.C",
        /*[DICE_STATE_FLUSH_BASE] = */  /* C */ "..EHKD..",
        /*[DICE_STATE_DICE_DIGIT] = */  /* D */ "..E..D..",
        /*[DICE_STATE_FLUSH_DICE] = */  /* E */ "....KF..",
        /*[DICE_STATE_SIDE_DIGIT] = */  /* F */ ".G.H.F.G",
        /*[DICE_STATE_FLUSH_SIDE] = */  /* G */ "...H....",
        /*[DICE_STATE_BONUS] = */       /* H */ "....KI..",
        /*[DICE_STATE_BONUS_DIGIT] = */ /* I */ ".....I.J",
        /*[DICE_STATE_FLUSH_BONUS] = */ /* J */ "........",
        /*[DICE_STATE_VAR] = */         /* K */ "......L.",
        /*[DICE_STATE_VAR_CHAR] = */    /* L */ ".CEH..LM",
        /*[DICE_STATE_FLUSH_ALL] = */   /* M */ "........"
    };

[Nick]

This default section is now missing. To fix that, maybe add something like:

if (!gf_force_obvious(typ) && (monster_handler == NULL))
{
    context.skipped = TRUE;
    context.dam = 0;
}

after calling monster_handler().

Actually there are no null monster handlers now, just some empty ones, so I guess the thing is just to set

    context.skipped = TRUE;

in all those.

[PowerWyrm]

In project_m(), GF_ flags with no associated handlers are not processed anymore. Before the restructure, there was a big switch with a default section at the end:

switch (typ)
{
    case GF_MISSILE (and all obvious effects): ...
    case GF_FIRE (and all handled effects): ...
    default: context.skipped = TRUE; context.dam = 0;
}

This default section is now missing. To fix that, maybe add something like:

if (!gf_force_obvious(typ) && (monster_handler == NULL))
{
    context.skipped = TRUE;
    context.dam = 0;
}

after calling monster_handler().

[Nick]

Before the restructure, the "obviousness when seen" line was the first line processed for each type. Therefore these two bits of code should be reversed.

With this code, nonliving monsters will erroneously identify a wand of drain life when before they didn't.

Ah yes, thanks - I thought there was still something not quite right there.

[PowerWyrm]

Another glitch due to the restructure in project_m():

if (monster_handler != NULL)
  monster_handler(&context);

/* Force obviousness for certain types if seen. */
if (gf_force_obvious(typ) && context.seen)
  context.obvious = TRUE;

Before the restructure, the "obviousness when seen" line was the first line processed for each type. Therefore these two bits of code should be reversed. Check for example project_monster_handler_OLD_DRAIN():

static void project_monster_handler_OLD_DRAIN(project_monster_handler_context_t *context)
{
	if (context->seen) {
		rf_on(context->l_ptr->flags, RF_UNDEAD);
		rf_on(context->l_ptr->flags, RF_DEMON);
	}
	if (monster_is_nonliving(context->m_ptr->race)) {
		context->hurt_msg = MON_MSG_UNAFFECTED;
		[COLOR="Red"]context->obvious = FALSE;[/COLOR]
		context->dam = 0;
	}
}

With this code, nonliving monsters will erroneously identify a wand of drain life when before they didn't.

[Nick]

Just a little gameplay change I've noted, which is probably due to changing "do_poly" from integer to boolean. This change even occured before 3.5, so it's unrelated to the restructure.

Prior to 3.5, "do_poly" was an integer, the polymorphing effect was using "p_ptr->lev" to compute the chance of a monster resisting it:
- dam = p_ptr->lev
- do_poly = dam
- savelvl = randint1(MAX(1, do_poly - 10)) + 10

In 3.5, "do_poly" was changed into a boolean without anything else changed. This made the formula bugged (the second affectation probably turns do_poly to 1 on most systems), making savelvl always equal to 11.

In the latest code, the formula was simply changed to "savelvl = 11".

This change wasn't probably intended?

Yes, indeed. This particular effect has a long and complicated history, and it's not clear which bits are intentional. It does need sorting out.

[PowerWyrm]

Just a little gameplay change I've noted, which is probably due to changing "do_poly" from integer to boolean. This change even occured before 3.5, so it's unrelated to the restructure.

Prior to 3.5, "do_poly" was an integer, the polymorphing effect was using "p_ptr->lev" to compute the chance of a monster resisting it:
- dam = p_ptr->lev
- do_poly = dam
- savelvl = randint1(MAX(1, do_poly - 10)) + 10

In 3.5, "do_poly" was changed into a boolean without anything else changed. This made the formula bugged (the second affectation probably turns do_poly to 1 on most systems), making savelvl always equal to 11.

In the latest code, the formula was simply changed to "savelvl = 11".

This change wasn't probably intended?