Simulating gravitational attraction between bodies

Question

I'm currently working on a simple simulator in Unity3D that involves planets orbiting each other. I wrote the following C# script to do that. It uses the classic gravitational formula,

$$F=G\frac{m_1m_2}{r^2}$$

using UnityEngine;

/// <summary>
/// Class used for calculating
/// how an object orbits another
/// object.
/// </summary>
/// <param name="parentObject">The parent gameObject to orbit.</param>
/// <param name="objectMass">The mass of the object.</param>
/// <param name="gravityConstant">The gravitational constant.</param>
[RequireComponent(typeof(Rigidbody))]
public class OrbitObject : MonoBehaviour 
{
    public OrbitObject parentObject;
    public float objectMass;
    public float gravityConstant;

    private Vector3 parentPosition;
    private float parentMass;

    /// <summary>
    /// Calculates the gravitational attraction.
    /// </summary>
    /// <returns>The force to apply</returns>
    Vector3 CalculateGravitationalForce()
    {
        this.parentPosition = this.parentObject.transform.position;
        Vector3 force = this.parentPosition - transform.position;
        float distance = force.magnitude;
        float strength = (this.gravityConstant * this.parentMass * this.objectMass) / (distance * distance);
        force.Normalize();
        force = force * strength;
        return force;
    }

    /// <summary>
    /// FixedUpdate is run once per frame.
    /// FixedUpdate is used so that calculations
    /// don't need to be multiplied by Time.deltaTime.
    /// </summary>
    void FixedUpdate()
    {
        rigidbody.AddForce(this.CalculateGravitationalForce());
    }

    /// <summary>
    /// Start this instance.
    /// </summary>
    void Start()
    {
        this.parentPosition = this.parentObject.gameObject.transform.position;
        this.parentMass = this.parentObject.objectMass;
    }
}

For those who are curious, a gif of the script in action can be found here.

Answer 1

Just a note on your commenting:

Should you be breaking XML comments up across lines like that?

/// <summary>
/// Class used for calculating
/// how an object orbits another
/// object.
/// </summary>

I feel that should be three lines:

/// <summary>
/// Class used for calculating how an object orbits another object.
/// </summary>

Likewise, I feel that the following comment:

/// <summary>
/// FixedUpdate is run once per frame.
/// FixedUpdate is used so that calculations
/// don't need to be multiplied by Time.deltaTime.
/// </summary>

Could be rewritten as:

/// <summary>
/// FixedUpdate is run once per frame.
/// </summary>
/// <remarks>
/// FixedUpdate is used so that calculations don't need to be multiplied by Time.deltaTime.
/// </remarks>

Personally, I think breaking comments along multiple lines like that makes it harder to read. You're eyes have to jump to unnatural places.

Likewise, the <remarks> bit is nice because those comments aren't shown in Intellisense, but they can still be placed in the generated XML documentation files.

The only other bit I would mention, is the use of this. You should either use it consistently, or try to limit it. Sometimes (especially in larger cases) it helps to clear things up. But if you use it for some of the properties and not others, it can have a negative impact.

I'm referring to the following:

Vector3 CalculateGravitationalForce()
{
    this.parentPosition = this.parentObject.transform.position;
    Vector3 force = this.parentPosition - transform.position;
    float distance = force.magnitude;
    float strength = (this.gravityConstant * this.parentMass * this.objectMass) / (distance * distance);
    force.Normalize();
    force = force * strength;
    return force;
}

Where does transform.position come from? The fact that you use this on all the other properties/fields of the class but not that one throws confusion into the mix. It's not a local variable, that much is true. But where exactly does it come from, is a little harder to notice right out of the box.

Again, these are all suggestions. Whether or not you like/use them is up to you, but this is how I would write it out.

Answer 2

Other answers have covered all of the points I'd make about the code already; I just wanted to get a bit physics-y.

Your implementation treats the "parentObject" as a fixed body and there is an implicit assumption about the relative masses of the two bodies.

That is to say, you are assuming that the parent object will have a mass very very much greater than the orbiting body and so will be relatively unaffected by the pull of the smaller body. That's fine for something like a satellite orbiting the earth but would be a bit odd if you have similar sized bodies orbiting each other in a different context.

For example, here's a two body system with bodies of similar masses in an elliptical orbit: https://en.wikipedia.org/wiki/Elliptic_orbit#/media/File:Orbit5.gif

Now, treating the real world as the real world is obviously much much more complicated. However, you could easily satisfy the physicist in me by renaming your object to CircularOrbit and ensure that the mass of the parent object is much greater.

This comes with the caveat that I don't actually know Unity so I might have misunderstood how the code works.

Answer 3

Your comments seem a little basic, at that, maybe write more than the basic reasoning of each function/execution, etc.

force = force * strength;
return force;

You can use the /=, *=, +=, -= operators for that:

return force *= strength;

---

I would also declare all your initial variables by type,

ParentObject
Float
Vector3