Implementing the restructure changes

[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...

[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.

[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

[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.

[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]

A little omission when factoring out check_state() and adding timed flags display to character flag screen: TMD_BOLD doesn't trigger OF_RES_FEAR (little "!" next to pFear). Happens when quaffing !heroism for example (mushroom of clear mind, for example, correctly triggers "!" next to pConf).

[PowerWyrm]

Another glitch in calc_bonuses (but this one was already there before the restructure):

old code

if (player_has(PF_BLESS_WEAPON) && !check_state(p_ptr, OF_BLESSED, p_ptr->state.flags) &&

new code

if (player_has(PF_BLESS_WEAPON) && !player_of_has(player, OF_BLESSED) &&

Since at this point we try to fill a "state" structure, it's logical to use this structure instead of the player->state one...

The code should then be:

if (player_has(PF_BLESS_WEAPON) && !of_has(state->flags, OF_BLESSED) &&

[PowerWyrm]

A little omission when factoring out check_state() and adding timed flags display to character flag screen: TMD_BOLD doesn't trigger OF_RES_FEAR (little "!" next to pFear). Happens when quaffing !heroism for example (mushroom of clear mind, for example, correctly triggers "!" next to pConf).

Also TMD_TERROR doesn't trigger OF_AFRAID (but TMD_AFRAID does).

[AnonymousHero]

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

Why is this parsing code even using an explicit state machine? It seems like it would be a lot simpler and easier to understand (and maintain!) if it were a simple recursive descent parser? (It should obviously (ideally) have tests either way, but IME it's also a lot easier to tell at a glance if a recursive descent parser is actually correct.)

[Nick]

Why is this parsing code even using an explicit state machine? It seems like it would be a lot simpler and easier to understand (and maintain!) if it were a simple recursive descent parser? (It should obviously (ideally) have tests either way, but IME it's also a lot easier to tell at a glance if a recursive descent parser is actually correct.)

molybdenum wrote it, it works, and I've had plenty of other things to do

[PowerWyrm]

Hmm there's something that's bothering me about object_base.txt, even more when I look from my variant's perspective.

Before the commit that nuked tval_to_attr, the default attr for inventory display was put into a pref file (font-xxx.prf), which clearly could be changed by the player. All other info bits, especially breakage chance and object flags, are clearly game core info and should NOT be changed by the player.

For my variant's perspective, the font-xxx.prf info was parsed on the client and transmitted to the server, which then used that info to display items in inventory for each player. The object_base.txt info was parsed on the server and therefore was used for all players.

At this point I have two unsatisfactory options:
- parse object_base.txt on the server and set the same default attr for inventory display for all players
- parse object_base.txt on both client and server and only transfer the attr to the server

For now I'll implement option #2.

[PowerWyrm]

Nevermind. The txt files in /lib/edit are server-side files and should not contain any player configurable info. Therefore the only thing I can do is implement option #1 and set the same default attr for inventory display for all players.

[Carnivean]

- parse object_base.txt on the server and set the same default attr for inventory display for all players

I'm not 100% sure what you're talking about, but can't you have the server deal with the default, and then have the client side interpret the value from the server into whatever the player has configured? Or vice versa?

[PowerWyrm]

I'm not 100% sure what you're talking about, but can't you have the server deal with the default, and then have the client side interpret the value from the server into whatever the player has configured? Or vice versa?

No, because the txt files are tied to game core, not game preferences. You cannot let players choose core properties like object or monster flags, it would make no sense (player edits monster.txt, set Morgoth hp to 1...).

[fizzix]

I think it highlights a failure of the core-ui split where display information is encoded in the edit files alongside gameplay information. I guess for now your solution is best, parse the player information and only pass over the display stuff.

I'm not sure if splitting everything up into two separate files is the way to go. Then you would need to make edits in two places anytime you wanted to add or remove an object or item, which is a bit of a pain.