Is there an OOP way of doing this?

Newbie & Debugging Questions
#1

The following describes something that happens in my game:

Projectiles should apply effects to whatever character collides with them.

Here the code I have:


for(Collision collision : rigidBody.getCollisions())
{
 if(collision.collider instanceof Character)
 {
  Character character = (Character) collision.collider;

  character.getEffects()
               .applyAll(effects);
 }
}

I am not too fond of instanceof. However, I cannot think of any satisfying way of doing this. I even googled how to replace instanceof but all the examples were rudimentary and did not apply to this situation.

Here of some other ways I thought of doing it and why I dislike them:

  1. Entity type enum —
    Just a poor man’s version of instanceof.

for(Collision collision : rigidBody.getCollisions())
{
 if(collision.collider.getEntityType() == EntityType.CHARACTER)
 {
  Character character = (Character) collision.collider;

  character.getEffects().applyAll(effects);
 }
}
  1. Keep a list of all characters —
    This also does not scale well because I could keep doing this and have 10-20 (maybe exaggerated) lists for each subclass. Also, my scene, which would store these list, does not care what the subclass is. However, the fields of my scene do need to know, making this complicated.

for(Collision collision : rigidBody.getCollisions())
{
 if(scene.getCharacters().contains(collision.collider)
 { 
  Character character = (Character) collision.collider;

  character.getEffects().applyAll(effects);
 }
}
  1. Keep a list of all characters #2—
    This is just a reverse way of the previous example that does not require casting.

for(Character character : scene.getCharacters())
{
 if(rigidBody.getCollisions().contains(character))
  character.getEffects().applyAll(effects);
}
#2

How exactly is

rigidBody.getCollisions()

calculated? What exactly does it contain?

If you need to know which characters are colliding with

rigidBody

, why don’t you have something like this:


for(CharacterCollision collision : rigidBody.getCharacterCollisions())
{
  collision.getCharacter().getEffects().applyAll(effects);
}

I’m also a little suspicious of why you need to keep around a collection of what’s colliding instead of doing applying the effects whenever you detect the collision, but in any case I think you should just go with whatever makes the most sense to you.

#3

@Kevin

I store collisions because I want to handle all logic in my update method. 'Tis also the reason why I don’t use observers. Both, observers and polling, work in this scenario. Observers, however, tend to be slightly harder to follow because they have no linear order.

'Tis not much different from my #2 and #3. Similarly, I want to minimize coupling of my character logic and physics logic. TBH, I would rather couple my scene and characters than my physics and characters because neither rigid body nor its fields need to know about characters.

#4

A modified #2 looks like a viable and - very good - simple solution.
Except using enhanced loops maybe.

#5

You don’t say where “effects” comes from. However, I would use an interface like ICollideable:-

interface ICollideable {
    void applyAll(Effects[] effects); // Maybe give it a more appropriate name and signature depending on how it's used.
}

then use it such as:-


for(Collision collision : rigidBody.getCollisions())
{
 if(collision.collider instanceof ICollideable)
 {
   ICollideable collideable = (ICollideable) collision.collider;
   collideable.applyAll(effects);
 }
}

The, for every entity that should do something when collided with (i.e Character), implement the interface.

#6

@SteveSmith
It should not matter where effects come from. But if you are curious, Effects is just a class that handles how my characters receive effects. Normally, I would keep this logic in my Character class but the class was getting long and so I encapsulated some logic in a field. A good a metaphor for this is a flash drive. It works like it was always there, a part a memory, but it is an addition to the system. I’m sure there is a design pattern for this.

Also, your solution is just a not so much different way of my initial method.

Anyway, here is what I decided to do


private void handleCharacterCollisions()
{
	for(Character character : scene.getCharacters())
	{
		if(collisionWith(character))
			character.getEffects().applyAll(effects);
	}
}
	
private boolean collisionWith(Character character)
{
	for(Collision collision : rigidBody.getCollisions())
	{
		if(collision.collided == character.getRigidBody())
			return true;
	}
		
	return false;
}

After some research, I found that an, possibly the only, OOP/casting-less way to do this is to use some form of the Visitor pattern. You simply find some way of handle all relevant subtypes instead of the abstracted supertypes.

I also believe that core problem stems from my code needing to search elsewhere for data. Ideal code, however, has all necessary data in the fields of the class or in the arguments of the method and so casting is never necessary. I am also starting to believe that I am using inheritance wrong. I do not think code in ANY project should store objects as their supertype and then rely on the specialized methods or data of the subtypes.

#7

Lots of looping now when always checking each character instead of only processing actual collisions.

#8

Hardly. I cull my collisions. The average is def less than 1. Similarly, there will not be that many characters. The most is prob around 50. Average is prob 10, maybe 20. This particular code only runs for projectiles too and so its running even less.

Also, i can optimize later.

#9

Ok, let’s say it like this: in opposite to your first solutions you now have one that is neither simple nor does it solve the original underlying design problems but adds a new (having to loop all characters to match a collision instead of a direct access).

#10

By using an interface, you can get any class (subtype or supertype) to handle the collisions by implementing the interface, and you will only need one “if (instanceof…)” line. (I’m assuming that other classes, not just Character, will need handle collisions).

#11

I think it is pretty simple. Its almost exactly the same as the description. Also, I said that I don’t want to couple physics with character logic.

#12

Ok but what if I want to do something else after a collision. Do I create another interface? No. Observers and polling make much more sense.

#13

Might be worth looking at how Box2D handles collisions: http://www.iforce2d.net/b2dtut/collision-callbacks

#14

It seems like you didn’t really want any advice, as you’ve shot down every idea that’s been presented pretty abruptly. It’s completely fine to do things your own way (in fact I think you should do whatever makes the most sense to you), but if what you really want to do is explain your approach to us instead of take suggestions, then maybe next time just make a post outlining your approach instead of asking for input. :stuck_out_tongue:

Judging from your other replies, you probably don’t want to hear any feedback, but I’ll just quickly note that you shouldn’t get too hung up on “the OOP way” or “design patterns” before you really understand what they mean. Write code that works and makes sense to you.

Anyway, it sounds like you’re having fun working on your code, and that’s what matters. Good luck, and happy coding.

#15

I understand the problem as I have encountered it before too.

I would either add a default method called “processCollision” if Collision is an interface or a normal method if it is an abstract class / class and leave it by default empty. Ie by default nothing is done. I would then override this method for Character only and put your effect code logic there.

I have to agree with Kevin a bit that it is a little bit confusing as to what constraints you are trying to put on yourself in the design. If you don’t want any code to be in character at all then you are making things complicated on yourself because there is no pretty way to filter for specific sub classes. Overloading won’t work because you can’t overload on a super class and a sub class as your sub class satisfies both method signatures. At this stage you have run out of OOP ways of doing things and you are left with the ugly solutions you have suggested.