Some minor/pointless changes

Posts regarding potential bugs, enhancement requests, and general feedback on use of dyn4j
mtsamis
Posts: 66
Joined: Thu May 09, 2013 5:36 pm

Some minor/pointless changes

Postby mtsamis » Wed Jun 15, 2016 8:26 am

Recently, at a time i was kind of bored, i started exploring some of dyn4j's source code, and found couple of things that could be changed. Note those are very minor stuff that could be skipped as well, i was just bored ;)

World.java

Code: Select all

for (int i = 0; i < size; i++) {
   Body body = this.bodies.get(i);
   body.setOnIsland(false);
   if (continuousDetectionMode != ContinuousDetectionMode.NONE) {
      body.transform0.set(body.getTransform());
   }
}


Maybe put the if branch outisde the loop, no need to be tested in every loop

Code: Select all

if (continuousDetectionMode == ContinuousDetectionMode.NONE) {
   for (int i = 0; i < size; i++) {
      Body body = this.bodies.get(i);
      body.setOnIsland(false);
   }
} else {
   for (int i = 0; i < size; i++) {
      Body body = this.bodies.get(i);
      body.setOnIsland(false);
      body.transform0.set(body.getTransform());
   }
}


The same could be done to this one in the World.detect() method, where "if (this.bounds != null) {" doesn't need to be checked in every loop (checked bodies.size() ^ 2 times)

Code: Select all

for (int i = 0; i < size; i++) {
   Body body = this.bodies.get(i);
   if (!body.isActive()) continue;
   body.contacts.clear();
   if (this.bounds != null) {
      if (this.bounds.isOutside(body)) {
         body.setActive(false);
         for (int j = 0; j < blSize; j++) {
            BoundsListener bl = boundsListeners.get(j);
            bl.outside(body);
         }
      }
   }
   this.broadphaseDetector.update(body);
}


Also in Sap.java there is this nested if some 3 times

Code: Select all

if (proxy.aabb.getMaxX() > aabb.getMinX()) {
   if (proxy.aabb.overlaps(aabb)) {
      ...
   }
}


But since AABB.overlaps(...) method starts with

Code: Select all

if (this.min.x > aabb.max.x || this.max.x < aabb.min.x) {
   // the aabbs do not overlap along the x-axis
   return false;
}


The previous nested if can be written just as

Code: Select all

if (proxy.aabb.overlaps(aabb)) {
   ...
}


And speaking of AABB.overlaps(...) method it could be written, for simplicity, as:

Code: Select all

public boolean overlaps(AABB aabb) {
   if (this.min.x > aabb.max.x || this.max.x < aabb.min.x) {
      return false;
   }
   
   if (this.min.y > aabb.max.y || this.max.y < aabb.min.y) {
      return false;
   }
   
   return true;
}

(kind of totally pointless too)

Also from World.java

Code: Select all

ContactEdge contactEdge = body.contacts.get(j);
constraint = contactConstraint = contactEdge.interaction;
if (contactConstraint.isSensor()) continue;
Body other = contactEdge.other; //unnecessary Object allocation if constraint.isOnIsland() == true
if (constraint.isOnIsland()) continue;

could be written as (To avoid a possible unnecessary Object allocation)

Code: Select all

ContactEdge contactEdge = body.contacts.get(j);
constraint = contactConstraint = contactEdge.interaction;
if (constraint.isOnIsland() || contactConstraint.isSensor()) continue;
Body other = contactEdge.other;

(there are couple some similar cases to this later in the code)

And finally i saw a lot of invocations of Math.min(double, double) and Math.max(double, double). Maybe you should replace those with two custom made equivalent methods, because they made quite some pointless (for the case) checks
Code from Math.java:

Code: Select all

public static double  min(double a, double b)  {
         if (a != a) return a;   // a is NaN
         
         if ((a == 0.0d) && (b == 0.0d) && (Double.doubleToLongBits(b) == negativeZeroDoubleBits)) {
                return b;
         }
         
         return (a <= b) ? a : b;
}

public static long  doubleToLongBits(double value) {
        long result = doubleToRawLongBits(value);
        if ( ((result & DoubleConsts.EXP_BIT_MASK) ==
              DoubleConsts.EXP_BIT_MASK) &&
             (result & DoubleConsts.SIGNIF_BIT_MASK) != 0L)
            result = 0x7ff8000000000000L;
        return result;
}

In your case this could be just

Code: Select all

public static double  min(double a, double b)  {
      return (a <= b) ? a : b;
}


(I removed dyn4j comments to occupy less space)

William
Site Admin
Posts: 351
Joined: Sat Feb 06, 2010 10:23 pm

Re: Some minor/pointless changes

Postby William » Thu Jun 16, 2016 8:20 pm

Thanks! I'll take a look at these for sure.

William


Return to “Bugs, Enhancements, Feedback”

Who is online

Users browsing this forum: No registered users and 1 guest