-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PlanetPhysicsHelper should contain all the things it needs #3
Comments
Hum... I'm not sure I understand well, you're saying PlanetPhysicsHelper should contain the physical equations? That would not be possible, we need to keep the plug-and-play physics, which imposes to be able to change at will the equations. |
Yes, the latter. PlanetPhysicsHelper should do all that construction that goes on in physics_helper_unit within itself and manage the objects it needs. That way, PlanetPhysics doesn't have to. |
PlanetPhysics should most certainly not have to do this, this is part of the previous lines of code. I don't see what is the difference if the objects are constructed in or out PlanetPhysics, Helper or not. |
This isn't an immediate issue. Something to do later. Just wanted to point out this would be a cleaner way to do things. |
Make the code so it has the following layout: PlanetPhysicsHelper makes all the internal arrangements as how things are related to each other (see model_doc for the graph), and should provide references to all the stored objects. |
Derivatives added, need tests now:
So, right now PlanetPhysicsHelper takes references and pointers to things it needs during the simulation. The intent was actually to have PlanetPhysicsHelper be self-contained. I would suggest at some point that it be updated to do this. The idea being that the constructor takes a GetPot file and builds up everything it needs internally. That way, it's much easier to construct PlanetPhysics. As it is, I'll house all that stuff in PlanetPhysics and build up the PlanetPhysicsHelper as is, but wanted to create this so that when I'm done enabling PlanetPhysics, this doesn't get lost.
The text was updated successfully, but these errors were encountered: