Difference between revisions of "Talk:Demon speed bug"

From DoomWiki.org

m (addendum)
 
Line 68: Line 68:
  
 
Well, now only if somebody would write a nice article... --[[User:Unmaker|Unmaker]] 16:10, 12 June 2012 (UTC)
 
Well, now only if somebody would write a nice article... --[[User:Unmaker|Unmaker]] 16:10, 12 June 2012 (UTC)
 +
 +
:So,
 +
'''if (state >= S_SARG_RUN1 && state <= S_SARG_PAIN2 && (fastparm || gameskill == sk_nightmare)) mobj->tics >>= 1;'''
 +
:would do the trick perfectly (even keeping DeHackEd compatibility). The Boom fix works as well, but conceptually still alters the state definition table, which doesn't seem like a good idea to me. (Though on the plus side, the Boom code makes sure not to halve length if it would zero it.)
 +
:Another way the bug can be nullified (maybe the simplest way, actually) is by having G_DoLoadGame() store the read skill level into d_skill instead of gameskill. That would force the game to adapt as required the state lengths when loading a game, since the bug happens because this is not done.

Latest revision as of 13:04, 12 June 2012

That's one pretty odd bug. Methinks the offending code is around here somewhere:
(from g_game.c:1422)

   if (fastparm || (skill == sk_nightmare && gameskill != sk_nightmare) )
   { 
       for (i=S_SARG_RUN1 ; i<=S_SARG_PAIN2 ; i++) 
           states[i].tics >>= 1; 
       mobjinfo[MT_BRUISERSHOT].speed = 20*FRACUNIT; 
       mobjinfo[MT_HEADSHOT].speed = 20*FRACUNIT; 
       mobjinfo[MT_TROOPSHOT].speed = 20*FRACUNIT; 
   } 
   else if (skill != sk_nightmare && gameskill == sk_nightmare) 
   { 
       for (i=S_SARG_RUN1 ; i<=S_SARG_PAIN2 ; i++) 
           states[i].tics <<= 1; 
       mobjinfo[MT_BRUISERSHOT].speed = 15*FRACUNIT; 
       mobjinfo[MT_HEADSHOT].speed = 10*FRACUNIT; 
       mobjinfo[MT_TROOPSHOT].speed = 10*FRACUNIT; 
   }

With my limited knowledge of programming, I comprehend that demon's frame lengths here are being bitshifted by one (that is, to multiply or divide them by two), while speeds of various fireballs are being explicitly redefined.
The bolded code is what confuses me. In the video, the player begins a game in Nightmare, makes a save, then begins a HMP game, and loads from saved slot, then repeats this again and again. My guess is, while gameskill is being read from a save, skill variable remains unchanged from previous session. The bolded check passes every time, and the demon continually slows down. I looked into save/load logic, but there are too many unfamiliar thingies in the code :)
A reverse process is similar. You save the game on skills 1-4, then start in NM and load your previous save. The demon's on crank :) But why it would cause the game to freeze?
What amuses me the most is how such an ugly bug had persisted through all versions. Weird. --Unmaker 00:39, 12 June 2012 (UTC)

Why would it end up freezing? Simple. Those values are integers being shifted away. At a moment, the speed will end up being 1. Then the next step, it'll become 0. Once the demon's states last 0 tics each, its behavior becomes an infinite loop, as it acts an infinite amount of time in the same tic.
As for the other skill-vs.-gameskill issue, I'm not sure. Skill is the first parameter of the function in which that code is found, G_InitNew(skill_t skill, int episode, int map). A few lines after that code, you see gameskill = skill. However, when loading a save, G_DoLoadGame (void) starts by setting gameskill to what's read and then calls G_InitNew() with it. So, when loading a savegame, this code is not triggered since skill and gameskill are equal. The speeds are therefore not changed.
The adjustments to the tic length that causes the demon to glitch therefore must happen when starting a new game. Starting a game from the menu sets a different value (d_skill) and ends up calling G_InitNew with that d_skill, so gameskill remains at the old value. So, loading a nightmare save sets gameskill to nightmare. Then start a new game not in nightmare: demon states length are doubled. Load the save again, demon state length aren't changed. Repeat as many times as you want.
Inversely, save a non-nightmare game. Start a new nightmare game, lengths are halved. Load the save, lengths aren't changed. Repeat as desired until the game freezes because of the infinite amount of 0-tic actions that the demon will go through.
Note that the reason why the demon has its state length doubled or halved while the other mobjs have their speed set to a given value is that it's not the same thing at all. For the projectile, the speed is increased so that they will move faster, but their states' lengths aren't changed (the animation of a fireball will stay at the same speed). For the demon, the movement speed isn't changed, but the length of each state is halved (this does result in faster movement since it moves twice as often as before, but an individual move is still the same distance).
A bug-free approach would be not to directly change the speed of the demon actor right in its definition, but to have that done in, for example, P_SetMobjState (mobj_t* mobj, statenum_t state):
	st = &states[state];
	mobj->state = st;
	mobj->tics = st->tics;
	if (mobj->type == MT_SERGEANT && (fastparm || gameskill == sk_nightmare)) mobj->tics >>= 1;
	mobj->sprite = st->sprite;
	mobj->frame = st->frame;
I bolded the line to add. This would result in the same thing (state durations are halved for the demon in nightmare and -fast) without overwriting the state table. --Gez 10:47, 12 June 2012 (UTC)


Thanks for answering. Your thoughts seem fairly reasonable. I wonder if this bug can be nullified if you end the game properly using "end current game" (or whatever it's named there). As for your solution, you might not have noticed that not all states are being changed, namely death and raise lenghts remain constant.

Here, how this is handled in Boom (from Boom source, g_game.c)

G_SetFastParms(fastparm || skill == sk_nightmare);  // killough 4/10/98
void G_SetFastParms(int fast_pending)
{
  static int fast = 0;            // remembers fast state
  int i;
  if (fast != fast_pending) {     /* only change if necessary */
    if ((fast = fast_pending))
      {
        for (i=S_SARG_RUN1; i<=S_SARG_PAIN2; i++)
          if (states[i].tics != 1 || demo_compatibility) // killough 4/10/98
            states[i].tics >>= 1;  // don't change 1->0 since it causes cycles
        mobjinfo[MT_BRUISERSHOT].speed = 20*FRACUNIT;
        mobjinfo[MT_HEADSHOT].speed = 20*FRACUNIT;
        mobjinfo[MT_TROOPSHOT].speed = 20*FRACUNIT;
      }
    else
      {
        for (i=S_SARG_RUN1; i<=S_SARG_PAIN2; i++)
          states[i].tics <<= 1;
        mobjinfo[MT_BRUISERSHOT].speed = 15*FRACUNIT;
        mobjinfo[MT_HEADSHOT].speed = 10*FRACUNIT;
        mobjinfo[MT_TROOPSHOT].speed = 10*FRACUNIT;
      }
  }
}

Well, now only if somebody would write a nice article... --Unmaker 16:10, 12 June 2012 (UTC)

So,
	if (state >= S_SARG_RUN1 && state <= S_SARG_PAIN2 && (fastparm || gameskill == sk_nightmare)) mobj->tics >>= 1;
would do the trick perfectly (even keeping DeHackEd compatibility). The Boom fix works as well, but conceptually still alters the state definition table, which doesn't seem like a good idea to me. (Though on the plus side, the Boom code makes sure not to halve length if it would zero it.)
Another way the bug can be nullified (maybe the simplest way, actually) is by having G_DoLoadGame() store the read skill level into d_skill instead of gameskill. That would force the game to adapt as required the state lengths when loading a game, since the bug happens because this is not done.