Basketball Collisions with Box2d

I'm working on a new prototype that is a simple drag and shoot arcade basketball game. For this project I am working with LibGDX and the Box2d physics library.

You can play the game here: Play arcade basketball!

This here is the custom collision listener for the game. One quirk that I have not been able to resolve is that there is no guarantee whether fixtureA or fixtureB will be first, and no way to guarantee which of them is which object, so in each of the methods I need to test against both of the fixtures to find the correct one.

Another problem is that I need to do frequent casting to access the properties of my classes, and I am frequently using the user data of the box2d bodies in order to store a reference to those classes. It makes the code hard to read and understand, but as long as I am careful it has not caused any crashes.

Here is the collision class:

```
public class BasketballListener implements ContactListener {

private WormyBasketballWorld basketballWorld;

public BasketballListener(WormyBasketballWorld basketballWorld) {
this.basketballWorld = basketballWorld;
}

@Override
public void preSolve(Contact contact, Manifold oldManifold) {
}
@Override
public void postSolve(Contact contact, ContactImpulse impulse) {
}
@Override
public void endContact(Contact contact) {
}
@Override
public void beginContact(Contact contact) {
Fixture fixtureA = contact.getFixtureA();
Fixture fixtureB = contact.getFixtureB();

this.ballAndHoopCollision(fixtureA, fixtureB);
this.ballAndTargetCollision(fixtureA, fixtureB);
this.scoreDetection(fixtureA, fixtureB);
}

private void ballAndHoopCollision(Fixture fixtureA, Fixture fixtureB) {
if (fixtureA.getBody().getFixtureList().get(0).getFilterData().categoryBits == CollisionFilter.HOOP) {
if (fixtureB.getBody().getUserData() instanceof SimpleWeaponOrb) {

//System.out.pri

Code Duplication

Each of ball*Collision() and scoreDetection() methods contain too much duplicated code. In each of them, the second outer if block can be simply cut out. An additional call of the method with inversed arguments will produce the same effect:

this.ballAndHoopCollision(fixtureA, fixtureB);
this.ballAndHoopCollision(fixtureB, fixtureA);

...

private void ballAndHoopCollision(Fixture fixtureA, Fixture fixtureB) {
    if (fixtureA.getBody().getFixtureList().get(0).getFilterData().categoryBits == CollisionFilter.HOOP) {
        if (fixtureB.getBody().getUserData() instanceof SimpleWeaponOrb) {
           this.basketballWorld.ballCollidedWithHoop(fixtureB.getBody().getAngle(), fixtureB.getBody().getLinearVelocity());
        }
    }
}

Law of Demeter

This code spits abundantly on the Law of Demeter. Calls like

fixtureA.getBody().getFixtureList().get(0).getFilterData().categoryBits

are simply awful and indicate that there is a deep design issue with the definition of the entities used throughout the code.

The worst case is when such calls modify the state of the deeply dependent objects, e.g. markDestroyed() or hasHitFirstTarget. It may be a real hell for debugging.

Encapsulation

categoryBits, hasAlreadyScored and hasHitFirstTarget should not be directly exposed and should be accessed/modified with getters/setters.

Missing Abstractions

There are many class casts like (SimpleWeaponOrb) or (HoopSensor) and checks with instanceof. It looks like the hierarchy of objects that may be returned by getUserData() is chaotic or there are some abstractions missing. I cannot say more about it, because the example is not enough to conclude about.

StackExchange Code Review Q#113438, answer score: 4