Fixed BetterCharacterControl "flickering" when not moving

I just tried to make a pull-request but I saw that the “pull guideline” says to post first here all changes.

The description:

Sometimes, when the BetterCharacterControl is not moving, it flickers due it physics being always active. The reason for it being always active is that on the prePhysicsTick method it always set the rigidBody’s linear velocity (and the method PhysicsRigidBody.setLinearVelocity(Vector3f) reactivates the physics each time is called).

The fix consist on just comparing if the current velocity and the setting one aren’t the same and, thus, not setting it (using ZERO_TOLERANCE instead 0 to best results).

The OLD code:

public void prePhysicsTick(PhysicsSpace space, float tpf) {
        checkOnGround();
        if (wantToUnDuck && checkCanUnDuck()) {
            setHeightPercent(1);
            wantToUnDuck = false;
            ducked = false;
        }
        TempVars vars = TempVars.get();
        
        // dampen existing x/z forces
        float existingLeftVelocity = velocity.dot(localLeft);
        float existingForwardVelocity = velocity.dot(localForward);
        Vector3f counter = vars.vect1;
        existingLeftVelocity = existingLeftVelocity * physicsDamping;
        existingForwardVelocity = existingForwardVelocity * physicsDamping;
        counter.set(-existingLeftVelocity, 0, -existingForwardVelocity);
        localForwardRotation.multLocal(counter);
        velocity.addLocal(counter);

        float designatedVelocity = walkDirection.length();
        if (designatedVelocity > 0) {
            Vector3f localWalkDirection = vars.vect1;
            //normalize walkdirection
            localWalkDirection.set(walkDirection).normalizeLocal();
            //check for the existing velocity in the desired direction
            float existingVelocity = velocity.dot(localWalkDirection);
            //calculate the final velocity in the desired direction
            float finalVelocity = designatedVelocity - existingVelocity;
            localWalkDirection.multLocal(finalVelocity);
            //add resulting vector to existing velocity
            velocity.addLocal(localWalkDirection);
        }
        rigidBody.setLinearVelocity(velocity);
        if (jump) {
            //TODO: precalculate jump force
            Vector3f rotatedJumpForce = vars.vect1;
            rotatedJumpForce.set(jumpForce);
            rigidBody.applyImpulse(localForwardRotation.multLocal(rotatedJumpForce), Vector3f.ZERO);
            jump = false;
        }
        vars.release();
    }

The NEW code:

public void prePhysicsTick(PhysicsSpace space, float tpf) {
        checkOnGround();
        if (wantToUnDuck && checkCanUnDuck()) {
            setHeightPercent(1);
            wantToUnDuck = false;
            ducked = false;
        }
        TempVars vars = TempVars.get();

        Vector3f currentVelocity = vars.vect2.set(velocity);
        
        // dampen existing x/z forces
        float existingLeftVelocity = velocity.dot(localLeft);
        float existingForwardVelocity = velocity.dot(localForward);
        Vector3f counter = vars.vect1;
        existingLeftVelocity = existingLeftVelocity * physicsDamping;
        existingForwardVelocity = existingForwardVelocity * physicsDamping;
        counter.set(-existingLeftVelocity, 0, -existingForwardVelocity);
        localForwardRotation.multLocal(counter);
        velocity.addLocal(counter);

        float designatedVelocity = walkDirection.length();
        if (designatedVelocity > 0) {
            Vector3f localWalkDirection = vars.vect1;
            //normalize walkdirection
            localWalkDirection.set(walkDirection).normalizeLocal();
            //check for the existing velocity in the desired direction
            float existingVelocity = velocity.dot(localWalkDirection);
            //calculate the final velocity in the desired direction
            float finalVelocity = designatedVelocity - existingVelocity;
            localWalkDirection.multLocal(finalVelocity);
            //add resulting vector to existing velocity
            velocity.addLocal(localWalkDirection);
        }
        if(currentVelocity.distance(velocity) > FastMath.ZERO_TOLERANCE) rigidBody.setLinearVelocity(velocity);
        if (jump) {
            //TODO: precalculate jump force
            Vector3f rotatedJumpForce = vars.vect1;
            rotatedJumpForce.set(jumpForce);
            rigidBody.applyImpulse(localForwardRotation.multLocal(rotatedJumpForce), Vector3f.ZERO);
            jump = false;
        }
        vars.release();
    }

The changed log:

         TempVars vars = TempVars.get();
 
+        Vector3f currentVelocity = vars.vect2.set(velocity);
+        
         // dampen existing x/z forces
         float existingLeftVelocity = velocity.dot(localLeft);
         float existingForwardVelocity = velocity.dot(localForward);
 @@ -194,7 +196,7 @@ public void prePhysicsTick(PhysicsSpace space, float tpf) {
             //add resulting vector to existing velocity
             velocity.addLocal(localWalkDirection);
         }
-        rigidBody.setLinearVelocity(velocity);
+        if(currentVelocity.distance(velocity) > FastMath.ZERO_TOLERANCE) rigidBody.setLinearVelocity(velocity);
         if (jump) {

Hope this change is valid and if not, I would like to know why :frowning:

3 Likes

Looks okay.

if(currentVelocity.distance(velocity) > FastMath.ZERO_TOLERANCE) rigidBody.setLinearVelocity(velocity);

Please avoid “if statements” that are on the same line and also make sure you add brackets around the code that follows the statement.

3 Likes

[quote=“DannyJo, post:3, topic:34650”]
Please avoid “if statements” that are on the same line[/quote]

Here I must differ with your “best practices”. For really simple ifs I like to have them in the same line (I’m agree on that this case can be too large and so, it is probably better separated on two lines).

Adding brackets to a if/for sentence that is only having, and potentially always having, one sentence inside is just ugly and unneeded. Again, I suppose this are “best practices” and may differ from each other.
However, if you jme3 core developers like it much more in that way, I can take care of it whenever I make a pull request.

1 Like

Requested then.

1 Like

I wasn’t stating anything about “my” best practices, I merely think that if you are going to contribute code to a project you make sure you format your code the way the project formats it’s code. I’m not going to argue if it’s better or not to have brackets and new lines. Either way, your request was accepted and merged so I’m sure it’s all good :smile:

1 Like

Its gonna be changed the next time some core member works on the file and presses Ctrl-Shift-F anyway :wink:

2 Likes

Yes, in the future, please always put braces around blocks for ifs… and especially for loops. It avoids a lot of needless issues and makes it a lot easier to maintain the code. Regardless of your own personal preference, we have lots of experience maintaining code and having the braces eliminates some strange accidental errors, makes the code easier to debug, and generally forces spacing the makes the code more readable even if the person writing it is somehow afraid of whitespace.

ie:

if( foo )
   someline
someimmediate other line

I used to leave the braces off of simple statements years ago as it was one of my last lazy holdouts… but I finally started counting all of the times I had to add them just to insert a log statement or test something and I decided it was worth having them. So this is one of those “styling” things that I not only agree with but strongly advocate.

And Normen is also right, though I don’t arbitrarily autoformat files, I will fix these issues when I see them. Some JME code has the ugly-form single line ifs, also and I try to fix them whenever I see them.

1 Like

Ok, is good to know the core preferences (I’ll try to take it in account ;)).

I didn’t have any of those errors yet… but I suppose that is normal, specially when different people touch the same code.

Just like that, I just like it more visually without them.

Whatever thanks for all the explanation about the “whys”. I’ll try to make it the “good way” next time :wink: