This forum has "quote" tags, much like source tags. Using them makes it easier to read your post and figure out where you are responding to others.
Quote:Original post by Storyyeller
// Hint: if something can be described as "temp"
// then it should be a local.
// newP; // newP is a temp vector used in movement calculations
I was going to make it local, but the Move function calls wallCollision, and I thought this would be better then just passing it by reference to every single time I set it up to call wallCollision in the middle becuase it uses interpolated (exact) collision detection for the walls so it needed to know where it would move to before it could finish moving.
I'm not sure I understand. Keeping it as a member and passing it to wallCollision() are functionally equivalent, but the latter saves memory.
Quote:
// Let the particle system handle the wall bounds.
// Just pass the walls you are interested to the particles
// This saves storage
//std::vector<Wall*> bounds;
I'm confused
Its simple: individual particles don't
store the walls. Instead, when you call collide() or move() or whatever, you can pass a reference to the wall container from the calling class.
This saves on memory.
Quote:
// Don't write destructors if you have no resources to release
// ~Particle();
Originally, each particle has a SDL_surface, but I got rid of that and never bothered to get rid of the destructor. I fixed it now.
That is fine.
Quote:
// I have no idea what this function does.
void setType(int);
It gives the particle a radius and mass of the associated element ( 0= Helium, 1 = Neon, 2 = Argon, 3= Krypton, 4 = Xenon)
I would suggest you use an enumerated type, an "enum":
enum ElementType{ HELIUM, NEON, ARGON, KYRPTON, XENON};
I would also take the "ElementType" as a constructor parameter rather than using "setType".
Quote:
// Unless an argument is allowed to be NULL, use a (const) reference.
bool checkCollision(const Particle &);
bool checkCollision(const Wall &);
The problem is that it also mutates both particles in the case of a collision. I guess checkandprocessCollision would be a better name, but it is a bit long
If it mutates the Particle, then just pass a non-const reference. I would probably call it "handleCollision".
Quote:
// Could this x,y,z triplet be represented by a Vector3?
double x;
double y;
double z;
Technically, it could be but there is no logical reason to, because all of the elements are meant to be accessed individually so it would just add an extra layer of indirection for no reason.
This is incorrect, IMO. If the values are logically related, then they should be in a vector. There is only a minor syntactic indirection, but I would argue that is a good thing.
Quote:
// What is a "g" Object? GameObject? Be explicit, especially with type names
// Consider a better name for PhySim (PhysicsSimulator or whatever)
It stands for GUI Object, but you are right about me not being good at naming things.
// "gasp" is a terrible member name. I have no idea what it represents.
Particles particles;
Why give two things the same name (not counting capatilization)? It seemes like a weird practice that might lead to confusion later on.
Well, it is part of my coding style. I always start a type with an uppercase letter. An instance begins with a lowercase letter. This simplifies the naming, unless I need an explicit name I can just use "Type type". So, I have a Renderer called "renderer", anonymous Entities are called "entity" and so on.
Quote:
// Can these be vectors?
// And can they be local?
double x1,x2,y1,y2,z1,z2;
They are the sides of the box. I don't think they can be local because they represent the state of the object. I guess if using vectors were important, the best way would be to create 8 vectors (one for each corner) and use those. But that seems a bit complicated.
If they are part of the objects state then that is acceptable, but this wasn't clear from the names. A box can be represented by two vectors, max and min. So I would use:
Vector3 boundsMax, boundsMin;
Quote:
// And I have no idea what addParticle(42) will do either.
void addParticle(int=0);
It adds a particle at random position and velocity having the specified type
I would again use the "ElementType" enum here, rather than a plain integer.
Quote:
As for the template thing,
is there anyway to make it return a variable where the type depends on the behavior of T's operators?
That sounds like a brittle approach.
Here is a different approach: use non-member functions. These are both more explicit and
E.g:
/* the V3 template, minus operator *() */typedef V3<double> Vector3;typedef V3<Vector3> Matrix33;double dot(const Vector3 &one, const Vector3 &two){ return one.x * two.x + one.y * two.y + one.z * two.z;}Vector3 mul(const Matrix33 &matrix, const Vector3 &vector){ // ...}Vector3 mul(const Vector3 &vector, double scalar){ // ...}
I would be wary of making binary operator * stand for anything other than simple multiplication. The first issue is readability, someone reading your code has to keep in mind your definition of operator *. But there are other problems, for example operator precedence. I've seen people overload some rather
odd operators for dot or cross product. The other issue is operator precedence.
