🔒 Locked 🎮 Unity

changed to: equating two classes

Started by daniel_i_l Aug 14, 2006 at 2:00 PM 21 replies 2.1k views

Original Post

Aug 14, 2006 at 2:00 PM

I have two classes that look like this:


class RigidBody;
class Force{
public:
	Force(){}
	Force(Vector2D a, Vector2D b, double k, 
              double d, double l, RigidBody &r)
	{
	    AncorPoint       = a; 
	    PointInBodySpace = b;
	    SpringConstant   = k;
	    Damping          = d;
	    RestLength       = l;
	    OwnerOfPoint     = r;
	}
	RigidBody  &OwnerOfPoint
	Vector2D AncorPoint;
	Vector2D PointInBodySpace;
	Vector2D PointInWorldSpace;
	double   SpringConstant;
	double   Damping;
	double   RestLength;

	void     TranslatePoints();
	
	Vector2D CalculateForce();
	double CalculateTorque();
};


class RigidBody{
private:
	vector<Vector2D> m_CornersInWorldSpace;
	vector<Vector2D> m_CornersInBodySpace;
	double           m_Size;

public:
	RigidBody(){}
	RigidBody(Vector2D p, Vector2D v, double m, double s);
	void UpdateGeometry();
	void WorldTransform(vector<Vector2D> points);
	void WorldTransform(Vector2D point);
	State            m_MyState;
	vector<Vector2D> GetCorners(){return m_CornersInWorldSpace;}
	Force  ForceOnBody; 
};

The force class holds all the information needed for a spring, functions to calculate force and torque, and a function to update the points world space. The rigid body has instance of the force class and it calls it to aply force. I want the Force class to have a referance to the body that the point is in so that it can update the point position according to the movement of the body. But I get an this error on the constructor: OwnerOfPoint' : must be initialized in constructor base/member initializer list and when I do OwnerOfPoint = r I get: c:\documents and settings\daniel lowengrub\my documents\visual studio 2005\projects\win\nehegl\RigidBody.h(21) : error C2582: 'operator = ' function is unavailable in 'RigidBody' Also, inside the RigidBody ctor can I initialize a Force with: Force(.....,this); - can I use this to pass a referance? Thanks [Edited by - daniel_i_l on August 15, 2006 2:14:10 AM]

SiCrane

Mod

473

SiCrane

Aug 14, 2006 at 2:03 PM

The error message tells you how to fix the problem: the constructor needs to use the member initialization list in order to initialize the reference. Ex:

	Force(Vector2D a, Vector2D b, double k,               double d, double l, RigidBody &r)	  : AncorPoint(a),	    PointInBodySpace(b),	    SpringConstant(k),	    Damping(d),	    RestLength(l),	    OwnerOfPoint(r)        {}

I have no idea what you're asking for the second part.

daniel_i_l

Author

daniel_i_l

Aug 14, 2006 at 2:12 PM

Thanks,
I did:
Force(Vector2D a, Vector2D b, double k,
double d, double l, RigidBody &r)
:AncorPoint(a),
PointInBodySpace(b),
SpringConstant(k),
Damping(d),
RestLength(l),
OwnerOfPoint(r)
{}
and got the same error?

In the second part I'm asking, if I'm inside the RigidBody ctor, how do I pass a referance of myself?
I tries "this" and got an error saying that I was passing the wtong type.
Thanks.

Aardvajk

1,172

Aardvajk

Aug 14, 2006 at 2:37 PM

Question 2: Try (*this) wherever you are trying to pass a reference to yourself, although beware doing this in a constructor as you may not have finished constructing the object fully.

Question 1: You can't be getting the same error. If you are getting an error, you must be getting a different error caused by something else.

Nitage

125

Nitage

Aug 14, 2006 at 2:42 PM

You need to initialise OwnerOfPoint in your default c'tor (Force(){}) as well.

daniel_i_l

Author

daniel_i_l

Aug 14, 2006 at 2:48 PM

Nitage: how do I do that?
Thanks

Maybe I'll ask a more general question:
I have the RigidBody class that holds info about a RB, it also has to hold a force or list of forces (I'm starting with one) acting on the body. But each force has to be able to access the body that it's acting on so that it knows were to put the ends of the spring. Whats a good way to design this?

NotAYakk

Aug 14, 2006 at 4:54 PM

Here is a short answer:

Don't use references in structs or classes. Use pointers instead.

References should only be used in structs/classes in rare instances and only if you really know what you are doing.

RigidBody  &OwnerOfPoint

Change the reference to a pointer:

RigidBody  *OwnerOfPoint;

Aardvajk

1,172

Aardvajk

Aug 14, 2006 at 5:14 PM

You want a class called RigidBody, with a list of forces, but the forces need to be aware of the RigidBody they belong to? Not sure if this is the best design in the first place, but you could implement it a bit like this:

stuff.h

class Force;class RigidBody{private:    vector<Force*> Forces;public:    RigidBody() { }    ~RigidBody();    void AddForce(Force *F);    void ApplyForces();};class Force{private:    RigidBody *RB; int Strength;public:    Force(int St) : Strength(St) { }    virtual ~Force(){ }    void SetRigidBody(RigidBody *R){ RB=R; }    virtual void Apply();};

stuff.cpp

RigidBody::~RigidBody(){    for(int i=0;i<Forces.size();++i) delete Forces;    // this assumes that the RigidBody "owns" the Forces passed to it}void RigidBody::AddForce(Force *F){    F->SetRigidBody(this);    Forces.push_back(F);}void RigidBody::ApplyForces(){    for(int i=0;i<Forces.size();++i) Forces->Apply();}void Force::Apply(){    // do whatever to RigidBody via RB->Whatever}

main.cpp

#include "stuff.h"class SpecialForce : public Force{public:    SpecialForce(int St) : Force(St) { }    void Apply(){ } // specialised apply, using RB->Members()};int main(){   RigidBody R;   R.AddForce(new Force(23));   R.AddForce(new SpecialForce(26));   R.ApplyForces();}

Not exactly going to win any deisgn theory awards but that's a reasonably simple and extensible solution. Alternatively, you could remove the RigidBody member pointer from the Forces class and instead pass it to each Force item via the Apply call, since this is called from the RigidBody ApplyForces function so would just be a case of

void Force::Apply(RigidBody *RB){ ... }

void RigidBody::ApplyForces()
{
for(blah blah) Forces->Apply(this)
}

although if I was going to have the RigidBody "own" the Force objects, I wouldn't do it like that personally since it would present an interface that the wrong RigidBody could be passed to.

From that point of view, it might be worth making SetRigidBody a private member of Force and making RigidBody a friend of force, so you couldn't even set the wrong RigidBody member of a Force, but I always get shouted at for suggesting using friends so don't take my word for it [smile].

daniel_i_l

Author

daniel_i_l

Aug 15, 2006 at 2:20 AM

Thanks everyone!
I did what you guys suggested and made OwnerOfPoint a class. Now another problem cropped up. In my integrater (RK4) I have this function:

Derivative CSimulation::Evaluate(RigidBody &initial, double dt, Derivative &d){    State      state;	RigidBody  body;	Derivative output;	body = initial;    	body.m_MyState.Position = initial.m_MyState.Position + d.dPosition * dt;	body.m_MyState.Velocity = initial.m_MyState.Velocity + d.dVelocity * dt;	body.m_MyState.Rotation        = initial.m_MyState.Rotation                                                 + d.dRotation        * dt;	body.m_MyState.AngularVelocity = initial.m_MyState.AngularVelocity                                          + d.dAngularVelocity * dt;	body.m_MyState.Mass = initial.m_MyState.Mass;	body.m_MyState.MomentOfInertia = initial.m_MyState.MomentOfInertia;	body.ApplyForces(); 	output.dPosition = body.m_MyState.Velocity;	output.dVelocity = Acceleration(body);	output.dRotation        = body.m_MyState.AngularVelocity;	output.dAngularVelocity = AngularAcceleration(body);    return output;}void CSimulation::Integrate(RigidBody &body, double dt){	Derivative a = Evaluate(body);	Derivative b = Evaluate(body, dt*0.5, a);	Derivative c = Evaluate(body, dt*0.5, b);	Derivative d = Evaluate(body, dt,     c);...}

My program crashed and I noticed that it crashed on this line:
body.ApplyForces();
when I checked it I found that in thr Integrate function, after calling Evaluate(..) for Deravitive b, the pointer to the force - body.Force[0] wasn't pointing to anything! Why?
Thanks!
EDIT: one more thing, when I was playing with the debugger I saw that a RigidBody has a pointer to a Force which has a pointer to the RigidBody that that has a pointer to the Force.... a sort of infinate loop of pointing. Is this a problem?
Thanks.

[Edited by - daniel_i_l on August 15, 2006 3:20:37 AM]

Aardvajk

1,172

Aardvajk

Aug 15, 2006 at 5:54 AM

I guess that you have set RigidBody up to own the Forces and are deleting the vector in RigidBody's destructor.

When you assign initial to body, whatever container you have stored the Force pointers in will also be copied into body, so the pointers will now be stored in both classes.

When body goes out of scope at the end of the function, the destructor will delete all the Forces owned by body, which will be the same pointers as those owned by initial.

So when you call the second Evaluate, the Forces owned by the body have already been deleted.

To solve this, you need to decide whether you want the Forces owned by Body. If you do, you will need to write an assignment operator for Body that does a proper copy of the Forces - i.e. creates a new vector, allocates new Forces and copies their details in so each body has its own copy of the Forces.

In terms of the second question, no this will not cause a problem as long as you don't do something silly like:

Body::Call()
{
Forces[0]->Call();
}

Force::Call()
{
BodyPointer->Call();
}

HTH Paul

daniel_i_l

Author

daniel_i_l

Aug 15, 2006 at 5:58 AM

Thanks a lot for that explanation. So what should I do to stop the pointer from getting deleted? Should I do:
{
...
body = new RigidBody(initial);
...
delete body;
}
Thanks.

Aardvajk

1,172

Aardvajk

Aug 15, 2006 at 6:28 AM

No. That will have exactly the same effect.

What you'll need to do is write a proper assignment operator for Body a bit like:

class RigidBody{    const RigidBody &operator=(const RigidBody &C);};const RigidBody &RigidBody::operator=(const RigidBody &C){    // copy everything else    Forces.clear();    for(int I=0;I<C.Forces.size();++I)        {        Force *F=new Force;        // copy C force details into F        // better to write an assignment op for Force as well so        // you can just to *F=*(C) but beware of the RigidBody        // pointer        Forces.push_back(F);        }    return *this;}

This will mean that when you do body=initial, body will have its own copies of the force objects and can safely delete these when it goes out of scope without affecting initial's forces.

Do bear in mind though that if you want to be able to do

RigidBody Body=Initial;

or

RigidBody *Body=new RigidBody(Initial);

you will also need to write a copy constructor like:

RigidBody::RigidBody(const RigidBody &B)
{
}

that does a similar thing but without clearing the Forces vector first.

So, in summary

RigidBody Body=Initial; // calls copy constructor
RigidBody *Body=new RigidBody(Initial); // calls copy constructor

RigidBody Body;
Body=Initial; // calls assignment operator

You know, thinking about it, it might be a lot easier to just have RigidBody have a vector of actual Force objects rather than pointers:

vector Forces;

and just push actual objects onto this vector. The default vector copy will then work as you wish, assuming Force has a copy constructor, and you don't have to worry about all the allocation issues. If the RigidBody owns the Forces this is probably a better solution really.

daniel_i_l

Author

daniel_i_l

Aug 15, 2006 at 6:50 AM

Hey, Thanks a lot Easily Confused (you're anything but confused;))!! I did what you said in the end and it ran fine! one question, in the end you said:
"assuming Force has a copy constructor" do I need to make one or is the default one ok?
Thanks a lot - you really helped me!

Aardvajk

1,172

Aardvajk

Aug 15, 2006 at 8:05 AM

You're welcome.

The answer to that question depends on whether or not Force contains any dynamically allocated data or not.

With the example above, you need to write your own copy constructor because you have raw pointers in the class and the default copy constructor will just copy the pointers which is how you got into trouble.

If your Force class just contains simple data types, or any dynamic data is wrapped in another class whose copy constructor et al handle the correct copying behaviour, like std::string for example, then the default copy constructor will do fine.

The copy constructor just copies each member as if you had done:

memberA=memberB;

in code somewhere, so for an integer, or a std::string etc where the wrapping class takes care of it, this is not a problem. If you had a char *data member that pointed to some allocated data, the default copy constructor would do

data=Copy.data;

which would obviously just copy the pointer and you end up with two pointers pointing at the same memory.

daniel_i_l

Author

daniel_i_l

Aug 15, 2006 at 8:30 AM

Is this good?

Force::Force (const Force &F){	AncorPoint = F.AncorPoint;	Damping    = F.Damping;	PointInBodySpace  = F.PointInBodySpace;	PointInWorldSpace = F.PointInWorldSpace;	RestLength  =F.RestLength;	SpringConstant = F.SpringConstant;		RigidBody *B=new RigidBody;	B = F.OwnerOfPoint;	OwnerOfPoint = B;}

and I have another little question. now RigidBody is passing a pointer of itself to Force. Does this pointer ever have to get deleted? And if the code above is right, where do I delete the new rigid body?
Thanks.

Aardvajk

1,172

Aardvajk

Aug 15, 2006 at 8:42 AM

Probably not [smile].

You are allocating a new RigidBody object inside the constructor for a Force object. I very much doubt this is what you want. With the code previously in this post, the RigidBody owner of the Force was being assigned when the Force was added to the RigidBody.

I guess what you want to do is set the OwnerOfPoint to NULL in the Force copy constructor, then assign the correct owner to this when you add the Force object to the RigidBody.

Under this system, you would not have to delete the RigidBody pointer in the Force object since this pointer does not declare that Force owns the RigidBody, just that it can reference it.

void f(){    RigidBody B;    Force *F=new Force(); // F->OwnerOfPoint is NULL at this point    B.AddForce(F); // F->OwnerOfPoint is set to &B in here, but F does not                   // own B, so would not delete it    // end of scope, B is deleted here automatically and in its destructor will    // delete the object pointed to by F above}

daniel_i_l

Author

daniel_i_l

Aug 15, 2006 at 8:56 AM

I changed the last 3 lines to:
OwnerOfPoint = NULL;
and my program crashed, from the debugger it looks like it was trying to access the OwnerOfPoint of a new force. Then I did:
OwnerOfPoint = F.OwnerOfPoint;
and it worked! Is that ok? I mean it shouldn't be a problem that they both point to the same place because the don't "own" the place and they never delete it or anything. This seems easier than calling AddForce for every time I call a copy ctor. Is this alright?
Thanks.

Aardvajk

1,172

Aardvajk

Aug 15, 2006 at 9:06 AM

The new Force then thinks it is owned by the original RigidBody, rather than the new RigidBody. It will appear to work until the Force tries to access its OwnerOfPoint, at which point it will be accessing the wrong one.

You can't really assign the correct RigidBody in the Force copy constructor because the only argument you can pass in is the Force object being copied, so you don't "know" at that point which RigidBody the new Force will be owned by.

So you need to do it in the RigidBody copy constructor, or assignment operator.

RigidBody::RigidBody(const RigidBody &C){    for(int i=0;i<C.Forces.size();++i)        {        Force *F=new Force(C.Forces);        F->OwnerOfPoint=this;        // or        F->SetOwner(this); // if OwnerOfPoint is private, you need an accessor        Forces.push_back(F);        }}

Hope this is clear. It can become a bit of a nightmare having these referential pointer systems. I'd suggest dumping the OwnerOfPoint all together and having the RigidBody pass itself into each Force member that needs to "know" about the Body that owns it:

void Force::Apply(RigidBody *B){    B->DoStuff();}void RigidBody::ApplyForces(){    for(int i=0;i<Forces.size();++i) Forces->Apply(this);}

daniel_i_l

Author

daniel_i_l

Aug 15, 2006 at 9:37 AM

First of all, thanks for your patiance with me.
Now about the body passing itself everytime, that would definatly be easier but then there's something else I can't figure out how to work into the design - I didn't put it in in the begining for simlification, but in reality each Force has to have access two TWO rigid bodies: the one that it's currently applying force to and the body that the other end of the spring is attached to, I need this and can't just keep it constant and update it each cycle because with the RK4 that I'm imlementing all of the bodies go through the 4 stages together so each body has to be able to keep track were the other end of the spring is so that it can exert the right amount of force at each stage according to the positions of each ends of the spring.
So even if each body passed its end of the string, what about the other end? Wouldn't it have to store that as a pointer anyway? Thats why I thought about getting it working like this so that it'd be easy to add the other pointer later. But is there a better way for the other end to?
-> I sort of have an idea that i just cant seem to put together, maybe I'd have some Link class that stores both ends and also applies the force to both? The problem again is the RK4 integration, since the bodies go through all the stages together (I don't have it woring that way yet but later I hope to pass a vector of bodies to the Integrate function, same with the Deravitive function) it seems like each body has to be able to apply force to itself, and also be able to track the position of the body on the other side of each spring.

Any advice? What's a better way to design this?
Thanks.

Aardvajk

1,172

Aardvajk

Aug 15, 2006 at 10:35 AM

To be honest, I know my C++ and my pointers, but I don't really consider myself skilled enough at the design side to really suggest a solution to such a complex problem. Perhaps someone with more design skills could help.

I don't want to suggest some overcomplicated solution that would end up hard to maintain or extend.

Sorry.

Paul

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.