Design of physics with Collision Detection using SFML

Question

I found out that in Visual Studio 2012, it is possible to create project of SFML easily with a template. I am not an experienced C# programmer. Hence I wanted to implement a physics component just for practice purposes.

PhysicsManager Class contains all the physical objects:

public sealed class PhysicsManager
{
    // Thread-Safe Singleton Pattern Definition
    private static volatile PhysicsManager instance = new PhysicsManager();
    private static object syncRoot = new Object();

    private PhysicsManager() { }

    public static PhysicsManager Instance
    {
        get
        {
            if (instance == null)
            {
                lock (syncRoot)
                {
                    if (instance == null)
                        instance = new PhysicsManager();
                }
            }
            return instance;
        }
    }
    ///////////////////////////////////////////////

    private List<RigidObject> physicsObjects = new List<RigidObject>();
    private double _environmentFriction = 0.6F;
    public double EnvironmentFriction
    {
        get
        {
            return this._environmentFriction;
        }
        set
        {
            this._environmentFriction = value;
            RigidObject.friction = value;
        }
    }

    public void AddRigidObject(RigidObject obj)
    {
        physicsObjects.Add(obj);
    }

    public void RemoveRigidObject(RigidObject obj)
    {
        physicsObjects.Remove(obj);
    }

    public void Update()
    {
        foreach(var rigit in physicsObjects)
        {
            rigit.UpdatePhysics();
        }

        // TODO: Collision Detection and Response (Should be refactored into a separate class).
        for (int n = 0; n < physicsObjects.Count; ++n)
        {
            RigidObject r = physicsObjects[n];
            for (int m = n+1; m < physicsObjects.Count; ++m)
            {
                RigidObject o = physicsObjects[m];

                if (r.isCollisionWith(o))
                {
                    r.HandleCollision();
                    o.HandleCollision();
                }                      
            }
        }
    }

}

RigidObject Abstract Class where the friction, force, and movement of physical objects are implemented:

abstract public class RigidObject
{
    // Environmental Friction
    static internal double friction = 0.6F;

    private double forceX = 0;
    private double forceY = 0;
    private double accelerationX = 0;
    private double accelerationY = 0;
    protected double velocityX = 0;
    protected double velocityY = 0;
    protected Vector2f Position = new Vector2f(0,0);
    protected Vector2f prevPosition = new Vector2f(0, 0);

    private double _mass = 10F;
    public double Mass
    {
        get
        {
            return this._mass;
        }
        set
        {
            this._mass = value;
        }
    }

    public void AddForce(double X, double Y)
    {
        forceX = forceX + X;
        forceY = forceY + Y;
    }

    internal void UpdatePhysics()
    {
        double frictionX = 0;
        double frictionY = 0;

        // Adjust friction according to velocity
        if (velocityX > 0) frictionX = friction;
        else if (velocityX < 0) frictionX = -friction;
        else frictionX = 0; // velocityX == 0

        if (velocityY > 0) frictionY = friction;
        else if (velocityY < 0) frictionY = -friction;
        else frictionY = 0; // velocityY == 0

        // Environment Friction effects forces on rigid body
        forceX = forceX - frictionX;
        forceY = forceY - frictionY;

        accelerationX = forceX / _mass;
        accelerationY = forceY / _mass;

        velocityX = velocityX + accelerationX;
        velocityY = velocityY + accelerationY;

        // Saves the previous position
        prevPosition = Position;

        // Move the rigid body according to velocity
        Position = new Vector2f(Position.X + (float)velocityX, Position.Y + (float)velocityY);
        DelegatePosition(Position);

        // Reset Forces
        forceX = 0;
        forceY = 0;
    }

    protected void Move(float PosX, float PosY)
    {
        Position = new Vector2f(Position.X + PosX, Position.Y + PosY);
        DelegatePosition(Position);
    }

    protected void MoveTo(Vector2f pos)
    {
        Position = pos;
        DelegatePosition(Position);
    }

    abstract protected void DelegatePosition(Vector2f Pos);


    internal Boolean isCollisionWith(RigidObject Obj)
    {
        if (Obj.GetType() == typeof(RigidCircle))
            return isCollisionWith(Obj as RigidCircle);

        return false;
    }

    abstract internal Boolean isCollisionWith(RigidCircle Obj);
    abstract internal void HandleCollision();
}

RigidCircle Class inherits RigidObject and contains CircleShape (class of SFML):

public class RigidCircle : RigidObject
{
    private CircleShape _baseShape;
    public Color FillColor
    {
        get
        {
            return _baseShape.FillColor;
        }
        set
        {
            _baseShape.FillColor = value;
        }
    }

    public RigidCircle(float PosX, float PosY, double mass, float radius)
    {
        _baseShape = new CircleShape(radius);
        Position = new Vector2f(PosX - radius, PosY - radius);
        _baseShape.Position = Position;
    }

    public CircleShape getShape()
    {
        return _baseShape;
    }

    public void SetPointCount(uint limit)
    {
        _baseShape.SetPointCount(limit);
    }

    protected override void DelegatePosition(Vector2f Pos)
    {
        _baseShape.Position = Pos;
    }

    internal override Boolean isCollisionWith(RigidCircle Obj)
    {
        // TODO: Needs restructuring (SFML Position means top-left corner, we needed center of circles)
        Vector2f distanceVec = Obj._baseShape.Position - _baseShape.Position;
        float radiusDiff = Obj._baseShape.Radius - _baseShape.Radius;
        distanceVec = distanceVec + new Vector2f(radiusDiff, radiusDiff); 
        double distance = distanceVec.X * distanceVec.X + distanceVec.Y * distanceVec.Y;
        double totalRadius = _baseShape.Radius + Obj._baseShape.Radius;
        totalRadius = totalRadius * totalRadius;

        return (distance < totalRadius)? true : false;
    }

    internal override void HandleCollision()
    {
        Position = prevPosition;
        _baseShape.Position = prevPosition;

        velocityX = -velocityX;
        velocityY = -velocityY;
    }
}

Program where the main loop is implemented:

public static class Program
{
    private static readonly Color CornflowerBlue = new Color(100, 149, 237);

    private static PhysicsManager Physics = PhysicsManager.Instance;

    public static void Main(string[] args)
    {
        ContextSettings settings = new ContextSettings();
        settings.AntialiasingLevel = 4;           

        RenderWindow window = new RenderWindow(new VideoMode(1280, 720), "SFML Window", Styles.Default, settings);
        window.Closed += (sender, eventArgs) => window.Close();
        window.SetFramerateLimit(24);

        Physics.EnvironmentFriction = 0.4F;

        // Scene Setup
        float radiusM = 80.0F;
        Vector2f Pos = window.GetView().Size / 2;
        RigidCircle MainCircle = new RigidCircle(radiusM, radiusM, 2.5F, radiusM);
        MainCircle.FillColor = new Color(100, 250, 50);
        MainCircle.SetPointCount(30);

        float radiusS = 50.0F;
        RigidCircle StaticCircle = new RigidCircle(Pos.X, Pos.Y, 2.5F, radiusS);
        StaticCircle.FillColor = new Color(50, 50, 50);
        StaticCircle.SetPointCount(25);

        Physics.AddRigidObject(StaticCircle);
        Physics.AddRigidObject(MainCircle);

        while (window.IsOpen())
        {
            window.DispatchEvents();

            // TODO: Insert Update Code Here
            if (Keyboard.IsKeyPressed(Keyboard.Key.Down))
                MainCircle.AddForce(0, 1.5F);
            if (Keyboard.IsKeyPressed(Keyboard.Key.Up))
                MainCircle.AddForce(0, -1.5F);
            if (Keyboard.IsKeyPressed(Keyboard.Key.Right))
                MainCircle.AddForce(1.5F, 0);
            if (Keyboard.IsKeyPressed(Keyboard.Key.Left))
                MainCircle.AddForce(-1.5F, 0);


            Physics.Update();

            window.Clear(CornflowerBlue);

            // TODO: Insert Draw Code Here
            window.Draw(StaticCircle.getShape());
            window.Draw(MainCircle.getShape());
            window.Display();
        }
    }
}

This runs correctly as one big green circle that can be controlled by arrow keys and a small gray circle that has static position.

My questions are:

1. In one of my classes I wanted to inherit from CircleShape but I couldn't since the class inherits another class (abstract), so I decided to contain a CircleShape. But it doesn't feel right in terms of software design. Is there a better way to design RigidObject, RigidCircle classes hence I don't need to write all methods and delegate them to CircleShape object inside RigidCircle?

2. Is there a neater way to implement Collision Detection and Response so such that it will allow me to add more rigid classes derived from RigidObject and customize the collision response? (For example having a default response for each concrete class but it will allow to customize behavior for each object easily.)

Answer 1

1. Use auto-properties when you can like in RigidObject and set default values in the constructor.

   public double Mass { get; set; }
   

2. I prefer to keep my private and protected members named with prefix underscore, and my method variables just lowercase. I saw you do some of this, and some not... it is good to keep this consistent.

3. While I'm on naming and capitalization. Method names should be PascalCase not camelCase (start with a capital letter), this is especially true when the method is public

4. So in a method like this, I would reccommend lowering the case of those xy's and using +=.

   public void AddForce(double x, double y)
   {
       forceX += x;
       forceY += y;
   }
   

   Same goes for later in your UpdatePhysics use -=.

5. Unless you plan on adding more logic to this method, you can tune this down to a single statement. As you can see, if the first condition is false, the statement will be immediately broken and return false, only if true will the next condition be checked as this is the nature of && vs &. Now the result of second condition will be anded with true, which will simply keep it the same, and the value will be returned.

   Edit: Note that I also renamed this method to start with a verb, which is common practice, typically when something starts with is, it is a property or some state on an object.

   internal Boolean CollidesWith(RigidObject obj)
   {
        return obj.GetType() == typeof(RigidCircle) && isCollisionWith(obj as RigidCircle);
   }
   

   If you do indeed want to add more rigid objects that this could collide with, you can always append this statement to include them.

   internal Boolean CollidesWith(RigidObject obj)
   {
        return 
            obj.GetType() == typeof(RigidCircle) && isCollisionWith(obj as RigidCircle) ||
            obj.GetType() == typeof(RigidRectangle) && isCollisionWith(obj as RigidRectangle);
   }
   

6. As Malachi pointed out, you should indeed be using vectors for force direction. However, similarly to Malachi's point, you should be using a set value instead of a magic number. You should have some member something like this

   double _keyboard_force = 1.5F;
   

   Then in the case of your code when you apply the force.

   if (Keyboard.IsKeyPressed(Keyboard.Key.Left))
       MainCircle.AddForce( -1F * _keyboard_force , 0);
   

Answer 2

for this little bit of code here

// TODO: Insert Update Code Here
if (Keyboard.IsKeyPressed(Keyboard.Key.Down))
    MainCircle.AddForce(0, 1.5F);
if (Keyboard.IsKeyPressed(Keyboard.Key.Up))
    MainCircle.AddForce(0, -1.5F);
if (Keyboard.IsKeyPressed(Keyboard.Key.Right))
    MainCircle.AddForce(1.5F, 0);
if (Keyboard.IsKeyPressed(Keyboard.Key.Left))
    MainCircle.AddForce(-1.5F, 0);

I was thinking that you might be able to use a case statement here instead of all these if statements.

it has been a while since I have used the Keyboard Methods. but I think it would be better for maintenance if you used a Switch then you would be able to add other keys if you wanted to.

this might take creating a method or function that will capture the KeyPressed Event for any Key on the keyboard and then return the key that was pressed. this would allow you to use it in a switch statement, which would make this bit of code much more maintainable.

---

I assume that your .AddForce(x,y) is a really asking for a vector (at least that is what it would be in XNA I think)

These look like you magic numbers, I would probably create some variables to hold these up, down, left, right numbers/Vectors/Force values, then you know which goes where and it would probably be easier to maintain.

If you did it like this you would probably only need two variables: up, right then when you want the down value, you just add a negative, same with left value. At least I think it should work that way.

---

I know that a lot of this review is telling you to "do more now to make it easier later", but I think it is important, especially in a game like this, these variables could be at a higher scope so that you can use them (up,down,right,left) anywhere in your code.