Here’s a little story about how I found out my code was hideous and how I fixed it.
First some context. I’m currently developing my very first commercial game and I was recycling some code from previous iterations of that same project to speed up the development, I imported a script I wrote months ago that allows corners to rotate the units inside it only when moving, here’s a visual representation:
It works perfectly, but the issue was that I spent almost 45 minutes figuring out how to make the script work correctly; the variables didn’t make any sense whatsoever, the color of the gizmos was mixed up, and I wasn’t able to test it in real-time, meaning I had to exit play mode to tweak the, hear me out, the EXPOSED variables in the inspector, Why did I expose variables in the first place? It didn’t make any sense. Here’s the original code:
public class Cornerer : MonoBehaviour
{
[Header("Angle Settings")]
[Range(-360, 360)] [SerializeField] float enteringAngle = 0;
[Range(-360, 360)] [SerializeField] float exitAngle = 0;
[Range(0, 360)] [SerializeField] float minAngle = 90;
[Range(0, 360)] [SerializeField] float maxAngle = 180;
[Header("Line Settings")]
[SerializeField] bool checkLine = false;
[SerializeField] bool useZDistance = false;
float angle;
float maxLerp;
float distance;
Collider col;
Quaternion enteringRotation;
Quaternion exitRotation;
private void Awake() { SetInitialValues(); }
private void SetInitialValues()
{
col = GetComponent<Collider>();
maxLerp = maxAngle - minAngle;
enteringRotation = Quaternion.Euler(Vector3.up * enteringAngle);
exitRotation = Quaternion.Euler(Vector3.up * exitAngle);
distance = useZDistance ? col.bounds.max.z - col.bounds.min.z : col.bounds.max.x - col.bounds.min.x;
}
private void OnTriggerStay(Collider other) { RotateAgent(other); }
private void RotateAgent(Collider other)
{
angle = checkLine ? 0 : CalculateAngle(other.transform);
float floatToCompare = checkLine ? distance : maxLerp;
float lerpValue = CalculateLerpValue(other.transform) / floatToCompare;
other.transform.rotation = Quaternion.Lerp(enteringRotation, exitRotation, lerpValue);
}
private float CalculateAngle(Transform t)
{
Vector3 playerYNormalized = t.position;
playerYNormalized.y = transform.position.y;
angle = Vector3.Angle(transform.forward, playerYNormalized - transform.position);
angle = Vector3.Angle(transform.right, playerYNormalized - transform.position) > 90 ? 360 - angle : angle;
return angle;
}
private float CalculateLerpValue(Transform other)
{
if (checkLine) { return useZDistance ? other.position.z - col.bounds.min.z : other.position.x - col.bounds.min.x; }
return maxAngle - angle;
}
//Gizmos code was here, removed to shorten the thread.
I had no idea how to make the script work correctly so I imported the original assets, the script was finally working, but I still had no clue about how it was working. What is angle? What does “checkLine” means? What are does weird spheres that appear when I click on “checkline”? What is this? I kid you not, figuring out this simple code took me a total time of 1 hour, and it took another 30 minutes to refactor it for the code to make any sort of sense, not just in code, but in Unity’s Inspector. Here’s the result:
[SerializeField] bool isAnAisle = false;
[SerializeField] bool useZDistance = false;
[Range(-360, 360)] [SerializeField] float enteringAgentRotation = 0;
[Range(-360, 360)] [SerializeField] float exitAgentRotation = 0;
[Range(0, 360)] [SerializeField] float enteringFace = 180;
[Range(0, 360)] [SerializeField] float exitFace = 90;
Collider col;
private Quaternion EnteringRotation { get => Quaternion.Euler(Vector3.up * enteringAgentRotation); }
private Quaternion ExitRotation { get => Quaternion.Euler(Vector3.up * exitAgentRotation); }
private void Awake()
{
col = GetComponent<Collider>();
}
private void OnTriggerStay(Collider other) { RotateAgent(other.transform); }
private void RotateAgent(Transform agent)
{
float angle = isAnAisle ? 0 : CalculateAgentAngle(agent);
float floatToCompare = isAnAisle ? CalculateAisleDistance() : enteringFace - exitFace;
agent.rotation = Quaternion.Lerp(EnteringRotation, ExitRotation, CalculateLerpValue(agent, angle) / floatToCompare);
}
private float CalculateAgentAngle(Transform agent)
{
Vector3 playerYNormalized = agent.position;
playerYNormalized.y = transform.position.y;
float agentAngle = Vector3.Angle(transform.forward, playerYNormalized - transform.position);
agentAngle =
Vector3.Angle(transform.right, playerYNormalized - transform.position) > 90 ? 360 - agentAngle : agentAngle;
return agentAngle;
}
private float CalculateAisleDistance()
{
return useZDistance ? col.bounds.max.z - col.bounds.min.z : col.bounds.max.x - col.bounds.min.x; ;
}
private float CalculateLerpValue(Transform agent, float angle)
{
if (isAnAisle) { return useZDistance ? agent.position.z - col.bounds.min.z : agent.position.x - col.bounds.min.x; }
return enteringFace - angle;
}
By following the SOLID principles I was able to make sense out of that code, then fix it so I was able to test things out in real-time instead of having to exit play mode every time I wanted to make a change.
The biggest issue with the code was definitely the naming, I had variables with names like “minAngle” and “enteringAngle”, What is that supposed to mean? I had no idea, not a single clue! After reading the code multiple times and testing things out I find out that “minAngle” was actually one of the faces of the corner, so I named both variables accordingly “enteringFace” and “exitFace”, same with the “enteringAngle”, Entering angle of what exactly? I found out it was referring to the unit’s rotation while entering from the “minAngle” so I named it accordingly “enteringAgentRotation”. I also fixed the gizmos, I didn’t show the previous ones but believe me, you don’t want to see them, they made no sense and the colors were mixed up pretty badly.
This took quite some time to fix, but I’m happy with the end result, not completely satisfied, but at least I know what the variables mean and do and I now can test things in real-time. Finally, I created a custom inspector that removes unnecessary data from the inspector when the corner is not a corner, yes, that’s what those “Line Settings” were for, the corner can also work as an aisle instead. Here’s how everything works in the inspector now:
White symbolizes the entering face, black the exit face, and Custom Editor also eliminates unnecessary data from the inspector, like the entering and exit face when it is an aisle instead of a corner. The code now also shows the entering and exit faces of the aisle, before it showed… wait for it… two spheres at the edge of the collider that pointed nowhere, you can imagine my confusion.
My biggest lessons from all of this:
- Name things as accurately as possible, it doesn’t matter if the name is 50 characters long, making things understandable is far better than making them short.
- Don’t cache things! Why did I cache all those variables? WHY?!?!?!?! It’s a real-time work environment, use the real-time part to your advantage!
- Make things consistent, in the previous gizmos the colors for the aisle and corner were inverted so I had no idea what they meant, white was the exit when in corner mode and the entrance when in aisle mode, now I have no hair from scratching my head for an hour straight trying to figuring out what the colors meant.
- Gizmos are meant to make things more understandable, not more confusing!
Lastly, I have to say I’m kinda happy about this whole situation, it made me realize that I have improved a lot since last year.
Hope you guys found this interesting and perhaps even helpful, now go and challenge yourselves, go and fix an old project!