2
\$\begingroup\$

I have made my first physics engine for simulating circles in a plane. Let me know what you think about it.

//main file
#include "col_resolver.cpp" // collision detector
#include<windows.h>
int main()
{
 int n=0,t=0; // number of bodies(n) and t is the number of instants we are going //to simulate
 int x=0; //misclaneous use
 cin>>n>>t;
 body B[n]; //array of bodies we are dealing with
 for(int h=0;h<n;++h) //for loop to input data
 {
 cin>>B[h].pos.i>>B[h].pos.j>>B[h].vel.i>>B[h].vel.j>>B[h].r>>B[h].m;
 }
 for(int i=0;i<t;++i) //for loop to simulate t times
 {
 for(int j=0;j<n;++j) //for loop to update position of bodies in next instant of time
 {
 pos_update(B[j]);
 }
 for(int k=0;k<n;++k) //for loop to check for collisions
 {
 for(int l=(k+1);l<n;++l) //nested for to detect collisions
 {
 x=col_checker(B[k],B[l]);
 if(x==1)
 {
 col_res(B[k],B[l]); //resolving collisions
 }
 }
 }
 cout<<"\n instant "<<i+1;
 for(int m=0;m<n;++m)
 {
 cout<<"\n position of body "<<m+1<<"\n"<<B[m].pos.i<<" "<<B[m].pos.j; //giving output for each body
 }
 }
 system("pause");
 return 0;
}

//collision resolver
#include"collision_detector.cpp"
using namespace std;
void col_res(body& A, body& B); //function to resolve collision
/*int main() //MAIN FUNCTION FOR DEBUGGING PURPOSE
{
}*/
void col_res(body& A,body& B)
{ 
 vec ua,ub,va,vb; //initial and final velocities of A and B in the rotated coordinate system
 float tan,sin,cos; //variables to hold sin cos and tan values for the angle through which we rotate our axis
 if(A.pos.j==B.pos.j)
 {
 sin=0;
 cos=1;
 }
 else if(A.pos.i==B.pos.i)
 {
 sin=1;
 cos=0;
 }
 else
 {
 tan=(A.pos.j-B.pos.j)/(A.pos.i-B.pos.i); // tan calculation by slope method
 sin=tan/(sqrt(1+(tan*tan)));
 cos=sin/tan;
 }
 //we are now rotating our axes to make the line through centre of the bodies the x axis
 ua.i=(A.vel.i*cos)+(A.vel.j*sin);
 ua.j=(A.vel.j*cos)-(A.vel.i*sin); //components in rotated system
 ub.i=(B.vel.i*cos)+(B.vel.j*sin);
 ub.j=(B.vel.j*cos)-(B.vel.i*sin);
 va.j=ua.j; //collision is along x axis
 vb.j=ub.j;
 va.i=((ua.i*(A.m-B.m))+(2*B.m*ub.i))/(A.m+B.m); //resolving collision along new x axis
 vb.i=((ub.i*(B.m-A.m))+(2*A.m*ua.i))/(A.m+B.m);
 // now we will convert va and vb back to original coordinate system
 A.vel.i=(va.i*cos)-(va.j*sin);
 B.vel.i=(vb.i*cos)-(vb.j*sin);
 A.vel.j=(va.i*sin)+(va.j*cos);
 B.vel.j=(vb.i*sin)+(vb.j*cos);
}

//collision detector
#include "kinematics.cpp"
#include<cmath>
using namespace std;
int col_checker(body, body); //function to check if 2 bodies are colliding
/*int main() //MAIN FUNCTION FOR DEBUGGING
{
 body A,B;
 int a;
 cin>>A.pos.i>>A.pos.j>>B.pos.i>>B.pos.j>>A.r>>B.r;
 a=col_checker(A,B);
 cout<<a;
 return 0;
}*/
int col_checker(body A, body B)
{
 int a; // a is 1 if there is collision and 0 if no collision
 if(sqrt(((A.pos.i-B.pos.i)*(A.pos.i-B.pos.i))+((A.pos.j-B.pos.j)*(A.pos.j-B.pos.j)))<=(A.r+B.r)) //applying distance formula
 {
 a=1;
 }
 else
 {
 a=0;
 }
 return a;
}

//kinematics simulator
#include "vectors.cpp" // vector manager file included
struct body //structure for velocity and position of a rigid circular body
{
 vec pos,vel; // position of centre of circle and velocity vector
 float r,m; //radius of circular body m for mass
};
void pos_update(body&); // function for updating the postion by taking body structure as argument
/*int main() 
{ //MAIN FUNCTION FOR DEBUGGING PURPOSE
 body a;
 cin>>a.pos.i>>a.pos.j>>a.vel.i>>a.vel.j; // taking everyting in input
 a=pos_update(a); // updating position
 cout<<a.pos.i<<" "<<a.pos.j; //giving new position as output
 return 0; 
}*/
void pos_update(body& x) // taking position and velocities as arguments
{
 x.pos=addition(x.pos,x.vel); // adding the 2 vectors because we are considering for unit time
}

// vector manager
#include<iostream>
#include<cmath>
using namespace std;
struct vec // structure for storing a 2d vector 
{
 float i,j,k; // 3 components,i,j and k k component is included to store magnitude of torque
};
vec addition(vec,vec); //addition of 2 vectors
vec subtraction(vec,vec) ; //subtraction of 2 vectors
vec scalarmultiply(float,vec); // multiplying a scalar and vector
float dotproduct(vec,vec) ; //dot product of 2 vectors
vec crossproduct(vec,vec) ; //cross product of 2 vectors
float mag(vec); //magnitude of vector
/*int main()
{
 vec a,b,c;
 int d;
 char o; //operation to be performed
 cin>>o;
 switch(o)
 {
 case 'a': //addion case
 cin>>a.i>>a.j>>b.i>>b.j; // taking inout in sequence
 c=addition(a,b); //performing action
 cout<<c.i<<" "<<c.j; // output
 break;
 case 's': // subtraction case
 cin>>a.i>>a.j>>b.i>>b.j; // taking inout in sequence
 c=subtraction(a,b);
 cout<<c.i<<" "<<c.j;
 break;
 case 'm': //scalar multiply
 cin>>d>>b.i>>b.j;
 c=scalarmultiply(d,b); //MAIN FUNCTION FOR DEBUGGING PURPOSE
 cout<<c.i<<" "<<c.j; // output
 break;
 case 'd': //dotproduct
 cin>>a.i>>a.j>>b.i>>b.j; // taking inout in sequence
 d=dotproduct(a,b);
 cout<<d;
 break;
 case 'c' : //crossproduct
 cin>>a.i>>a.j>>b.i>>b.j; // taking inout in sequence
 c=crossproduct(a,b);
 cout<<c.k;
 break;
 }
 return 0; 
}*/
vec addition(vec a,vec b) //addition function
{
 vec c; //final vector
 c.i=a.i+b.i; // adding 'i' components
 c.j=a.j+b.j; // adding 'j' components
 return c; // returning final vector
}
vec subtraction(vec a,vec b) //subtraction function
{
 vec c;
 c.i=a.i-b.i; //subtracting 'i' components
 c.j=a.j-b.j; //subtracting 'j' components
 return c; //returning final vector
}
vec scalarmultiply(float a, vec b) // multiplying vector by a scalar a is scalar and b is vector
{
 vec c;
 c.i=a*b.i; //formula applied
 c.j=a*b.j;
 return c;
}
float dotproduct(vec a,vec b) //scalar product of 2 vectors, result is a scalar
{
 float c; // resultant scalar
 c=(a.i*b.i)+(a.j*b.j); // application of formula 
 return c;
}
vec crossproduct(vec a,vec b) //vector product of 2 products
{
 vec c;
 c.i=0; // i and j components are 0 as cross product is perpendicular to the plane containing the original vectors
 c.j=0;
 c.k=(a.i*b.j)-(a.j*b.i); //applying formula
 return c;
}
float mag(vec a)
{
 float m;
 m=sqrt((a.i*a.i)+(a.j*a.j));
 return m;
}
πάντα ῥεῖ
5,1524 gold badges23 silver badges32 bronze badges
asked May 12, 2017 at 16:23
\$\endgroup\$
2
  • 2
    \$\begingroup\$ All in all that looks more like c code than c++. \$\endgroup\$ Commented May 12, 2017 at 16:37
  • 2
    \$\begingroup\$ Most of your comments are devoid of sense or just restate the code. In other words, they are worse than useless. \$\endgroup\$ Commented May 12, 2017 at 16:46

2 Answers 2

3
\$\begingroup\$

Holy moly, there is some really mean stuff in here.

To reiterate the obvious that has already been mentioned

  1. Do not include headers
  2. Do no use namespace std;
  3. Encapsulate your data

  4. Lines are cheap so use it. Stuff like float i,j,k; is a bad practice.

  5. Characters are cheap so use them. Names like float i,j,k; are terrible because noone that read some code knows what it is. If you name the variable torque rather than k it is not necessary to write a comment about it and the user sees the purpose whenever the variable is in the code.

  6. Have a look at the standard constructs like operators.

For example you should create a class for your vector:

class myVector {
public: 
 // Default to the standard constructors
 myVector() = default;
 myVector(const myVector&) = default;
 myVector(myVector&&) = default;
 // Defaulting to the standard assignment operators
 myVector& operator=(const myVector&) = default;
 myVector& operator=(myVector&&) = default;
 ~myVector() = default;
 // create the appropriate operators for mathematical operations
 // Addition with a new vector
 myVector operator+(const myVector& otherVec) {
 myVector temp;
 temp.x_coordinate = x_coordinate + otherVec.x_coordinate;
 temp.y_coordinate = y_coordinate + otherVec.y_coordinate;
 return temp;
 }
 // In place addition
 myVector& operator+=(const myVector& otherVec) {
 x_coordinate += otherVec.x_coordinate;
 y_coordinate += otherVec.y_coordinate;
 return *this;
 }
 // Here come the other operations
private:
 double x_coordinate;
 double y_coordinate;
 double torque;
}

Similarly you should create a class for your body

//kinematics simulator
#include "myVector.h"
//structure for velocity and position of a rigid circular body
class myBody {
public:
 myBody() = default;
 myBody(const myBody&) = default;
 myBody(myBody &&) = default;
 // Defaulting to the standard assignment operators
 myBody& operator=(const myBody&) = default;
 myBody& operator=(myBody&&) = default;
 void updatePosition();
 void updateVelocity();
private:
 myVector pos; // Position on the 2D map
 myVector vel; // Velocity
 double r; // Physical radiums of the body
 double m; // Mass of the body
};
answered May 12, 2017 at 19:34
\$\endgroup\$
4
  • \$\begingroup\$ Naturally, you wouldn't explicitly default any function without good reason... \$\endgroup\$ Commented May 12, 2017 at 19:48
  • \$\begingroup\$ For complex classes obviously not, but for one that holds 3 doulbe, zero initialization is kind of save. Clearly there should also be a constructor that takes values \$\endgroup\$ Commented May 12, 2017 at 19:59
  • 1
    \$\begingroup\$ "do not include headers" I guess you meant source files \$\endgroup\$ Commented May 12, 2017 at 20:25
  • \$\begingroup\$ OOps you are obviously right \$\endgroup\$ Commented May 12, 2017 at 21:33
5
\$\begingroup\$

1. Never ever ...

... do this:

#include "col_resolver.cpp"

cpp files aren't intended to be included, but to be compiled separately and linked with your main.cpp.

Separate out function declarations to header files instead, include these, and provide the implementation in translation units (.cpp files).
I'd expect you would get problems with linking, if those sources are managed in a project file of your IDE.

2. Don't use using namespace std;

While that would work in your particular case, it's considered bad practice. Especially when you move out your code to separate header files.

See more details here please:

Why is "using namespace std;" considered bad practice?

3. Use classes to encapsulate your data

Stuff like that

struct body //structure for velocity and position of a rigid circular body
{
 vec pos,vel; // position of centre of circle and velocity vector
 float r,m; //radius of circular body m for mass
};
void pos_update(body&); // function for updating the postion by taking body structure as argument
// ...
void pos_update(body& x) // taking position and velocities as arguments
{
 x.pos=addition(x.pos,x.vel); // adding the 2 vectors because we are considering for unit time
}

should be placed in a class with appropriate member functions instead:

class body
{
 vec pos,vel; // position of centre of circle and velocity vector
 float r,m; //radius of circular body m for mass
public:
 void pos_update(); // function for updating the postion by taking body structure as argument
};
// ...
void body::pos_update() // taking position and velocities as arguments
{
 pos=addition(pos,vel); // adding the 2 vectors because we are considering for unit time
}

4. Don't comment the obvious

The comment here (and in other places)

 x.pos=addition(x.pos,x.vel); // adding the 2 vectors because 

is useless. It's quite ovious what that line of code does.

Rather use comments to document design decisions, that aren't obvious for future readers/maintainers of your code.


Conclusion

Let me know what you think about it.

All in all I believe that your implementation isn't acceptable and valid c++ code.
It looks more like c code that is "furnished up" with some basic c++ standard library functionality.

Though even with the c language you would obviously miss the point of #include statements, and separating declaration done in header files, and definition provided in translation units which will be stiched together in the linking proces.

The best advice I can give is to pick a book from this list and start to learn the basics.
An overworked c++ style code would look fairly different from what you've been presenting here.

answered May 12, 2017 at 16:27
\$\endgroup\$
3
  • 2
    \$\begingroup\$ No matther who writes a C++ review, I always read the same sentence there: don't use using namespace std; I think it should be automatically posted as soon as you add the c++ tag ;-) \$\endgroup\$ Commented May 12, 2017 at 19:42
  • 1
    \$\begingroup\$ @t3chb0t I've been copying that part from one of my earlier answers of course ;-) \$\endgroup\$ Commented May 12, 2017 at 20:13
  • \$\begingroup\$ Also, use an existing, well tested linear algebra library like "eigen" instead of writing out all the operations manually. \$\endgroup\$ Commented May 12, 2017 at 23:31

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.