Maik Justus wrote: > I would be very thankful for any review of this version. And I like > to ask Andy, to review if this version could go into cvs.
This looks much, much better to me. Here's one spot that I'd like to see changed, in Model.cpp: + if (_rotorgear) + _rotorgear->calcForces(&_rotors,&_rotorparts,&_body); //it adds several torques to the body. Therfore it needs the _body as + //a parameter Yes, that's a 131 character line you wrote; I have a 1920x1200 screen and it takes of 2/3 of my horizontal real estate to read. There are a few other much-longer-than-80-column lines in there too. But that's the not actual complaint: The various calcForces() routines are idempotent by design. They should *only* calculate forces, never change state. The solver and various test code I've written actually relies on that property so it can (for example) get forces for a Surface in isolation, etc... The proper mechanism here is to fill in two 3-vectors for force and a torque which are added to the RigidBody object in the top-level Model::calcForces(). If you want to use the two-argument version of RigidBody::addForce() (for at a position), then you will need to return a slightly more complicated data structure. Also (and this is really a question about you design than a requirement to go into CVS) why do the pointers to _rotors and _rotorparts need to be passed to a method on _rotorgear? Shouldn't one of these objects contain the other ones? See PropEngine for an example: it's a Thruster object that presents a unified interface, but contains a Propeller and Engine so that they don't have to be visible to most of the Model code. Maybe the real question is: what exactly is the difference between a "rotor part" a "rotor" and a "rotor gear"? Shouldn't there just be one object? What is the abstraction here? Andy ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 _______________________________________________ Flightgear-devel mailing list Flightgear-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/flightgear-devel
