[airstrike] bug in timing code

[Eero Tamminen]

Hi,

On Friday 02 September 2005 11:47, Erik Auerswald wrote:
> > > The comments should be updated and IMHO delays should be specified in
> > > the same time unit in the whole code. What unit to use (game frames,
> > > 100ms, 10ms or something else) should be discussed and the result
> > > documented.
> >
> > The documentation should be correct & consistent, but don't see it
> > necessarily a problem if core uses frame based and game engine uses
> > time based timings.
>
> Well, you're right that there is no need to change the frame based code
> (in fact I had not realized that the game physics are frame based).
>
> > It's just that the FRAMERATE should be handled only in one place in the
> > engine/, in schedule.c, currently it's not:
> > -----------
> > $ grep FRAMERATE */*.c
> > core/display.c:    .frame_time = FRAMERATE,
> > engine/schedule.c:      *timer = now + delay * FRAMERATE / 10;
> > engine/sprite.c:
> > schedule(delay*FRAMERATE/10,SCHED_NORMAL,send_alarm,msg,target);
> > tests/testengine.c:  for (idx = 0; idx < 2*FRAMERATE; idx++) {
> > ------------
> >
> > Rest of the code, the game objects (AIs, sprites etc), should then use
> > the schedule functions and whatever time base they have.
>
> Exactly. I'd say
> - move the #define FRAMERATE 30 to schedule.h
> - write a frame_time() function and use it in display.c:

Display is in core which doesn't know about engine, engine knows about core.
I.e. everything display.c uses, should be in core (or SDL or libc).

>   .frame_time = frame_time();
> - write a delay_to_frames() function =>
>   *time = now + delay_to_frames(delay);
>   schedule(delay_to_frames(delay), ...);
>
> I did not work out what the use of FRAMERATE in testengine.c shall
> accomplish, so I don't know how to change this (if at all).

> > tests/testengine.c:  for (idx = 0; idx < 2*FRAMERATE; idx++) {

This drivers the engine "mainloop" for two seconds after showing
the level end message (after which level switches). If it would just
SDL_Delay, the game would freeze while the level end message is shown.


> > Hm.  Why you've changed the delay values so much (100 to 900 instead of
> > 1000 etc.) and to wierd values?
> >
> > For example:
> > -       sprite_alarm(120, b, msg_kill());
> > +       sprite_alarm(1080, b, msg_kill());
> >
> > When seeing a value like 1080 in a code, I would immediately wonder
> > about the 80 part, is that somehow significant?  :-)
>
> I changed the existing values to the new representation. The current
> uses a time base of 90ms,

1/10sec = 100ms.  Where did you get the 90ms value?

> my patch uses a time base of 10ms -> all 
> values are multiplied by 9 (see my old mail for an explanation of this).
>
> My suggested changes can be seperated in 2 steps:
>
> 1) The changes suggested above to make the timing code consistent (incl.
>    updating the comments) with a time base of 100ms (what seems to be
>    the intended time base at the moment, i.e. delay values would not be
>    changed, actual timing would be about 10% slower).
> 2) Changing the time base to 10ms, adjusting the comments and the
>    values.
>
> Step 1) does not seem to be controversial, I do not insist on step 2),
> but I would prefer it this way. If no one objects I will implement step
> 1) during the weekend.


	- Eero