HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Car controlled by steering wheel

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
unity3dcodereviewcsharpstackoverflowc#
steeringwheelcarcontrolled

Problem

I'm a young coder who is looking to improve in my coding practices because I was told by my supervisor to look into a more healthier method to code.

What I would like to ask is how can I improve my coding structures? For example, is using a whole lot of bool to check for conditions considered acceptable? // Other than commenting it.

PS : I'm not going to present an updated copy of the code to my supervisor, I just want to improve myself for next time.

PSS : I study coding, but for coding structures, I was only taught the theory. There were no coding assignments.

PSSS : This code here is for a car in Unity that can drive back and front using a Logitech steering wheel GT Force.

I don't really know where else to ask since I'm put to internship and I won't be meeting my lecturers for a long time.

Here is an example of a code I've recently worked on:

```
using System.Collections;
using System.Collections.Generic;
using UnityEngine;

// DEBUG : Make sure when alternating between pedals, the momentum is still there

public class VehicleMovements : MonoBehaviour {

private Rigidbody vehicleRigidbody;
private Transform vehicleTransformer;
public float momentum;
private float turning;

[SerializeField]
float moveSpeed;
[SerializeField]
float reverseSpeed;
[SerializeField]
float steeringWheel;
[SerializeField]
float turningSpeed; // 1
[SerializeField]
float pedal;
[SerializeField]
float momentumMultiplier; // 1.05
[SerializeField]
float startOffSpeed; // 0.05

// Can change to private when done testing
[SerializeField]
bool isPedal;
[SerializeField]
bool isBrake;
[SerializeField]
bool hasBrakeReset;
[SerializeField]
bool stillBacking;

// Use this for initialization
void Start () {
vehicleRigidbody = GetComponent();
vehicleTransformer = GetComponent();
reverseSpeed -= (reverseSpeed + reverseSpeed);
}

// Update is called o

Solution

Short Answer: you can use Dictionary instead of declaring many float. I didn't use Array or List because we need naming values instead of using index.

next I use enum....why I didn't use Dictionary?

because enum & switch is better than using Dictionary for conditions.However, if the conditions were related you should use Dictionary.

if(a && b && c) -> use Dictionary
if(a || b || c) -> use enum


Well this isn't confusing.

using UnityEngine;
using System.Collections.Generic;

public class VehicleMovements : MonoBehaviour
{
    private Rigidbody vehicleRigidbody;
    private Transform vehicleTransformer;
    public float momentum;
    private float turning;

    public Dictionary CarFloats = new Dictionary()
    {
    {"steeringWheel",0},
    {"turningSpeed",0},
    {"pedal",0},
    {"momentumMultiplier",1},
    {"moveSpeed",1.5f},
    {"startOffSpeed",0.05f},
    };

    public enum Modes { pedal , brake , hasBrakeReset,stillBacking };
    public Modes modes;

    void Update() 
    {
        if(CarFloats["steeringWheel"] == 0)
        {
            print("hello");
        }

        switch (modes)
        {
            case Modes.pedal: print("pedal");
                break;
            case Modes.brake: print("brake");
                break;
            case Modes.hasBrakeReset: print("hasBrakeReset");
                break;
            case Modes.stillBacking: print("stillBacking");
                break;
            default: print("None");
                break;
        }
    }
}


Another way to prevent confusing is using CustomEditor.you can grouping your variables by using Foldout.

using UnityEngine;

[System.Serializable]
public class VehicleMovements : MonoBehaviour
{
    private Rigidbody vehicleRigidbody;
    private Transform vehicleTransformer;
    public float momentum;
    private float turning;

    //HideInInspector This allows you hide public variables in inspector
    //floats
    [HideInInspector]
    public float reverseSpeed,
    moveSpeed,
    steeringWheel,
    turningSpeed = 1,
    pedal,
    momentumMultiplier = 1.5f,
    startOffSpeed = 0.05f;

    [HideInInspector]
    //booleans
    public bool isPedal,
    isBrake,
    hasBrakeReset,
    stillBacking;
}


Create Folder as Editor and put this script in it.

using UnityEngine;
using UnityEditor;

[CustomEditor(typeof(VehicleMovements))]
[CanEditMultipleObjects()]
public class TriggerEditor : Editor
{
    public bool CarFloats = true;
    public bool CarBooleans = true;

    override public void OnInspectorGUI()
    {
        //if you need DefaultInspector use this (it works like base class in overriding)
        DrawDefaultInspector();

        //2 space
        for (int i = 0; i < 2; i++)
        {
            EditorGUILayout.Space();
        }
        //use VehicleMovements class variables
        VehicleMovements targetPlayer = (VehicleMovements)target;

        //grouping variables by Foldout
        CarFloats = EditorGUILayout.Foldout(CarFloats, "Car Floats");

        //if click
        if (CarFloats)
        {

            targetPlayer.reverseSpeed = EditorGUILayout.Slider("ReverseSpeed", targetPlayer.reverseSpeed, 0, 100);
            targetPlayer.steeringWheel = EditorGUILayout.FloatField("SteeringWheel", targetPlayer.steeringWheel);
            targetPlayer.turningSpeed = EditorGUILayout.FloatField("TurningSpeed", targetPlayer.turningSpeed);
            targetPlayer.pedal = EditorGUILayout.FloatField("Pedal", targetPlayer.pedal);
            targetPlayer.momentumMultiplier = EditorGUILayout.FloatField("MomentumMultiplier", targetPlayer.momentumMultiplier);
            targetPlayer.startOffSpeed = EditorGUILayout.FloatField("StartOffSpeed", targetPlayer.startOffSpeed);

        }

        //2 space
        for (int i = 0; i < 2; i++)
        {
            EditorGUILayout.Space();
        }

        //grouping variables by Foldout
        CarBooleans = EditorGUILayout.Foldout(CarBooleans, "Car Booleans");

        if (CarBooleans)
        {

            targetPlayer.isPedal = EditorGUILayout.Toggle("IsPedal", targetPlayer.isPedal);
            targetPlayer.isBrake = EditorGUILayout.Toggle("IsBrake", targetPlayer.isBrake);
            targetPlayer.hasBrakeReset = EditorGUILayout.Toggle("HasBrakeReset", targetPlayer.hasBrakeReset);
            targetPlayer.stillBacking = EditorGUILayout.Toggle("StillBacking", targetPlayer.stillBacking);
            targetPlayer.isPedal = EditorGUILayout.Toggle("IsPedal", targetPlayer.isPedal);

        }
    }
}


but this is simple example of using CustomEditor
I already replied to a question in relation to CustomEditor so this useful link for you

Your obvious problem in your code is that you have longer FixedUpdate.I wonder why you didn't use the functions in your code.

this is part of your code that i try to use functions to clearing and improving your code.

```
void Start()
{
InitializeComponents();
}

void InitializeComponents()

Code Snippets

if(a && b && c) -> use Dictionary<string, bool>
if(a || b || c) -> use enum
using UnityEngine;
using System.Collections.Generic;

public class VehicleMovements : MonoBehaviour
{
    private Rigidbody vehicleRigidbody;
    private Transform vehicleTransformer;
    public float momentum;
    private float turning;

    public Dictionary<string, float> CarFloats = new Dictionary<string, float>()
    {
    {"steeringWheel",0},
    {"turningSpeed",0},
    {"pedal",0},
    {"momentumMultiplier",1},
    {"moveSpeed",1.5f},
    {"startOffSpeed",0.05f},
    };


    public enum Modes { pedal , brake , hasBrakeReset,stillBacking };
    public Modes modes;


    void Update() 
    {
        if(CarFloats["steeringWheel"] == 0)
        {
            print("hello");
        }

        switch (modes)
        {
            case Modes.pedal: print("pedal");
                break;
            case Modes.brake: print("brake");
                break;
            case Modes.hasBrakeReset: print("hasBrakeReset");
                break;
            case Modes.stillBacking: print("stillBacking");
                break;
            default: print("None");
                break;
        }
    }
}
using UnityEngine;

[System.Serializable]
public class VehicleMovements : MonoBehaviour
{
    private Rigidbody vehicleRigidbody;
    private Transform vehicleTransformer;
    public float momentum;
    private float turning;

    //HideInInspector This allows you hide public variables in inspector
    //floats
    [HideInInspector]
    public float reverseSpeed,
    moveSpeed,
    steeringWheel,
    turningSpeed = 1,
    pedal,
    momentumMultiplier = 1.5f,
    startOffSpeed = 0.05f;


    [HideInInspector]
    //booleans
    public bool isPedal,
    isBrake,
    hasBrakeReset,
    stillBacking;
}
using UnityEngine;
using UnityEditor;

[CustomEditor(typeof(VehicleMovements))]
[CanEditMultipleObjects()]
public class TriggerEditor : Editor
{
    public bool CarFloats = true;
    public bool CarBooleans = true;

    override public void OnInspectorGUI()
    {
        //if you need DefaultInspector use this (it works like base class in overriding)
        DrawDefaultInspector();

        //2 space
        for (int i = 0; i < 2; i++)
        {
            EditorGUILayout.Space();
        }
        //use VehicleMovements class variables
        VehicleMovements targetPlayer = (VehicleMovements)target;

        //grouping variables by Foldout
        CarFloats = EditorGUILayout.Foldout(CarFloats, "Car Floats");

        //if click
        if (CarFloats)
        {

            targetPlayer.reverseSpeed = EditorGUILayout.Slider("ReverseSpeed", targetPlayer.reverseSpeed, 0, 100);
            targetPlayer.steeringWheel = EditorGUILayout.FloatField("SteeringWheel", targetPlayer.steeringWheel);
            targetPlayer.turningSpeed = EditorGUILayout.FloatField("TurningSpeed", targetPlayer.turningSpeed);
            targetPlayer.pedal = EditorGUILayout.FloatField("Pedal", targetPlayer.pedal);
            targetPlayer.momentumMultiplier = EditorGUILayout.FloatField("MomentumMultiplier", targetPlayer.momentumMultiplier);
            targetPlayer.startOffSpeed = EditorGUILayout.FloatField("StartOffSpeed", targetPlayer.startOffSpeed);

        }

        //2 space
        for (int i = 0; i < 2; i++)
        {
            EditorGUILayout.Space();
        }

        //grouping variables by Foldout
        CarBooleans = EditorGUILayout.Foldout(CarBooleans, "Car Booleans");

        if (CarBooleans)
        {

            targetPlayer.isPedal = EditorGUILayout.Toggle("IsPedal", targetPlayer.isPedal);
            targetPlayer.isBrake = EditorGUILayout.Toggle("IsBrake", targetPlayer.isBrake);
            targetPlayer.hasBrakeReset = EditorGUILayout.Toggle("HasBrakeReset", targetPlayer.hasBrakeReset);
            targetPlayer.stillBacking = EditorGUILayout.Toggle("StillBacking", targetPlayer.stillBacking);
            targetPlayer.isPedal = EditorGUILayout.Toggle("IsPedal", targetPlayer.isPedal);

        }
    }
}
void Start()
{
    InitializeComponents();
}


void InitializeComponents()
{
    vehicleRigidbody = GetComponent<Rigidbody>();
    vehicleTransformer = GetComponent<Transform>();
    reverseSpeed -= (reverseSpeed + reverseSpeed);
}

void FixedUpdate()
{
    Axis();
    CheckPedal();
}


void Axis()
{
    steeringWheel = Input.GetAxis("SteeringWheel");
    pedal = Input.GetAxis("Pedal");
}

void CheckPedal(){
    if (pedal > 0.1)
    {
        BrakeReset();
    }
    else if (Pedal)
    {
        print("blah blah")
    }
}

void BrakeReset()
{
    hasBrakeReset = true;
    isBrake = false;
    stillBacking = false;
    if (!isPedal)
    {
        momentum = startOffSpeed;
        isPedal = true;
    }
    if (momentum <= pedal) momentum *= momentumMultiplier;
    else momentum /= momentumMultiplier;
    moveVehicle(moveSpeed);
}


public bool Pedal() {
    if (pedal >= 0 && pedal <= 0.1)
    {
        return true;
    } else
    {
        return false;
    }
}

Context

StackExchange Code Review Q#162095, answer score: 4

Revisions (0)

No revisions yet.