JHGuitarFreak / UQM-MegaMod

A fork of The Ur-Quan Masters + HD-mod that remasters the HD graphics with a veritable smorgasbord of extra features, options, QoL improvements, and much more...
https://uqm-mods.sourceforge.net
GNU General Public License v2.0
80 stars 22 forks source link

Kruzenshtern's Bugs: LIST of minor issues. #44

Closed Serosis closed 4 years ago

Serosis commented 4 years ago

A gentleperson named Kruzenshtern got in touch with me and sent me a lovely list of bugs that I'm entering in here so they can be tracked proper.

This is their twelfth bug, verbatim:

  1. ~Game resolution always set to maximum in HD and fullscreen enabled (probably intended)~
  2. ~IMO planets in interplanetary rotates way too fast (whet textured planets option is enabled). Just look at Jupiter.~
  3. ~Slight frame drops when textured planets option is enabled and SIS is close to the star.~
  4. ~Frame drops when we are at the planet orbit and ready to scan it. Even with all fancy options disabled. (Issue in MegaMod 0.8.0.84, debug version is fine for some reason, I didn’t fix anything relating to this).~

UPDATE: Edited to switch from unordered list to numbered list and to cross out bugs.

Kruzen commented 4 years ago

A couple of more small issues close to nitpicking:

5. ~Planet rotation slows down~

Steps to reproduce: 1) Set graphics to HD 2) Enter planet orbit and be ready to scan it 3) Rapidly scroll menu (press and hold down arrow key)

6. ~Leaving inner systems of some planets cause outer system to be drawn twice~

Steps to reproduce 1) Set transitions to smooth 2) Set rotating planets to false 3) Enter Betelgeuse system 4) Fly straight to inner system of planet III (just move forward without any turning) 5) Leave inner system

Caused by call of DrawSystemTransition (FALSE) in IP_Frame() and immediate call of ScaleSystem (newRadius) here on the next frame.

EDIT BY SEROSIS: Edited to add numbers to the bugs to make them easier to reference. ~And to cross out bugs.~

Kruzen commented 4 years ago

I withdraw fast rotating planets and frame drops issues. I had time dilation set to fast.

Serosis commented 4 years ago

There are still frame drops in Classic mode while in orbit. Been trying to find the source of it for awhile.

Kruzen commented 4 years ago

My bad. I meant frame drops in interplanetary. ⦁ IMO planets in interplanetary rotates way too fast (whet textured planets option is enabled). Just look at Jupiter. ⦁ Slight frame drops when textured planets option is enabled and SIS is close to the star. I withdraw these.

Kruzen commented 4 years ago

I kinda fixed Leaving inner systems of some planets cause outer system to be drawn twice. I modified DrawSystemTransition (BOOLEAN inner, SIZE Radius) by adding new parameter Radius and passing here newRadius from IP_Frame() when we are drawing outer system (and NULL everywhere else). Then I called here CheckShipLocation (&newRadius), and if Radius != newRadius - DON'T DRAW outer system. Let ScaleSystem (newRadius) handle it. The final code here looks like:

DrawSystemTransition (BOOLEAN inner, SIZE Radius)
{
    SIZE newRadius;

    SetTransitionSource (NULL);
    CheckShipLocation (&newRadius);
    if (inner)
    {
        BatchGraphics ();
        DrawInnerSystem ();
        RedrawQueue (FALSE);
        if (optIPScaler == OPT_3DO)
            ScreenTransition (3, NULL);
        UnbatchGraphics ();
    }
    else if (Radius == newRadius)
    {
        BatchGraphics ();
        DrawOuterSystem ();
        RedrawQueue (FALSE);
        if (optIPScaler == OPT_3DO)
            ScreenTransition (3, NULL);
        UnbatchGraphics ();
    }
}
Serosis commented 4 years ago

Implemented your fix with commit 89a95606c41d6a5a2844c665b8a3698686ec3388

Serosis commented 4 years ago

Reverted issue 6 with commit f004d1de54d1d851b6fb2aee70092da4ec2b795a

Kruzen commented 4 years ago

Ok, so basically it is a problem with planets that are on the edge of OuterSystemScaling. For example - leaving inner and immediately starting system scaling (Betelgeuse III) and scaling system while entering inner (Alpha Centauri I from the system geometric center). I've come up with a couple solutions using boolean variables that are global for solarsys.c. 1) Add boolean variable lockTransition which is false by default. Then use it in function below:

DrawSystemTransition (BOOLEAN inner)
{
        if(optIPScaler == OPT_3DO)// bad for stepped screen, not an issue in the first place.
        lockTransition = true;
    SetTransitionSource (NULL);
    BatchGraphics ();
    if (inner)
        DrawInnerSystem ();
    else
        DrawOuterSystem (); 
    RedrawQueue (FALSE);
        ScreenTransition (3, NULL, optIPScaler == OPT_3DO);
    lockTransition = false;
    UnbatchGraphics ();
}

And finally change this piece of code in IP_Frame ():

if (locChange && !lockTransition)
        {
        if (playerInInnerSystem ()) 
               {    // Entering inner system
            DrawSystemTransition (TRUE);
           } 
               else if (pSolarSysState->SunDesc[0].radius == newRadius)
               {        // Leaving inner system
            DrawSystemTransition (FALSE);
           } 
               else 
               {    // Zooming outer system
            ScaleSystem (newRadius);
           }
    }

This will lock any other transitions or scalings while one is occuring.

Kruzen commented 4 years ago

2) Add new function:

static BOOLEAN
CheckScreenChange (void)
{   
    if (!playerInInnerSystem ()
            && pointWithinRect (scaleRect, GLOBAL (ShipStamp.origin)))
        return TRUE;
    return FALSE;
}

This function return true if conditions for system scaling are met. Then we use it in DrawOuterSystem ():

DrawOuterSystem (void)
{
    ValidateOrbits ();  
    if (!CheckScreenChange ())
    {
        DrawSystem (pSolarSysState->SunDesc[0].radius, FALSE);
        if (optOrbitingPlanets || optTexturedPlanets)
            DrawOuterPlanets(pSolarSysState->SunDesc[0].radius);
        DrawHyperCoords (CurStarDescPtr->star_pt);
    }
    else
    {
        lockTransition = true;
        IP_frame();
    }
    IP_frame();
}

CheckScreenChange () will return TRUE only if caller was DrawSystemTransition () since ScaleSystem () already recalculated SIS position at this point by XFormIPLoc (&GLOBAL (ip_location), &GLOBAL (ShipStamp.origin), TRUE). So, if we are leaving inner system and ready to scale it, then don't draw anything. And also turn off scaler in DrawSystemTransition ():

DrawSystemTransition (BOOLEAN inner)
{
    SetTransitionSource (NULL);
    BatchGraphics ();
    if (inner)
        DrawInnerSystem ();
    else
        DrawOuterSystem (); 
    RedrawQueue (FALSE);
    if (lockTransition == false)
        ScreenTransition (3, NULL, optIPScaler == OPT_3DO);
    lockTransition = false;
    UnbatchGraphics ();
}

And finally modify ScaleSystem () so Star coordinates also fades in while we leave inner system:

BatchGraphics ();
DrawOuterSystem ();
if (lockTransition)
    ScreenTransition (3, NULL, optIPScaler == OPT_3DO);
else
    ScreenTransition (3, &r, optIPScaler == OPT_3DO);
UnbatchGraphics ();
Kruzen commented 4 years ago

That left us with scaling system while entering inner (Alpha Centauri I). And I sadly can't find simple solution. It gets overcomplicated. So it is solution 1 in reverse. But we are ignoring collision for 2 frames while scale transition is occuring by using second boolean (false by defaul):

if (optIPScaler == OPT_3DO)// same reason, on stepped eyes can hurt
    lockColl = true;
GetContextClipRect (&r);
SetTransitionSource (&r);
BatchGraphics ();
DrawOuterSystem ();
if (lockTransition)
    ScreenTransition (3, NULL, optIPScaler == OPT_3DO);
else
    ScreenTransition (3, &r, optIPScaler == OPT_3DO);
UnbatchGraphics ();
lockColl = false;

Then modify CheckShipLocation (SIZE *newRadius) part where we are looking for planet collision:

if (GLOBAL (autopilot.x) == ~0 && GLOBAL (autopilot.y) == ~0 && !lockColl)// here
    {   // Not on autopilot -- may collide with a planet
        PLANET_DESC *planet = CheckIntersect ();
        if (planet)
        {...

So we get an effect of an auto-pilot collision ignore for 2 frames. So transition into inner won't start instantly. The final thing is to modify DrawOuterSystem () again so SIS Title will be correct on stepped transitions (it displays star coords instead of Planet I).

DrawOuterSystem (void)
{
    ValidateOrbits ();  
    if (!CheckScreenChange ())
    {
        //lockColl = true;
        DrawSystem (pSolarSysState->SunDesc[0].radius, FALSE);
        if (optOrbitingPlanets || optTexturedPlanets)
            DrawOuterPlanets(pSolarSysState->SunDesc[0].radius);
    }
    else
    {
        lockTransition = true;
        IP_frame();// from DrawSystem ()
    }
    IP_frame();
    if (!playerInInnerSystem ())
        DrawHyperCoords (CurStarDescPtr->star_pt);//move it down to make sure we didn't get into inner 2 frames before
}
Kruzen commented 4 years ago

First issue is actually not reproduced on 0.8.0.85. You can close it.