HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Design of physics with Collision Detection using SFML

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
collisionsfmlphysicswithdesignusingdetection

Problem

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 physicsObjects = new List();
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))
{

Solution

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

public double Mass { get; set; }


-
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.

-
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

-
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 -=.

-
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);
}


-
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);

Code Snippets

public double Mass { get; set; }
public void AddForce(double x, double y)
{
    forceX += x;
    forceY += y;
}
internal Boolean CollidesWith(RigidObject obj)
{
     return obj.GetType() == typeof(RigidCircle) && isCollisionWith(obj as RigidCircle);
}
internal Boolean CollidesWith(RigidObject obj)
{
     return 
         obj.GetType() == typeof(RigidCircle) && isCollisionWith(obj as RigidCircle) ||
         obj.GetType() == typeof(RigidRectangle) && isCollisionWith(obj as RigidRectangle);
}
double _keyboard_force = 1.5F;

Context

StackExchange Code Review Q#45353, answer score: 6

Revisions (0)

No revisions yet.