Ugly jump mechanics (C#.NET) or aren't there any beautiful ones?

[Falke88]

Hey guys - I just finished my working jump mechanics for a game which I wrote with the help of the SFML.NET Game DEV Book and the .NET bindings for sure.

I'm now off to my own code - which feels trashy right away. Gonna show you my code snipped for the jump mechanics where hopefully every field is being named well enough to leave no questions.

What kind of help I need? I just want you experiences buddys to look over the snipped and try to give me some tips n tricks to do it better, more beautiful or even more effective!?


Some things to know before reading:

PlayerControl.isJumping
PlayerControl.isMaxHeightReached   // These things are static variables within the Player.cs (PlayerControl.cs)
PlayerControl.maxJumpHeight
PlayerControl.oldHeight

Mack
 

void JumpMechanics(SceneNode node, Time dt)
        {
            ///////////// Jump Feature Code //////////////////////////////////////

            float playerPosY = ((Entity)node).Position.Y;                           // actual playerPos.Y or Height for more readability later
            float gravity = ((Entity)node).gravity;                                 // same reason as above
            Vector2f oldVelocity = ((Entity)node).GetVelocity();                    // and same reason again

            if (PlayerControl.isJumping && !PlayerControl.isMaxHeightReached)       // Will ask if still in a jump phase and the max height isnt reached yet
                if (playerPosY > PlayerControl.maxJumpHeight)                       // Is the actual player height lower then the maximum jump height (Remember Cart System goes downwards with positive values)
                    ((Entity)node).SetVelocity(oldVelocity.X, -350.0f - gravity);   // if yes then please keep actualy velocity.X but increase velocity.Y and watching out for the gravity (200.0F) aswell
                else if (playerPosY <= PlayerControl.maxJumpHeight)                 // Is the players height higher or equal as the maximum jump height...
                {          
                    PlayerControl.isMaxHeightReached = true;                        // ... then please set true to the booleon field which asks if the MaxHeight is being reached
                    ((Entity)node).SetVelocity(oldVelocity.X, 0.0f);                // and set the vertical velocity to 0.0f. No jumping up anymore
                }                                                                   // now the gravity will take over and its velocity is always Y = 200.0f;

            if (PlayerControl.isJumping && PlayerControl.isMaxHeightReached)        // Is the player still in a jump and has already reached its maximum point
                if (playerPosY == PlayerControl.oldHeight)                          // annnnd is the actualy players height equal to the oldHeight when he started jumpin'
                {
                    if (!Keyboard.IsKeyPressed(Keyboard.Key.Up))                    // Check if the jump key (UP) is still being pressed. To avoid re-jumps all the time.
                    {                                                               // that way there won't be any re-jumps all the time
                        PlayerControl.isJumping = false;                            // Set Jumpytime to false
                        PlayerControl.isMaxHeightReached = false;                   // and reset if the maxheight is being reached for the next jump phase.
                    }
                    if(!Keyboard.IsKeyPressed(Keyboard.Key.Down))                   // Watch out if the Crouching buttin isn't in use - would bring in some TextureChanging artifacts if not watched.
                        ((Mack)node).ChangeTexture(Mack.Type.Normal);               // ah yea...and give the sprite his normal texture back. That jumpy one looked awful.

                }
            ///////////// Jump Feature Code //////////////////////////////////////
        }

[Ixrec]

I don't use C#, but I see some things that bug me about this code that should be relatively language-agnostic:

- It feels strange that some of your information is coming from static PlayerControl variables and other information is coming from casting(?) the SceneNode to an Entity or a Mack.  That's an "architecture smell" to me.

- Having to frequently cast(?) the SceneNode to what I assume are some of its derived classes also seems like a code smell.  I would either expect the interface to have a handleInput() method you can call that's just blank for non-player objects, or one of the update methods to have a single line that says "If this node is a Player, call handleInput() on it" so the downcasting(?) is kept to a bare minimum.  Maybe this is different in C#.

- About these two lines...

if (playerPosY > PlayerControl.maxJumpHeight)
// Is the actual player height lower then the maximum jump height (Remember Cart System goes downwards with positive values)

else if (playerPosY <= PlayerControl.maxJumpHeight)
// Is the players height higher or equal as the maximum jump height...

It seems like the comparison operators you're using are the opposite of what you intended to use.

- This comment here...

if(!Keyboard.IsKeyPressed(Keyboard.Key.Down))
// Watch out if the Crouching buttin isn't in use - would bring in some TextureChanging artifacts if not watched.

tells me you would benefit from implementing a state machine for your player character, as described at http://gameprogrammingpatterns.com/state.html.

- This pair of conditions didn't make any sense to me the first few times I looked at it:

if (PlayerControl.isJumping && PlayerControl.isMaxHeightReached)
// Is the player still in a jump and has already reached its maximum point
if (playerPosY == PlayerControl.oldHeight)
// annnnd is the actualy players height equal to the oldHeight when he started jumpin'

At first it looks like this is saying "if the player is jumping, is at the maximum possible jump height, and is also at the height where he started his jump" which is obviously impossible unless the max jump height is zero.  I think it might be clearer if isMaxHeightReached was renamed to something more like isDoneAcceleratingUpward, since I think that's what it's actually being used as.  I'm guessing the comparison to oldHeight is meant to test if the player has "landed" on solid ground or not, but unless all of your platforms are at exactly the same height, that doesn't seem right.

- In general, if you feel the need to comment your code that much, then your code probably isn't as self-explanatory or readable as it should be.  You'll notice in some of the things I just pointed out that a few comments contradict the code (like the PosY to maxJumpHeight comparisons), a few others restate the code instead of explaining it (like the == oldHeight comment), and a few others literally say the code's not readable enough.

[Falke88]

Heya and first thanks to your responds and taking your time for me here.

About

if (playerPosY > PlayerControl.maxJumpHeight)

and the other one. Well actually it works pretty good at runtime. I ask in this example here if the playerPosY (So the HEIGHT of the player cause of teh jumpingz) is "higher" than the maximum of the height which is reachable. I know what you mean by opposite - since in fact the Player should be lower than the maximum height in this case semantically. But since the cartesian system here in SFML (mybe in every graphics engine?) works that way that the Y Axis goes positive downwards not upwards like we guys do know it from mathmatics in school it really fits here. I'm not sure if you ever used SMFL with this...but I doubt you didn't - mybe you just forgot about that fact shortly or I misunderstood you here.

About that state machine - thanks for the link - I kinda read about that in my Game AI Programming lecture - but am not experiences enough to realise its fields of use yet. Should read more about it I think.

if (playerPosY == PlayerControl.oldHeight)

Well sorry my fault not explaining it good enough... You should be aware of the fact that my gravity field is always updated on the velocity of all entities within the gameworld. So actually if the player jumps up and hasn't reached it the maximum height yet - the velocity is on the run for gaining height. After the player reached the maximum the velocity is turned off and therefore only the gravity will affect any movement...which actually is only downwards.
So if the player finally hit his "oldHeight" the flag for jumping will be turned off again so another jump can be executed.

Well I comment that much to actually get commenting into my blood - Because I didn't commented before that at all - and really fucked up after reading code after a break of a month.
Also I comment everything to really rethink what I do - kinda explaining the code to an imagined invisible friend


But thanks for your critics at all - really made me rethink some processes here.