Player Rotation script

Question

This is my player rotation script. I just want to make sure my naming conventions are right. If there is any way to make this code more readable I'm open to suggestions. This is also my 3rd post about along the same line so please be brutal.

public class ShipRotation : MonoBehaviour
{
 [SerializeField] Canvas canvas;
 ShipManager shipControls;
 GameObject ship;
 DebugController debugController;
 Camera shortHandCamera;
 [SerializeField] GameObject mainCamera;
 float distFromCamera;
 void Start()
 {
 shortHandCamera = Camera.main;
 shipControls = GetComponent<ShipManager>();
 debugController = canvas.GetComponent<DebugController>();
 ship = GameObject.Find("Player");
 distFromCamera = Mathf.Abs(mainCamera.transform.position.z);
 target = new Vector3(0f, 2f, ship.transform.position.z);
 }
 enum StateOfRotation
 {
 waiting,
 rotating
 }
 public LayerMask touchInputMask;
 Vector3 target;
 void Update()
 {
 foreach (Touch touch in Input.touches)
 {
 Ray ray = Camera.main.ScreenPointToRay(touch.position);
 RaycastHit hit;
 if (!Physics.Raycast(ray, out hit, touchInputMask)) { continue; }
 GameObject recipient = hit.transform.gameObject;
 if (recipient.tag != "Player")
 {
 target = Camera.main.ScreenToWorldPoint(new Vector3(touch.position.x, touch.position.y, ship.transform.position.z + distFromCamera));
 }
 }
 ship.transform.LookAt(target);
 debugController.ChangeText(6, target.ToString());
 }
}

Answer 1

A couple additional notes:

I would never do this:

if (!Physics.Raycast(ray, out hit, touchInputMask)) { continue; }

In my opinion, it destroys the layout of the code.

Instead, do:

if (!Physics.Raycast(ray, out hit, touchInputMask))
 continue;

This makes it easier to read and follow, in my opinion.

Any public properties, fields, etc. should be PascalCase.

public LayerMask touchInputMask;

To

public LayerMask TouchInputMask;

Likewise, never use public fields. Use properties:

public LayerMask TouchInputMask { get; set; }

Additionally, I recommend separating block by no more or less than 1 empty line.

public LayerMask TouchInputMask { get; set; }
Vector3 target;
void Update()
{
 foreach (Touch touch in Input.touches)
 {
 Ray ray = Camera.main.ScreenPointToRay(touch.position);
 RaycastHit hit;
 if (!Physics.Raycast(ray, out hit, touchInputMask))
 continue;
 GameObject recipient = hit.transform.gameObject;
 if (recipient.tag != "Player")
 {
 target = Camera.main.ScreenToWorldPoint(new Vector3(touch.position.x, touch.position.y, ship.transform.position.z + distFromCamera));
 }
 }
 ship.transform.LookAt(target);
 debugController.ChangeText(6, target.ToString());
}

All of these are opinions, you can use or ignore them at your own discretion, but these are some more changes I would make.

• Funlamb: One question about your use of properties. If I use a variable like private int Score what would you name to property to access it? public int MyScore {get; set;}? This is something that has been confusing me with everyone telling my to use PascalCase with variable names.
• Der Kommissar: Generally: private int score; public int Score { get { return score; } set { score = value; } }

Answer 2

A couple of quick minor points:

Try not to abbreviate names, characters cost you nothing.

float distFromCamera

should be renamed to:

float distanceFromCamera

---

Names should relate to what they are ShipManager shipControls is it a "Manager" or a set of controls? The type name doesn't match the variable name in my opinion. As an aside, I generally dislike ***Manager as a name (and I'm not the only one);

---

Enum members should be PascalCase:

enum StateOfRotation
{
 Waiting,
 Rotating
}

---

It is nicer to put all of your fields at the top of your file. You have some mixed in between method definitions.

---

I prefer explicit access modifiers on everything. e.g. private float distanceFromCamera. In Unity, with the whole public field thing, it can make the class easier to skim as well.