Getting specific result from the classes of two objects with polymorphism

[Daniel Keepfort]

Hi!
I just registered on the forums but I have been reading for a while.

I have a problem with a project we're working on. It's a 2D platformer game and I have programmed the physics and collision handling.

The problem is, I want to get a specific response out of a collision depending on the actual types of the two colliding objects, usig polymorphism instead of instanceof and getClass checks.
Once a collision has occurred, I invoke a metod similar to this one (simplified):

// Invoked once a collision has occured
private void collide(GameObject a, GameObject b) {
	
	a.collisionEffect(b);
	b.collisionEffect(a);
	
	if ( isPhysicalCollision(a.getClass(), b.getClass()) ) {
		handlePhysicalCollision(a, b);
	}
	
}

Where

GameObject.collisionEffect(GameObject other)

handles additional effects such as picking up an object or getting hurt by an enemy. This method uses instanceof to switch on the type of 'other'.

Is there a better way to do this? (I'm sure there is!)

Thanks, Daniel

(edit: sorry if this should go in the Beginners section!)

[Istvan Kovacs]

You are right, branching based on object type is a code smell.

You could overload the method, and have different argument types:

collisionEffect(SomeType other)
collisionEffect(SomeOtherType other)
But to use them, you'd have to cast the argument you use in the call, which is not elegant.

if (b instanceof SomeType) {
  a.collisionEffect((SomeType) b); // will call collisionEffect(SomeType other)
} else if (b instanceof SomeOtherType) {
  a.collisionEffect((SomeOtherType) b); // will call collisionEffect(SomeOtherType other)
} else {
  a.collisionEffect(b); // will call collisionEffect(GameObject other)
}

A nicer solution would be to get the collision effect from the current argument:

a.runCollisionEffect(b.getCollisionEffect());

This way, each class can define its own collision effect.

If the collision effect depends on both types (type of 'a' as well as type of 'b'), you could use a Map. A centralised version would look like this:

Map<String, CollisionEffect> collisionEffects = ...
collisionEffects.put(makeKeyFromTypes(SomeType.class, SomeOtherType.class), new SomeTypeAndSomeOtherTypeCollisionEffect());
...repeat with other types...

with

private static String makeKeyFromTypes(Class<?> oneType, Class<?> otherType) {
  return oneType.getName() + "-" + otherType.getName();
}

Or you could have a similar map in GameObject, and each subtype would fill it with different values. In SomeType:

Map<Class<? extends GameObject>, CollisionEffect> effects = new HashMap...
effects.put(SomeOtherType, new SomeTypeAndSomeOtherTypeCollisionEffect());
... add some more...
super.setCollisionEffects(effects);

void collisionEffect(GameObject other) {
  super.getCollisionEffectFor(other.getClass()).runEffect();
}

[Daniel Keepfort]

Hi!
Thank you for the response.

Yes, the effects depend on the type of both objects, and is different in both objects.

Maybe your solution of using a static map is the best so far. But it still depends on a type check when making and checking type strings.
I did something like this for the isPhysicalCollision method:

	public static boolean isPhysicalCollision(Class<? extends GameObject> class1, Class<? extends GameObject> class2) {
		
		String cn1 = class1.getSimpleName();
		String cn2 = class2.getSimpleName();
		String concat;
		
		// Sort the two class names lexicographically
		if (cn1.compareTo(cn2) < 0) {
			concat = cn1 + "-" + cn2;
		}
		else {
			concat = cn2 + "-" + cn1;
		}
		
		// See if the combined string of names is in physicalCollisions
		if (physicalCollisions.contains(concat))
			return true;
		
		return false;
	}
	
	private static List<String> physicalCollisions = new ArrayList<String>();
	static {
		physicalCollisions.add("Enemy-TerrainSection");
		physicalCollisions.add("Enemy-Obstacle");
		physicalCollisions.add("Enemy-Player");
		physicalCollisions.add("Enemy-Projectile");
		// etc...
	}

It works but is somewhat fragile, and you have to add strings to the list if you add new types.
I would like to use polymorphism but I'm not sure if it's possible.

[Karthik Shiraly]

Hi,

Here're my suggestions for the modelling, to make the code less dependent on types:

First, I introduce some new concepts for better abstraction:
A collision involves a Subject who responds to a Stimulant, the response being different for different Stimulus.
So instead of responses based on class name, the response will now be based on the 'collision stimulus' given to it. Each GameObject is modelled as providing a particular kind of stimulus in a collision.

Model the set of stimuli in the system as Strings or enums.

class Stimuli {
    public static final String INJURER = "injurer";
    public static final String OBSTACLE = "obstacle";
    ...
}

or

enum Stimuli {
    INJURER,
    OBSTACLE,
    ...
}

Each GameObject has an attribute called the 'collision stimulus'.

abstract class GameObject {
    abstract Stimuli getCollisionStimulus() ;
}

class Enemy extends GameObject {
    Stimuli getCollisionStimulus()  {
          return Stimuli.INJURER;
    }
}

class Obstacle extends GameObject {
    Stimuli getCollisionStimulus()  {
          return Stimuli.OBSTACLE;
    }
}

For each stimulus registered with it, a GameObject has a different response. I feel this should be a separate map per GameObject subtype.

abstract class GameObject {
    void collisionEffect(GameObject otherObj) {
          this.respondTo(otherObj.getCollisionStimulus();
    }

    abstract void respondTo(Stimuli stimulus);
}

class Player {
    static Map<Stimuli, Effect> responses; 
    static {
        responses.put(Stimuli.INJURER, new GetInjuredEffect());
        responses.put(Stimuli.OBSTACLE, new BlockMovementEffect());
    }

    void respondTo(Stimuli stimulus) {
         Effect effect = responses.get(stimulus);
         effect.run();
    }

    private static class GetInjuredEffect {
        ...
    }

    private static class BlockMovementEffect {
        ...
    }
}

class Enemy {
    static Map<Stimuli, Effect> responses; 
    static {
        responses.put(Stimuli.INJURABLE, new DoInjuryEffect());
        responses.put(Stimuli.PROJECTILE, new VanishIntoThinAirEffect());
    }

    void respondTo(Stimuli stimulus) {
         Effect effect = responses.get(stimulus);
         effect.run();
    }

    private static class DoInjuryEffect {
        ...
    }

    private static class VanishIntoThinAirEffect {
        ...
    }
}

Addition of a new stimulus involves:
- New entry in the Stimuli class/enum.
- New entries in each GameObject subtype's responses map, depending on the relevance of that stimulus to objects of that subtype.

The advantages of this approach:
- Multiple subtypes can provide the same stimulus, so only one mapping is needed in the response maps, not one per subtype.
- Class name changes don't matter anymore, since the response is based on a stimulus. The stimuli names should be chosen carefully so as not to become coupled to one particular class, or become meaningless if new subtypes are introduced.

Do let me know your opinions on this approach, or improvements on it.

EDIT: If you want to store all responses in one central map, then in addition to 'collision stimulus', introduce 'collision subject roles' and a 'collision subject role' attribute per GameObject. Each effect in this central map is still a visible inner class of the particular GameObject subtype it manipulates, so the effects are still encapsulated.

[Istvan Kovacs]

Karthik Shiraly wrote:
A collision involves a Subject who responds to a Stimulant, the response being different for different Stimulus.
So instead of responses based on class name, the response will now be based on the 'collision stimulus' given to it. Each GameObject is modelled as providing a particular kind of stimulus in a collision.

enum Stimuli {
    INJURER,
    OBSTACLE,
    ...
}

I agree with you on separating responsibilities.
The success of your approach depends on how closely stimuli and GameObject subtypes are related. If the mapping is 1:1, you're effectively repeating the subtypes in the enum, making the enum very fragile. If that is the case, Daniel may be better off by mapping the pairs of GameObjects into CollisionObjects with a central map. Fragility of that map may be avoided by filling it in from the GameObject subtypes.
You could create a pair:

class CollisionPair {
  public CollisionPair(Class<? extends GameObject> class1, Class<? extends GameObject> class2) {...}
  private Class<? extends GameObject> oneClass;
  private Class<? extends GameObject> otherClass;
  public boolean equals(Object o) {return true if other is a pair of the same two classes;}
  public int hashCode() {return oneClass.hashCode() ^ otherClass.hashCode();}
}

Then use that as the key in the map.

Daniel Keepfort wrote:the effects depend on the type of both objects, and is different in both objects.

... using a static map ... depends on a type check when making and checking type strings.

I did something like this for the isPhysicalCollision method:
...

	static {
		physicalCollisions.add("Enemy-TerrainSection");
		physicalCollisions.add("Enemy-Obstacle");
		physicalCollisions.add("Enemy-Player");
		physicalCollisions.add("Enemy-Projectile");
		// etc...
	}

It works but is somewhat fragile, and you have to add strings to the list if you add new types.

You can avoid the fragility if the map is managed by the GameObject subclasses:

static {
CollisionMap.put(new CollisionPair(SomeGameObject.class, AnotherGameObject.class), new SomeCollisionEffect(...));
...
}

It does not matter if you add a pair twice, as long as you're consistent.

[Daniel Keepfort]

Thank you for the responses!

Karthik, your solution is interesting as it avoid checking by type, which is exactly what I wanted. Although how would you handle the case if a GameObject has multiple stimulus roles? I can not think of an example of an object with multiple roles for my project though, so this might be the right way to go, still that feels a little limiting. (unless I have misunderstood your suggestion)
I will try to implement this solution.

Istvan, yes, that seems like the more straight forward and flexible solution, and the one I would normally choose. But the tutor of the course the project is for specifically told us to not use any instanceof or getClass, if it's not absolutely neccessary.

[Daniel Keepfort]

I implemented the stimulus approach with a slight modification.
I made the objects stimulus a List<CollisionStimulus> so that an object can have multiple roles. It's working nicely!