Game engine WTF.



  • Heya. I'm a game dev, and I went bug hunting today in the engine Im currently forced to use. The engine were made by a bunch of other guys, and I have had some pretty big problems with it and once in a while I state "WTF" out loud when I see something. I thought I'd post todays piece of code here...

     

    <FONT color=#0000ff size=2>

    float</FONT><FONT size=2> Terrain::GetHeightOnPositionXZ( </FONT><FONT color=#0000ff size=2>float</FONT><FONT size=2> x, </FONT><FONT color=#0000ff size=2>float</FONT><FONT size=2> z ) </FONT><FONT color=#0000ff size=2>const
    </FONT><FONT size=2>{

    </FONT><FONT color=#0000ff size=2>float</FONT><FONT size=2> Px, Pz, Py;
    </FONT><FONT color=#0000ff size=2>float</FONT><FONT size=2> Ay, By, Cy;
    </FONT><FONT color=#0000ff size=2>float</FONT><FONT size=2> dAB, dAC, dCB;

    x /= m_scale;
    z /= m_scale;
    Px = x -(</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)x;
    Pz = z -(</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)z;

    </FONT><FONT color=#008000 size=2>// Top triangle

    </FONT><FONT size=2>

    </FONT><FONT color=#0000ff size=2>if</FONT><FONT size=2>( Pz < ( 1 - Px ) )
    {
    Ay = m_heightValues[(</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)x + (</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)z * m_size];
    By = m_heightValues[(</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)x + (</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)(z + 1) * m_size];
    Cy = m_heightValues[(</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)(x + 1) + (</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)z * m_size];

    dAB = By - Ay;
    dAC = Cy - Ay;
    Py = Ay + (dAB * Pz);
    Py += dAC * Px;
    }

    </FONT><FONT color=#008000 size=2>// Lower triangle

    </FONT><FONT size=2>

    </FONT>//Snipped the same stuff as in the if only for the lower triangle. Pretty much exactly the same code.<FONT size=2>

    </FONT><FONT color=#0000ff size=2>return</FONT><FONT size=2> Py;</FONT>

    <FONT size=2>}</FONT>

    <FONT size=2></FONT> 

    <FONT size=2>This code actually can be called with negative X and Y values.
    This specific code is from a terrain class, and it returns the height on the position you feed it.

    Lets see if you can find the WTF.</FONT><FONT size=2>

    </FONT>


  • And yes, Im aware of the sad state of my english, thats not the WTF. ;)



  • @Ulvhamne said:

    Py = Ay + (dAB * Pz)+dAC * Px;




    Looks suspicious for me. Bad interpolation?



  • Well, thats a minor one. ;)



  • <font>x /= m_scale;
    z /= m_scale;
    Px = x -(</font><font>int</font><font>)x;
    Pz = z -(</font><font>int</font><font>)z;</font>


    <font>Px and Pz are always 0?
    </font>



  • I think it lies with the Pz and Pz calculations

    I'm not sure how the rounding will be handled for the int casting.  So the results are not consistent. 



  • Yeah, I'm an idiot. Px and Pz aren't always zero...



  • Looks like Py does not get set to anything unless the if statement if ( Pz < ( 1 - Px ) ) is true
    I guess it would return 0 in that case?

    Obviously, the lack of any helpful comments or variable names makes this difficult to understand and maintain and is WTFworthy.
    Also, no explanation of m_scale and m_size
    I guess they are member variables of the Terrain class or global variables.



  • @ferrengi said:

    Looks like Py does not get set to anything unless the if statement if ( Pz < ( 1 - Px ) ) is true
    I guess it would return 0 in that case?


    I might be wrong but I think Py will be handled by the else block that was snipped out by the original poster. As for the WTF, yes I also think it is the slightly weird calc of the reminder of Px and Pz. And the function should at least check for negative values.



  • Not quite there yet. ;)

    And yes, the m_ are members.
    And yes, ´there is supposed to be an else block there too.. Accidently snipped away the "else { " too. Heh.



  • Well, i'll take a shot on it also, couldnt help my self trying to think about it :)
    could this be  the problem perhaps ?

    pretend z = -5.9 and m_scale = 1
    pz = z - (int)z = -5.9 - (-5) => pz = -0.9

    if(m_size  > 1) and
    if x < -(z * m_size) or x < 0 indexation of array m_heightValues will be negative



  • @Ulvhamne said:

    Heya. I'm a game dev, and I went bug hunting today in the engine Im currently forced to use. The engine were made by a bunch of other guys, and I have had some pretty big problems with it and once in a while I state "WTF" out loud when I see something. I thought I'd post todays piece of code here...

     

    <font color="#0000ff" size="2"> </font>

    <font color="#0000ff" size="2">float</font><font size="2"> Terrain::GetHeightOnPositionXZ( </font><font color="#0000ff" size="2">float</font><font size="2"> x, </font><font color="#0000ff" size="2">float</font><font size="2"> z ) </font><font color="#0000ff" size="2">const
    </font><font size="2">{</font>

    <font color="#0000ff" size="2">if</font>( Pz < ( 1 - Px ) )

    <font size="2">{
    Ay = m_heightValues[(</font><font color="#0000ff" size="2">int</font><font size="2">)x + (</font><font color="#0000ff" size="2">int</font><font size="2">)z * m_size];
    By = m_heightValues[(</font><font color="#0000ff" size="2">int</font><font size="2">)x + (</font><font color="#0000ff" size="2">int</font><font size="2">)(z + 1) * m_size];
    Cy = m_heightValues[(</font><font color="#0000ff" size="2">int</font><font size="2">)(x + 1) + (</font><font color="#0000ff" size="2">int</font><font size="2">)z * m_size];</font>

    <font size="2">}</font>

    <font color="#008000" size="2">// Lower triangle</font>

    <font size="2"></font>

    <font size="2">This code actually can be called with negative X and Y values.
    This specific code is from a terrain class, and it returns the height on the position you feed it.

    Lets see if you can find the WTF.</font><font size="2"></font>



    If x and z can be negative, that means that it is possible that the index being fed into the m_heightValues[] array can be negative. That means that there is an out of bounds error that is not being caught. The values in Ay, By, and Cy are not being checked so the code has no way of knowing if a valid value was read from the array.


  • yeah, and the terrain is supposed to work with negative numbers. At least according to the spec.



  • Wow, count the WTFs:

    TheUseOfCamelCasing

    sHungarian sNotation

    Defining all the variables up at the top

    Poor checking to see which triangle the point is in

    Having "pretty much exactly the same code" twice in the same function



  • @Ulvhamne said:

    yeah, and the terrain is supposed to work with negative numbers. At least according to the spec.


    So, we got it right?



  • yeah. ;)

    the density of wtf's in that class are the same all over..



    I actually removed 1/3rd of the code because it didnt do something once.

    I might post something more later on. ;) Or a horror story.




  • @Lews said:

    Wow, count the WTFs:
    TheUseOfCamelCasing
    sHungarian sNotation
    Defining all the variables up at the top
    Poor checking to see which triangle the point is in
    Having "pretty much exactly the same code" twice in the same function

    use of float



  • Honestly, and I'm ashamed to admit it, this looks almost identical to my own code.

    The only difference is that for me, the terrain was stored as a grid of triangles, not just an array of height values.



  • 1) TheUseOfCamelCasing

    1. sHungarian sNotation
    2. Defining all the variables up at the top
    3. Poor checking to see which triangle the point is in
    4. Having "pretty much exactly the same code" twice in the same function
    5. use of float


    Let's see.
    • used in many langages and many APIs. Maybe it is adequate in that case. not a WTF
    • widely used. recommanded by many (but not me). not a WTF
    • sometimes it makes the code much more readable. not a WTF

      4)  bug? not a WTF
    • can it be factorized? not sure. not a WTF
    • this is the best choice. fast. thread-safe. precise. not a WTF.




  • Another one:

    The width of the terrain is called "size".

    @trollable said:

    5) can it be factorized? not sure. not a WTF


    Most likely, it can. If you're doing the same thing several times with only a few differences, you make a goddamn function. It can be inline, so performance is not an issue.



  • @Zlodo said:

    Another one:

    The width of the terrain is called "size".

    @trollable said:
    5) can it be factorized? not sure. not a WTF


    Most likely, it can. If you're doing the same thing several times with only a few differences, you make a goddamn function. It can be inline, so performance is not an issue.


    You take that as a rule? Personally, I think it confuses and adds obfuscation if you factor code that is only repeated once or twice, and stick it in a function. In this case, it is either one triangle or another - you don't need to make a function to do that.



  • @trollable said:

    1) TheUseOfCamelCasing
    2) sHungarian sNotation
    3) Defining all the variables up at the top
    4) Poor checking to see which triangle the point is in
    5) Having "pretty much exactly the same code" twice in the same function
    6) use of float

    Let's see.
    1) used in many langages and many APIs. Maybe it is adequate in that case. not a WTF
    2) widely used. recommanded by many (but not me). not a WTF
    3) sometimes it makes the code much more readable. not a WTF
    4)  bug? not a WTF
    5) can it be factorized? not sure. not a WTF
    6) this is the best choice. fast. thread-safe. precise. not a WTF.

     

    Ditto from me on all counts...except maybe the float/double thing.  Pretty sure doubles are handled without multiple instructions in the FPU, which would make them thread-safe.  On the other hand, I really should go look that up.  [^o)]



  • @mrprogguy said:

    @trollable said:

    1) TheUseOfCamelCasing
    2) sHungarian sNotation
    3) Defining all the variables up at the top
    4) Poor checking to see which triangle the point is in
    5) Having "pretty much exactly the same code" twice in the same function
    6) use of float

    Let's see.
    1) used in many langages and many APIs. Maybe it is adequate in that case. not a WTF
    2) widely used. recommanded by many (but not me). not a WTF
    3) sometimes it makes the code much more readable. not a WTF
    4)  bug? not a WTF
    5) can it be factorized? not sure. not a WTF
    6) this is the best choice. fast. thread-safe. precise. not a WTF.

     

    Ditto from me on all counts...except maybe the float/double thing.  Pretty sure doubles are handled without multiple instructions in the FPU, which would make them thread-safe.  On the other hand, I really should go look that up.  [^o)]

    depends on the CPU...

    Hungarian Notation is a WTF, though it's a WTF not so much of the persons using it but of the person inventing it [;)]



  • @Ulvhamne said:

    Heya. I'm a game dev, and I went bug hunting today in the engine Im currently forced to use. The engine were made by a bunch of other guys, and I have had some pretty big problems with it and once in a while I state "WTF" out loud when I see something. I thought I'd post todays piece of code here...

     

    <FONT color=#0000ff size=2>

    float</FONT><FONT size=2> Terrain::GetHeightOnPositionXZ( </FONT><FONT color=#0000ff size=2>float</FONT><FONT size=2> x, </FONT><FONT color=#0000ff size=2>float</FONT><FONT size=2> z ) </FONT><FONT color=#0000ff size=2>const
    </FONT><FONT size=2>{

    </FONT><FONT color=#0000ff size=2>float</FONT><FONT size=2> Px, Pz, Py;
    </FONT><FONT color=#0000ff size=2>float</FONT><FONT size=2> Ay, By, Cy;
    </FONT><FONT color=#0000ff size=2>float</FONT><FONT size=2> dAB, dAC, dCB;

    x /= m_scale;
    z /= m_scale;
    Px = x -(</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)x;
    Pz = z -(</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)z;

    </FONT><FONT color=#008000 size=2>// Top triangle

    </FONT><FONT size=2>

    </FONT><FONT color=#0000ff size=2>if</FONT><FONT size=2>( Pz < ( 1 - Px ) )
    {
    Ay = m_heightValues[(</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)x + (</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)z * m_size];
    By = m_heightValues[(</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)x + (</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)(z + 1) * m_size];
    Cy = m_heightValues[(</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)(x + 1) + (</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)z * m_size];

    dAB = By - Ay;
    dAC = Cy - Ay;
    Py = Ay + (dAB * Pz);
    Py += dAC * Px;
    }

    </FONT><FONT color=#008000 size=2>// Lower triangle

    </FONT><FONT size=2>

    </FONT>//Snipped the same stuff as in the if only for the lower triangle. Pretty much exactly the same code.<FONT size=2>

    </FONT><FONT color=#0000ff size=2>return</FONT><FONT size=2> Py;</FONT>

    <FONT size=2>}</FONT>

    <FONT size=2></FONT> 

    <FONT size=2>This code actually can be called with negative X and Y values.
    This specific code is from a terrain class, and it returns the height on the position you feed it.

    Lets see if you can find the WTF.</FONT><FONT size=2>

    </FONT>

     

    Looks to me like sample code straight out of an Andre Lamothe book written in 1995



  • @Lews said:

    Wow, count the WTFs:
    sHungarian sNotation

    Is this in reference to the dAB variable naming?  In all honesty, I thought the 'd' in this case was referring to distance, perhaps, rather than double.

     



  • @Hux said:

    Is this in reference to the dAB variable naming?  In all honesty, I thought the 'd' in this case was referring to distance, perhaps, rather than double.

    There's another strike against hungarian notation.  Can easily be misleading, rather than informational.



  • @e.thermal said:

    @Ulvhamne said:

    Heya. I'm a game dev, and I went bug hunting today in the engine Im currently forced to use. The engine were made by a bunch of other guys, and I have had some pretty big problems with it and once in a while I state "WTF" out loud when I see something. I thought I'd post todays piece of code here...

     

    <FONT color=#0000ff size=2>

    float</FONT><FONT size=2> Terrain::GetHeightOnPositionXZ( </FONT><FONT color=#0000ff size=2>float</FONT><FONT size=2> x, </FONT><FONT color=#0000ff size=2>float</FONT><FONT size=2> z ) </FONT><FONT color=#0000ff size=2>const
    </FONT><FONT size=2>{

    </FONT><FONT color=#0000ff size=2>float</FONT><FONT size=2> Px, Pz, Py;
    </FONT><FONT color=#0000ff size=2>float</FONT><FONT size=2> Ay, By, Cy;
    </FONT><FONT color=#0000ff size=2>float</FONT><FONT size=2> dAB, dAC, dCB;

    x /= m_scale;
    z /= m_scale;
    Px = x -(</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)x;
    Pz = z -(</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)z;

    </FONT><FONT color=#008000 size=2>// Top triangle

    </FONT><FONT size=2>

    </FONT><FONT color=#0000ff size=2>if</FONT><FONT size=2>( Pz < ( 1 - Px ) )
    {
    Ay = m_heightValues[(</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)x + (</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)z * m_size];
    By = m_heightValues[(</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)x + (</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)(z + 1) * m_size];
    Cy = m_heightValues[(</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)(x + 1) + (</FONT><FONT color=#0000ff size=2>int</FONT><FONT size=2>)z * m_size];

    dAB = By - Ay;
    dAC = Cy - Ay;
    Py = Ay + (dAB * Pz);
    Py += dAC * Px;
    }

    </FONT><FONT color=#008000 size=2>// Lower triangle

    </FONT><FONT size=2>

    </FONT>//Snipped the same stuff as in the if only for the lower triangle. Pretty much exactly the same code.<FONT size=2>

    </FONT><FONT color=#0000ff size=2>return</FONT><FONT size=2> Py;</FONT>

    <FONT size=2>}</FONT>

    <FONT size=2></FONT> 

    <FONT size=2>This code actually can be called with negative X and Y values.
    This specific code is from a terrain class, and it returns the height on the position you feed it.

    Lets see if you can find the WTF.</FONT><FONT size=2>

    </FONT>

     

    Looks to me like sample code straight out of an Andre Lamothe book written in 1995

     

    Not impossible. Never read the book though. And a book that simply disregards cheching values doesnt really sound too hot.



  • @e.thermal said:

    Not impossible. Never read the book though. And a book that simply disregards cheching values doesnt really sound too hot.

    That's a bit unfair.  Most code in books has a lot of that kind of thing stripped out to focus on illustrating the subject at hand without adding too much noise.  It's pretty common for error checking, return codes, exceptions etc to fall by the wayside to aid clarity.



  • @LoztInSpace said:

    @e.thermal said:

    Not impossible. Never read the book though. And a book that simply disregards cheching values doesnt really sound too hot.

    That's a bit unfair.  Most code in books has a lot of that kind of thing stripped out to focus on illustrating the subject at hand without adding too much noise.  It's pretty common for error checking, return codes, exceptions etc to fall by the wayside to aid clarity.

    Yes, forgive me for being a bit bitter. I have to work with these obnoxiously slightly broken classes. ;)
    But any books that show off this terrain class... *Shudder*


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.