The times you felt you had to go and redo a game mechanic due to quality concerns

I know this is likely a regular scenario in the world of game development, and we, those who develop using the BGE would be no different in encountering these scenarios. You create a game mechanic, then try to enhance it, then due to concerns you might not be able to get it to work right if you tried altering what you got, you decided to scrub it and start over with a fresh overview and new ideas.

I had this situation recently with one of my games where you have to drive a car along various courses to the end concerning what I had set up for the camera to avoid as much clipping as possible, I thought at the time I would easily be able to achieve this camera system with what I had in my script, but then months later after doing other game projects that involved python I looked at the code again (posted below)


#--------Variables that are constantly set for camera related logic--------#
#only set RD1-4 variables when ray sensors are positive#
if DPX.positive==True:
    RD1 = ob.getDistanceTo(HP1)
else:
    RD1=30

if DPZ.positive==True:
    RD2 = ob.getDistanceTo(HP2)
else:
    RD2=30

if DNZ.positive==True:
    RD3 = ob.getDistanceTo(HP3)
else:
    RD3=30
    
if DNX.positive==True:
    RD4 = ob.getDistanceTo(HP1)
else:
    RD4=30

#set Rspectrum variable#
ob["RSpectrum"] = RD1+RD2+RD3+RD4/4
    
    
#---camera controls pre-curser, set variables important for the camera system---#
#move the camera itself under certain conditions#
if ob["C"]==0:
    if ob["init2"] == 1 and Surface.positive==True and CamX.positive==False and CamXN.positive==False:
        if Left.positive==True or Right.positive==True:
            if ob["Limit"] < ob["Lmax"]:
                ob["Limit"] += 0.004
            
            if ob["Lfactor"] < ob["LFmax"]:
                ob["Lfactor"] += 0.0125
        else:
            if ob["Lfactor"] > 0:
                ob["Lfactor"] -= 0.0004
        
            if ob["Limit"] > 0:
                ob["Limit"] -= 0.00125
            
        if VxL >= -5:
            Cam_move.dLoc=[RzG/2.25*ob["Lfactor"]*ob["Limit"],0,0]
            Cam_move.useLocalDLoc=True
            c.activate(Cam_move)
        elif VxL < -5 and VxL >= -35:
            Cam_move.dLoc=[RzG/1.6*ob["Lfactor"]*ob["Limit"],0,0]
            Cam_move.useLocalDLoc=True
            c.activate(Cam_move)
        elif VxL < -35:
            Cam_move.dLoc=[RzG/1.3*ob["Lfactor"]*ob["Limit"],0,0]
            Cam_move.useLocalDLoc=True
            c.activate(Cam_move)
        
    elif CamX.positive==True or CamXN.positive==True: #if the camera is about go through a vertical wall--
        
        if CamX.positive==True and CamXN.positive==False:
            Cam_move.dLoc=[1.5*ob["Lfactor2"]*ob["Limit2"],0,0]
            Cam_move.useLocalDLoc=True
            c.activate(Cam_move)
        if CamXN.positive==True and CamX.positive==False:
            Cam_move.dLoc=[-1.5*ob["Lfactor2"]*ob["Limit2"],0,0]
            Cam_move.useLocalDLoc=True
            c.activate(Cam_move)

    #set Dlength#
    if VxL < -2 and Fsight.positive==False and VzG >= -1 and ob["RD_G"] < 3:
        ob["Dlength"]+= 0.18
    else:
        ob["Dlength"]=0
    
    if DNY.positive==True:
        ob["RD_G"] = ob.getDistanceTo(HP_G)
    else:
        ob["RD_G"]=15
    
    #set Flength to 0 when Dlength is the correct value#
    if ob["Dlength"] > 20:
        ob["Flength"] = 0
    
    if CamX.positive==False and CamXN.positive==False:
        ob["Lfactor2"] = 0
        ob["Limit2"] = 0
    else:
        if ob["Limit2"] < 1:
            ob["Limit2"] += 0.004
            
        if ob["Lfactor2"] < 1:
            ob["Lfactor2"] += 0.0125
            
if Carcam.positive==True:
    if ob["C"]==0:
        ob["C"]=1
    if ob["C"]==2:
        ob["C"] = -1

    
#-----------------------------------CAMERA CONTROLS------------------------------------------#

#default position with no obstructions#
if ob["C"]>=1:
    if ob["C"]==1:
        c.deactivate(Cam)
        ob.visible=False
        Camera.position=(px, py-2.1, pz+2.3)
        Camera.orientation = OrCam.orientation
        Camera.setParent(ob, 0, 1)
        ob["C"]=2
if ob["C"]<=0:
    if ob["C"]== -1:
        Camera.removeParent()
        ob.visible=True
        ob["C"]=0

    if DPX.positive==False and DNZ.positive==False and DPZ.positive==False and DNX.positive==False:
        Cam.max=26
        Cam.min=26
        Cam.height=7
        Cam.useXY=False
        c.activate(Cam)
        ob["KeyMod"]=0
        ob["Cooldown"]=1
        ob["Lmax"]=1
        ob["LFmax"]=1

    #script for camera when on slopes, near obstructions, ect...#       
    if DPX.positive==True or DNZ.positive==True or DPZ.positive==True or DNX.positive==True:
    
        #math to set the RSpectrum variable, and declare most of the large variable assortment for the camera values#
        Divide_H = ob["RSpectrum"]/100
        Divide_L = ob["RSpectrum"]/100
        Hfactor = ob["RSpectrum"]*Divide_H
        Dfactor = ob["RSpectrum"]*Divide_L
        Sfactor = VzG*-0.5
        Cfactor = -40+ob["RSpectrum"]*Divide_H/2.6
        Divide_Fac=ob["CSmod"]/20
        Afactor = ob["RSpectrum"]/29
        CSmod2 = ob["RD_G"]*4
    
        if VzG < 0.3 and VzG > -0.3:
            ob["CSmod3"] = ob["RD_G"]-2.3783
        else:
            ob["CSmod3"] = ob["RD_G"]*4
    
        #Avoid un-neccesary application of anti-clipping system for ramps by checking various conditions for Tlength#
        if Left.positive==True or Right.positive==True:
            if VzG < -0.4:
                ob["Tlength"]+=1
        
        elif VzG > -0.5 and ob["Tlength"] > 10 and VzG < 0.5:
            ob["Tlength"]+=1
        
        elif Fsight.positive==True and ob["Tlength"] > 40 and VzG > -0.5 and VzG < 0.5:
            ob["Tlength"]+=1
        else:
            ob["Tlength"]=0
    
        #Use all these variables to set KeyMod, which influences camera height, to make the camera more monkey proof#
        if ob["Dlength"] < 20:
            if VzG < -1 and ob["Flength"] < 1:
                ob["Flength"] = 1
        
            if Left.positive==True or Right.positive==True:
                if ob["Tlength"] > 50:
                    ob["Flength"] = 2
        
            if Foward.positive==True and VzG > 1 and ob["Flength"]>=2:
                ob["Flength"]+=1
                if ob["Flength"] >= 5 and ob["RD_G"] <= 3: #monkey proof shallower slopes--
                    ob["Cooldown"]+=0.3/CSmod2
                    ob["KeyMod"]=CSmod2*3/ob["Cooldown"]
                elif ob["Flength"] >= 5 and ob["RD_G"] > 3: #monkey proof steeper slopes--
                    ob["Cooldown"]+=0.3/CSmod2/2.6
                    ob["KeyMod"]=CSmod2*3/ob["Cooldown"]
            else:
                ob["KeyMod"]=0
            
                if ob["Flength"] < 10:
                    ob["Cooldown"]=1

        #Limit Rspectrum to 400#
        if ob["RSpectrum"] >= 400:
            ob["RSpectrum"] = 385
    
        #use the ray pointing down to boost the camera up slightly, to avoid clipping through the track in most remaining cases#
        if ob["RD_G"] >= 3:
            ob["CSmod"]=ob["RD_G"]*8
        else:
            ob["CSmod"]=1
    
        #Create limit for Cslide variable to avoid too high a multiplier for Camera height#
        if Cfactor >= 5:
            ob["Cslide"] = 5-ob["RSpectrum"]/10
        else:
            ob["Cslide"] = Cfactor
    
        #only use these camera values when Rspectrum or VzG is below or above a certain value, simple math is used to produce the sliding# 
        #values#
        if ob["RSpectrum"] <= 160: #spectrum band below 160
            if ob["RSpectrum"] > 67:
                ob["Lmax"]=0.25
                ob["LFmax"]=0.25
            
            if VzG < 1 or ob["RSpectrum"] < 67: #VzG < 1 or spectrum band below 67
                ob["Lmax"]=0.1
                ob["LFmax"]=0.1
            
                if VzG > 0.5:
                    Cam.max=26
                    Cam.min=26
                else:
                    if VzG < -0.3:
                        Cam.max=26+ob["RSpectrum"]/Dfactor*10
                        Cam.min=26+ob["RSpectrum"]/Dfactor*10
                    Cam.height=ob["RSpectrum"]/Hfactor-ob["Cslide"]+ob["CSmod"]/2.4+ob["KeyMod"]
            else:
                if VzG > -0.3:
                    Cam.max=26
                    Cam.min=26
                else:
                    Cam.max=26+ob["RSpectrum"]/Dfactor*10
                    Cam.min=26+ob["RSpectrum"]/Dfactor*10
                Cam.height=ob["RSpectrum"]/Hfactor*Sfactor+6-Divide_Fac+ob["KeyMod"]
            
        #for the spectrum band above 200, both cases testing for positive or negative Z-velocity#
        elif ob["RSpectrum"] > 200 and VzG < 1.5:
            Cam.max=26+ob["RSpectrum"]/Dfactor
            Cam.min=26+ob["RSpectrum"]/Dfactor
            Cam.height=ob["RSpectrum"]/Hfactor*Sfactor+Afactor/2.5+ob["CSmod3"]+ob["KeyMod"]
        
        elif ob["RSpectrum"] > 200 and VzG > 1.5:
            Cam.max=26+ob["RSpectrum"]/Dfactor*10
            Cam.min=26+ob["RSpectrum"]/Dfactor*10
            Cam.height=ob["RSpectrum"]/Hfactor-ob["CSmod"]/4+ob["KeyMod"]
        else:
            Cam.max=26+ob["RSpectrum"]/Dfactor
            Cam.min=26+ob["RSpectrum"]/Dfactor
            Cam.height=ob["RSpectrum"]/Hfactor*Sfactor+Afactor-ob["RD_G"]+ob["KeyMod"]
        Cam.useXY=True
        c.activate(Cam)

see below…

Continued, the forum software said my post was too long, so I have to split it.

The whole thing was a monster of a piece of code I wrote just to get the camera working, and I thought that the whole thing has become big, unwieldy, too many hardcoded values, so many modulating values that are similar I forgot how every variable contributed, and too much code in general. On top of that, the last part wasn’t very workable and the code was made worse by the fact I didn’t know how to get BGE python to output a proper average distance from the length of various rays from various sensors. On top of that it made driving a track at times akin to holding a camera in an earthquake, quite jerky in many areas.

I started back on it by trying to whittle it down bit by bit, first by changing one of the main properties to be a proper average, then trying to make it less dependent on hard-coded numbers, trying to reduce the jerkiness by (another convoluted), piece of code to interpolate it out, but then I realized that I wouldn’t be able to fix this, ripped all the old code out leaving only the part that gets the length of the rays.

In the last week or so I worked off and on to create a new camera system that used less code and had more elegant movement, no jerkiness, better commented, and more extensible to cover various cases utilizing new ideas on how to tackle various scenarios during the levels, so far it does nearly everything I planned for the camera system to do and with no jerks from conflicting pieces of code trying to move the camera in different directions. I’d say it was worth getting the ugly pile of code posted above out of the game for good.

Now then, have you had situations like this, how does the old code for that mechanic you rewrote compare with the new code, what did you learn? For those mainly using logic bricks, how does the old setup compare with the new one?

Redoing something will always improve it. The only thing is is it better to redo entirely, or just edit the existing code.
Well, it all comes down to two things:

  • Time
  • Quality

If you completely redo the code it takes a lot of time, but you end up with much higher quality code that is more readable and runs faster. Whereas if you just edit the old code it is a lot quicker to smother the movement, but it is harder to read and understand, and probably far less efficient.

So there is no right or wrong, it just depends on how accurate you need the finished code to be, and how much time you want to spend.

(oh, and I would like to see the finished code if you are willing to share…)

@Ace - That definitely looks unwieldy. I’m glad that you got proper code. I definitely have had situations in which I implement a new feature or fix an old method of coding.

This really depends on the individual situation. If the situation changes the code might need to change as well.

You will not necessarily get better code, but you learnt from the old code. This gives you a very good chance to result in an improvement.
If you find your new code is worse than the old one, you would most-likely remove it and jump back ;).

There are some development schemes dealing with this:

  • re-factor
  • test driven development
  • (maybe more)

They assume you do not want to change the interface but the implementation. That means you can change part of the system rather than to touch everything :cool:.

It may or may not be ‘finished’ as in nothing else will be added to it, but it does about everything the old code was supposed to do, but does it without the bugs and jerks, there’s also some information variables like a ray cast from the camera to the car that isn’t in this part. Several variables from the old version were kept, because they’re either now done properly or they just provided scene information that was useful.


#--------Variables that are constantly set for camera related logic--------#
#only set RD1-4 variables when ray sensors are positive#
#the tClear variable is also reset here depending on whicb ray is the shortest at the time, more in the next area#

if DPX.positive==True:
    RD1 = ob.getDistanceTo(HP1)
else:
    if ob['tClear'] == 2 and ob['Short'] == 1: #wait until the car is away from a slope to avoid clipping--
        ob['tClear']=0
    RD1=25

if DPY.positive==True:
    RD2 = ob.getDistanceTo(HP2)
else:
    if ob['tClear'] == 2 and ob['Short'] == 2:
        ob['tClear']=0
    RD2=25

if DNY.positive==True:
    RD3 = ob.getDistanceTo(HP3)
else:
    if ob['tClear'] == 2 and ob['Short'] == 3:
        ob['tClear']=0
    RD3=25

if DNX.positive==True:
    RD4 = ob.getDistanceTo(HP4)
else:
    if ob['tClear'] == 2 and ob['Short'] == 4:
        ob['tClear']=0
    RD4=25

if trackRay.positive==True:
    gDist = ob.getDistanceTo(HP_G)
else:
    gDist = 0

if CamX.positive == True:
    CPhit = Camera.getDistanceTo(CPX)
else:
    CPhit = 3.5

if CamXN.positive == True:
    CNhit = Camera.getDistanceTo(CNX)
else:
    CNhit = 3.5

#find the shortest ray, the resultant value of 'Short' is then used to determine which sensor needs to go false--
#to reset tClear and unlock the camera height in the code above, this avoids clipping even if you're turning--
#before you get to the bottom of the slope#
if RD1 < RD2 and RD1 < RD3 and RD1 < RD4:
    ob['Short'] = 1
elif RD2 < RD1 and RD2 < RD3 and RD2 < RD4:
    ob['Short'] = 2
elif RD3 < RD1 and RD3 < RD1 and RD3 < RD4:
    ob['Short'] = 3
else:
    ob['Short'] = 4

#create an average to interpolate between values, this will prevent sudden jerks#
RayTotal = RD1+RD2+RD3+RD4

length = len(GameLogic.init)

ob['RSpectrum'] = RayTotal/4

#alternative condition to reset tClear, reset it in this case even if the sensor
#needing to go false is still true#
if ob['RSpectrum'] == 30 and ob['tClear'] == 2:
    ob['tClear'] = 0

#use the speed to determine height modulator#
if VzG > 5:
    if ob['tClear'] == 1:
        ob['Carry'] = ob['gMult']
        ob['tClear'] = 2
    ob['gMult'] = gDist-2*4
    
elif VzG < -5:
    ob['gMult'] = gDist-2*-4
    if ob['tClear'] == 0:
        ob['tClear'] = 1
    
else:
    if ob['tClear'] == 1:
        ob['Carry'] = ob['gMult']
        ob['tClear'] = 2
    ob['gMult'] = 0

#move the camera forward if a line between the car and the camera is broken#
if not 'None' in str(carDetect):
    ob['zMove'] -= 0.2
else:
    ob['zMove'] = 0

#-----------------------------------CAMERA CONTROLS------------------------------------------#
#perform regular camera controls when the camera isn't near a wall#
if CPhit > 3 and CNhit > 3:
    Cam_move.dLoc = [RzG/2.37,0,0] #keep the camera behind the car at all times when no walls are nearby--
    Cam.useXY=False
    Cam.max=26
    Cam.min=26
    Cam.damping=1
elif CPhit <= 3:
    Cam_move.dLoc = [RzG/2.37*CPhit/3-0.1*-1,0,ob['zMove']] #slow the camera to the point where it stops before clipping the wall--
else:
    Cam_move.dLoc = [RzG/3.37*CNhit/3-0.1,0,ob['zMove']]

Cam_move.useLocalDLoc = True
c.activate(Cam_move)

#set the camera height according to the values we calculated for the modulating variables#
if ob['tClear'] is not 2:
    Cam.height=ob["RSpectrum"]/4+ob['gMult']+trackZ
else:
    Cam.height=ob["RSpectrum"]/4+ob['Carry']+trackZ

c.activate(Cam)

It’s still a bit of code, but there’s a lot less guessing of which variable does which because there’s a higher differentiation between how each value is set. Though I do wish I knew a quicker way to find the smallest value in a set instead of running a test for each of them.

I tend to find a lot of comments when re-reading my old code that read something like

#This is really ugly, MUST get round to rewriting this !!!

or

#OMG! What was I thinking here??

It’s sometimes amusing to just go through old scripts to see how much I’ve improved. :slight_smile: