Roll-a-ball controller

Question

I've been toying around with Unity 5 lately, and in an effort to start making an actual game, I've built a ball controller similar to the one found in the Unity tutorial Roll-a-ball.

In essence, the player controls a small ball that rolls around a "level". The ball can accelerate at a rate in the range \$1\rightarrow10\$ depending on what I (the designer) choose. The controller also manually calculates drag to create a "low-gravity" feel, but still have a sense of speed. Drag is calculated using the following formula:

$$\text{drag}=\frac{\text{acceleration vector magnitude}}{\text{some dividend}}$$

I'm wondering about the following things:

• Is it appropriate for the controller to do things such as drag calculation, or should this be separated into a separate area?
• I'm currently using a dictionary, looping through it, checking for keypresses, and then applying force if a key is pressed. Is this a good way to handle it? Will this ever slow anything down?
• Are there any performance improvements that can be made?
• Anything else?

BallController.cs

using UnityEngine;
using System.Collections.Generic;

/// <summary>
/// This class contains various methods and attributes
/// used to control the "vehicle".
/// </summary>
[System.Serializable]
[RequireComponent(typeof(Rigidbody))]
public class MoveVehicle : MonoBehaviour
{
    /// <summary>
    /// The force of an acceleration. This will be applied
    /// to the ball when it's accelerating.
    /// </summary>
    [Range(1, 10)]
    public float accelerationForce;

    /// <summary>
    /// This determines the drag being exerted on the ball
    /// as it moves. Drag is determined through the formula
    /// drag = accelerationForce / dragDividend
    /// </summary>
    [Range(0, 1000)]
    public float dragDividend;

    private Rigidbody rigidBody;
    private Dictionary<KeyCode, Vector3> keyMappings = new Dictionary<KeyCode, Vector3>() {
        {KeyCode.Space, new Vector3(0, 5.5f, 0)},

        {KeyCode.W, new Vector3(0, 0, 1)},
        {KeyCode.A, new Vector3(-1, 0, 0)},
        {KeyCode.S, new Vector3(0, 0, -1)},
        {KeyCode.D, new Vector3(1, 0, 0)},

        {KeyCode.UpArrow, new Vector3(0, 0, 1)},
        {KeyCode.LeftArrow, new Vector3(-1, 0, 0)},
        {KeyCode.DownArrow, new Vector3(0, 0, -1)},
        {KeyCode.RightArrow, new Vector3(1, 0, 0)}
    };

    /// <summary>
    /// Check if the ball is near to, or touching the
    /// ground. This is used to determine whether the
    /// vehicle can move or not.
    /// </summary>
    public bool IsGrounded()
    { return Physics.Raycast(this.transform.position, -Vector3.up, 0.65f); }

    /// <summary>
    /// Initialize things like our Rigidbody component,
    /// or other things that can only be initialized in
    /// Start.
    /// </summary>
    public void Start()
    { this.rigidBody = GetComponent<Rigidbody>(); }

    /// <summary>
    /// Apply thrust to the ball. The faster the vehicle
    /// gets, the more drag we apply to the rigidbody. This will
    /// create the effect of reaching a "maximum" speed.
    /// </summary>
    public void FixedUpdate()
    {
        foreach(KeyValuePair<KeyCode, Vector3> keyMapping in this.keyMappings)
        {
            if(Input.GetKey(keyMapping.Key) && this.IsGrounded())
            {
                Vector3 objectForce = keyMapping.Value * this.accelerationForce;
                this.rigidBody.drag = objectForce.sqrMagnitude / this.dragDividend;
                this.rigidBody.AddForce(objectForce);
            }
        }
    }
}

Answer 1

First let us nitpick a little bit.

MoveVehicle would be a nice name for a method but for a class it isn't choosen well. A class name should be made out of a noun or a noun phrase. So for instance VehicleMover or VehicleController would be a much better name for this class.

---

The method in question

public void FixedUpdate()
{
    foreach(KeyValuePair<KeyCode, Vector3> keyMapping in this.keyMappings)
    {
        if(Input.GetKey(keyMapping.Key) && this.IsGrounded())
        {
            Vector3 objectForce = keyMapping.Value * this.accelerationForce;
            this.rigidBody.drag = objectForce.sqrMagnitude / this.dragDividend;
            this.rigidBody.AddForce(objectForce);
        }
    }
}  

here I would suggest to use var rather than KeyValuePair<KeyCode, Vector3> which would make your code a little bit cleaner and its obvious that it will be a KeyValuePair<TKey,TValue.

Otherwise your code looks nice and tidy. Good job.

I don't know Unity3D at all, so I don't know if it is possible to switch the public float accelerationForce; to a public property. If this is possible, I would suggest inside the setter to change all the values of the Dictionary so they represent the objectforce. This would involve a second kind of collection or some constants to hold the currently predifined values of the current Dictionary.

If it is likely that the accelerationForce is called less often than the FixedUpdate() method this will speed the whole thing up.

Answer 2

Your file is named BallController.cs. Your class is named MoveVehicle.

Problem is, both names are wrong. You're not moving a ball or a vehicle. You're moving the a Player. So why isn't your file named PlayerController.cs?

If you change the shape of your Player, it's still a Player. But it may no longer be a Ball. PlayerController would be a nice class-name as well. I know it's the standard name used in the tutorial, but it so happens to be a very good one.