Honestly, not bad at all. I’ve gone through and done a quick refactor, and made comments from your original to the refactored version. I removed your comments to avoid confusion. If anything is unclear, let me know.
Annotated code:
using UnityEngine;
using System.Collections;
public class BoostMovement : MonoBehaviour {
/* All of these are public, which is absolutely fine if you want
* them to be accessible from the Unity editor.
*
* Quite frequently, I like to group like-properties into their own struct or class,
* Especially in large projects. See Fig 2.2
*/
public float speed = 6;
public float jumpSpeed = 8;
public float gravity = 25; // <- See Fig 2.1
public float boostForce = 18;
public float groundDashForce = 18;
public float airDashForce = 33;
float mass = 1;
/* You might want to make your own time tracking class, so you can use it in other objects as well.
* See Fig 1
*/
float dashTimer = 0;
float dashDelay = 0.7f;
/* I feel like these should both be properties, rather than fields. They're assignable
* from outside code and that is fundamentally insecure.
* See Fig 2.3
*/
public bool isGrounded = true;
public bool usedBoost = false;
private Vector3 impact = Vector3.zero;
private Vector3 moveDirection = Vector3.zero;
CharacterController controller;
//I would use Awake() for this initialization rather than Start().
//Here's why: http://unity3d.com/learn/tutorials/modules/beginner/scripting/awake-and-start
void Start () {
controller = GetComponent <CharacterController> ();
}
void Update ()
{
/*
* If a function does many things, i like to split it up into smaller functions so that it's easier to follow.
* Especially in the update loop, because it's such an important loop.
* See Fig 3.1
*/
CollisionFlags flags = controller.Move (moveDirection * Time.deltaTime);
isGrounded = (flags & CollisionFlags.CollidedBelow) != 0;
if(isGrounded)
{
moveDirection = new Vector3(Input.GetAxis("Horizontal"), 0, Input.GetAxis("Vertical"));
moveDirection = transform.TransformDirection(moveDirection);
moveDirection *= speed;
/* GetKeyDown is fine, but if you use Input.GetButton instead, you can define
* custom user input in Edit -> Project Settings -> Input.
*
* That way, you can change the button you want to be used for jump or even let players
* define their own buttons. See Fig 1
*/
if (Input.GetKeyDown ("space"))
{
moveDirection.y = jumpSpeed;
}
usedBoost = false;
}
else if(!isGrounded)
{ /* These two conditions can be combined.
* See Fig 3.2 */
if(Input.GetKeyDown ("space") && !usedBoost)
{
Boost ();
}
}
if(Input.GetKeyDown("left shift") && dashTimer <= 0)
{
Dash ();
}
if (impact.magnitude > 0.2F)
{
controller.Move(impact * Time.deltaTime);
}
impact = Vector3.Lerp(impact, Vector3.zero, 5 * Time.deltaTime);
//Should gravity only be applied if isGrounded? Fig 4
moveDirection.y -= gravity * Time.deltaTime;
//Notice that, using the timer class in the refactored code, you don't need to decrement the timer yourself.
if(dashTimer > 0)
dashTimer -= Time.deltaTime;
}
//In the refactor, I combined dash into the ApplyUserInput function. See Fig 3.5
void Dash()
{
// This condition might be a bit more complicated than it needs to be. I'm assuming you're using A S and D
// as left movement, right movement, and back movement. As it turns out, these keys also correspond with
// the axis values. A much simpler condition would be (isGrounded and vertical <= 0f) See Fig 3.4
if(!isGrounded || (isGrounded && (Input.GetKey(KeyCode.A) || Input.GetKey(KeyCode.S) || Input.GetKey(KeyCode.D))))
{
//There's a bit of redundancy here, i've cleaned it up, See Fig 3.6
moveDirection = new Vector3(Input.GetAxis("Horizontal"), 0, Input.GetAxis("Vertical"));
moveDirection = transform.TransformDirection(moveDirection);
AddImpact(new Vector3 (moveDirection.x,0,moveDirection.z), isGrounded ? groundDashForce : airDashForce);
dashTimer = dashDelay;
}
}
//In the refactor, I combined boost into the ApplyUserInput function. See Fig 3.3
void Boost()
{
usedBoost = true;
moveDirection.y = boostForce;
}
// This should be combined with your Start function. See Fig 2.4
void Awake ()
{
CharacterController controller = GetComponent<CharacterController>();
if(!controller)
gameObject.AddComponent ("CharacterController");
}
public void AddImpact(Vector3 dir, float force)
{
dir.Normalize();
if (dir.y < 0)
dir.y = -dir.y; // reflect down force on the ground
impact += dir.normalized * force / mass;
}
}
Refactored Code:
using UnityEngine;
using System.Collections;
/* Figure 1
* Having helper classes that can be used in other objects can be
* beneficial! This is a simple example.
*/
public class Timer {
float timeFinished;
float lastAmount;
public bool expired {
get {
return (timeFinished >= Time.time);
}
}
public float remaining {
get {
if (expired) {
return 0f;
} else {
return timeFinished - Time.time;
}
}
}
public Timer (float amount) {
Set(amount);
}
public void Set(float amount) {
if (amount <= 0f) {
throw new System.Exception("Timers cannot be set with negative amounts.");
}
timeFinished = Time.time + amount;
lastAmount = amount;
}
public void Reset(){
Set (lastAmount);
}
}
/* Fig 2.1: Universal Constants
*
* I wouldn't put constants for a particular object that are applicable to many objects in a single objects class.
* Now, I don't know how your game works, maybe every object is supposed to have it's own gravity, but tyically
* I would put all my world constants in a static class and then reference them from there.
*
* Like so: */
public static class Game {
//I also imagine gravity would generally be constant.
public const float Gravity = 25f;
public static class Buttons {
public const string Jump = "Jump";
public const string Dash = "Dash";
}
}
public class Example : MonoBehaviour {
// Fig 2.2 grouped fields
// Makes this class viewable in the editor
[System.Serializable]
public class Settings {
// In the unity editor, there will be a slider for these ranges
[Range(1f,10f)]
public float speed = 6f;
[Range(1f,10f)]
public float jumpSpeed = 8f;
[Range(10f,40f)]
public float boostForce = 18f;
[Range(10f,40f)]
public float dashForceGround = 18f;
[Range(10f,40f)]
public float dashForceAir = 33f;
[Range(0f,1f)]
public float dashTimeDelay = 0.7f;
//Mass wouldn't change, but outside code would be able to look at it
public const float Mass = 1f;
public const float ImpactMovementThreshold = 0.2f;
public const float ImpactEnergyAbsorption = 5f;
}
public Settings settings = new Settings();
// Fig 2.3 read-only properties
bool _isGrounded = true;
public bool isGrounded {
get {
return _isGrounded;
}
}
bool _usedBoost = false;
public bool usedBoost {
get {
return _usedBoost;
}
}
Vector3 impact = Vector3.zero;
Vector3 moveDirection = Vector3.zero;
Timer dashTimer;
CharacterController controller;
//regions are another way to help organize code
#region main
void Awake(){
//Fig 2.4 Combined intializations
controller = GetComponent <CharacterController> ();
if (!controller) {
controller = gameObject.AddComponent <CharacterController>();
}
}
void Start() {
//dashTimer set once Behaviour created and enabled
dashTimer = new Timer(settings.dashTimeDelay);
}
void Update () {
//Fig 3.1 Organized Update Loop
DetermineGrounded();
ApplyUserInput();
ApplyForces();
}
#endregion
#region update loop
void DetermineGrounded(){
CollisionFlags flags = controller.Move (moveDirection * Time.deltaTime);
_isGrounded = (flags & CollisionFlags.CollidedBelow) != 0;
}
void ApplyUserInput(){
var horizontal = Input.GetAxis("Horizontal");
var vertical = Input.GetAxis("Vertical");
Vector3 inputVector = new Vector3(horizontal, 0f, vertical);
if (isGrounded) {
moveDirection = transform.TransformDirection(inputVector) * settings.speed;
if (Input.GetButtonDown(Game.Buttons.Jump)) {
moveDirection.y = settings.jumpSpeed;
}
_usedBoost = false;
//Fig 3.2 combined redundant conditions.
//We'll only get here if we're not grounded.
} else if (Input.GetButtonDown(Game.Buttons.Jump) && !_usedBoost) {
//Fig 3.3 boost is combined into this function
moveDirection.y = settings.boostForce;
_usedBoost = true;
}
//Fig 3.4 dash validation.
//If it's grounded or you're not moving forward, a dash is valid.
bool dashValid = isGrounded || vertical < 0f;
//Fig 3.5 combined dash into this function
if (Input.GetButtonDown(Game.Buttons.Dash) && dashTimer.expired && dashValid) {
//Fig 3.6
//Cleaned up a bit for less redundancy. Notice the inclusion of the
//inputVector at the top.
float dashForce = (isGrounded) ? settings.dashForceGround : settings.dashForceAir;
moveDirection = transform.TransformDirection(inputVector);
AddImpact(moveDirection, dashForce);
dashTimer.Reset();
}
}
void ApplyForces(){
if (impact.magnitude > Settings.ImpactMovementThreshold) {
controller.Move(impact * Time.deltaTime);
}
impact = Vector3.Lerp(impact, Vector3.zero, Settings.ImpactEnergyAbsorption * Time.deltaTime);
//Fig 4 only apply gravity EDIT: when NOT gounded.
if (!isGrounded) {
moveDirection.y -= Game.Gravity * Time.deltaTime;
}
}
#endregion
#region helper functions
public void AddImpact(Vector3 dir, float force)
{
dir.Normalize();
if (dir.y < 0)
dir.y = -dir.y;
impact += dir.normalized * force / Settings.Mass;
}
#endregion
}
