Bug fix: Timed do-function recreating timer

Posted by David Haley on Mon 08 Aug 2005 08:11 PM — 7 posts, 29,399 views.

David Haley USA Mon 08 Aug 2005 08:11 PM #0

Hi all,

While debugging Samryn's problem - see http://www.gammon.com.au/forum/bbshowpost.php?bbsubject_id=5776&page=999999 - we came across an apparent problem in SMAUG. It would seem that a timed do-function cannot recreate a do-function timer. This means that you cannot have a do-function timer "beat" at regular intervals.

The fix is really quite simple. I've included my reply to Samryn with problem diagnosis + a solution that he said fixes his problem.

Maybe this'll help somebody out there, and if it's actually a reliable fix, maybe should go up for inclusion in SMAUGFUSS.

(Disclaimer: I haven't tested this myself and don't know if it breaks something that depends on this somewhat odd behavior of SMAUG's)

Quote:
I think I found the source of your problem. On my copy of SMAUG - which, admittedly, is based off of SMAUG 1.0 - I have this in the 'violence_update' function:

        for ( timer = ch->first_timer; timer; timer = timer_next )
        {
            timer_next = timer->next;
            if ( --timer->count <= 0 )
            {
                if ( timer->type == TIMER_DO_FUN )
                {
                    int tempsub;

                    tempsub = ch->substate;
                    ch->substate = timer->value;
                    (timer->do_fun)( ch, "" );
                    if ( char_died(ch) )
                        break;
                    ch->substate = tempsub; 
                }
                extract_timer( ch, timer );
            }
        }

What does this mean? Well, it means that after executing a timer, it is removed. However, if we look at the 'add_timer' function...

void add_timer( CHAR_DATA *ch, sh_int type, sh_int count, DO_FUN *fun, int value )
{
    TIMER *timer;
    
    for ( timer = ch->first_timer; timer; timer = timer->next )
    {
        if ( timer->type == type )
        {
           timer->count  = count;
           timer->do_fun = fun;
           timer->value  = value;
           break;
        }
    }

    if ( !timer )
    {
        CREATE( timer, TIMER, 1 );
        timer->count    = count;
        timer->type = type;
        timer->do_fun   = fun;
        timer->value    = value;
        LINK( timer, ch->first_timer, ch->last_timer, next, prev );
    }
}

Here, we see that when we add a timer, we first loop through the existing timers, seeing if we already have one of that type.

In your case, we do have a timer, because we're in the middle of executing one, and we haven't removed it yet. So, we'll edit it, and then we'll remove it right after running it!

To fix this, we'll have to make sure that the timer is not extracted in violence_update if it has count left. This should be as simple as adding:

if ( timer->count == 0 )

before the 'extract_timer' call.

EDIT: it would probably be safer to have the ifcheck read <=, not ==...

Samryn United Kingdom Tue 09 Aug 2005 12:03 AM #1

seeing as i've already done it, thought i'd tell you how its doing...

There doesn't seem to be any leaks or any issues with it, its working great and im sure everyone can thing of a few things they can do now just using add_timer instead having to go the long way round and add it all the update.c etc...

Anyway just thought i'd say its something that should be in everyones code from now on!

Thanks again for helping me out.

-- Samryn

Gohan_TheDragonball USA Wed 10 Aug 2005 03:49 AM #2

actually, you can have a function beat at regular intervals:


do_function () {
 switch ( ch->substate ) {
  default: // first use of the function
   add_timer( ch, TIMER_DO_FUN, 2, __FUNCTION__, 1 );
   break;
  case 1:
   // first loop
   add_timer( ch, TIMER_DO_FUN, 2, __FUNCTION__, 2 );
  case 2:
   // second loop
   add_timer( ch, TIMER_DO_FUN, 2, __FUNCTION__, 1 );
  case SUB_TIMER_DO_ABORT:
   // they typed something!!! run
   return;
 }
}

that is the most basic way to loop a function, I use it for stuff like meditation.

David Haley USA Wed 10 Aug 2005 07:07 AM #3

That will not work without the bugfix (or an alternate fix, of course). It is possible that your code has a fix already. But if you look at the code I posted - which, again, was from an older version of SMAUG but seemed to fix Samryn's problem - adding a timer when acting on a timer doesn't do anything.

Gohan_TheDragonball USA Wed 10 Aug 2005 10:49 PM #4

yeah, i guess i fixed this a long time ago. lol, i did it a different way, don't know if its worse or better than what you did.


                if ( timer->type == TIMER_DO_FUN )
                {
                    int tempsub;

                    tempsub = ch->substate;
                    ch->substate = timer->value;
                    (timer->do_fun)( ch, "" );
                    if ( char_died(ch) )
                        break;
                    if (ch->substate != timer->value)
                        repeat = TRUE;
                    ch->substate = tempsub;
                }

                if (!repeat)
                    extract_timer( ch, timer );

David Haley USA Wed 10 Aug 2005 11:14 PM #5

Your solution works in that it will let you loop commands, but what I propose is better (more general) in that it will allow timers to repeat even if they send the character into the same substate. I'm not sure why you'd ever want to prevent a timer from explicitly repeating itself, even if it goes into the same substate.

Gohan_TheDragonball USA Wed 10 Aug 2005 11:30 PM #6

Yeah, i must say i do like your idea better. I think this was a fix posted on a forum a long time ago. Truthfully don't think i would have said to myself "hmm, i think i can fix this by going into different substates", and i say this because i am not that imaginative. Plus it will cut down on code, not having to repeat everything twice to loop it.