Many resources online feature industry professionals reviewing beginner code to provide guidance on improvements. This approach is different. I wondered if I could refactor my first-ever game and assess it from an intermediate game developer's perspective. I could see where my understanding of game development has grown. Think of these as tips for myself that you can apply to your own work. As this is my first ever article, please send me feedback via the Contact Button in the header. You can click the cursive, underlined parts of the article, since they are hyperlinks.
Single Responsibility Principle (SRP)
The SRP enhances code readability, maintainability, and testability by ensuring each class has a single responsibility. In my case, classes were handling too many responsibilities, such as weapons controlling both firing mechanics and input and all weapon types coded in one class, so a pistol for instance knew about the shotgun logic.
The approach I used to fix this issue is to read the inputs through the Player. The Fire class attached to the weapon only serves to provide methods like Fire, Reload, and helper methods to identify the current state of the weapon. To fix the issue of a Pistol knowing about the properties of a Shotgun, I decided to outsource the weapon logic using the Flyweight pattern (a design pattern that minimizes memory usage by sharing data with similar objects). Variables like the prefab, the number of bullets, spread, and fire rate are all kept outside. The Fire class only reads the values and uses them for the shooting action. Therefore, the shooting action doesn't have any weapon-specific properties.
The approach I used to fix this issue is to read the inputs through the Player. The Fire class attached to the weapon only serves to provide methods like Fire, Reload, and helper methods to identify the current state of the weapon. To fix the issue of a Pistol knowing about the properties of a Shotgun, I decided to outsource the weapon logic using the Flyweight pattern (a design pattern that minimizes memory usage by sharing data with similar objects). Variables like the prefab, the number of bullets, spread, and fire rate are all kept outside. The Fire class only reads the values and uses them for the shooting action. Therefore, the shooting action doesn't have any weapon-specific properties.
Another example is the player health component managing both health and item collection. While this specific case isn't a significant issue, I noticed similar patterns throughout the entire project. To tackle this, I decided to put all the logic related to interactions into its own class.
Inconsistent Coding
Multiple methods (FindObjectOfType, SerializeField, GetComponent) were used to reference other objects and components, making the code hard to maintain and understand.
Moreover, there were multiple ways of detecting interactions, including distance calculations, collision checks, and triggers. This inconsistency in approach leads to code that is harder to understand, as there's no clear standard for how interactions should be detected and handled throughout the project.
Finally I used three ways of keeping references: FindObjectOfType, SerializeField (but also using public) and sometimes variables were public but were obtained using GetComponent.
Spaghetti-Code and Unpredictable Behavior
When the influence of classes on each other is so significant that they activate and deactivate each other, it becomes a problem. It becomes unclear what happens when and why a particular component is suddenly deactivated. I noticed this especially with my enemies. In this case, a check in the attack class constantly asked for the hitpoints of the health class in order to deactivate itsself when the enemy died.
One solution to not having to deactivate components and behavior is to use either Goal-Oriented Action Planning (GOAP) or a Finite State Machine (FSM). For the scope of this project, I decided to implement an FSM for the advantage of putting the entity into a specific state, and everything that the entity should do in that state is defined within the state itself. If the enemy should attack the player without moving, you don't need to deactivate a movement script; instead, you simply don't include movement in the attack state. The transitions between the States are managed from one central class. Furthermore, this approach aligns with the Single Responsibility Principle, addressing two issues at once: it eliminates the need for classes to directly manage each other's states and ensures that each state is responsible for a single behavior.
Overusing Tags: Sacrificing Type Safety
Everything and everyone had a tag. Of course, a tag query is faster than a reading a component off an entity in runtime, but type safety should be guaranteed, which is not the case with tags (unless you use a workaround, as tags cannot be exposed in the Inspector without Editor Extensions). In this project, tags were used here for enemies, players, sensors, and areas. If a new person were to work on the project, they would have a hard time understanding it. Instead, it makes more sense to identify specific components and use them for recognition or creating your own entity-recognition component with an enum eg. Enemy, BaseEnemy, Player.
Component-Based Design
The most striking realization for me was that I didn't reuse classes effectively. For example, I had tutorial collision areas that opened doors and played voice lines, with a total of 9 classes doing the same thing, differing only in the sound they played, the door they opened, and the tag of the collision area. In the improved version, there is only one class that can be reused.
In the improved version, the collision handling functionality is consolidated into a single, reusable class that can be configured with different sounds, doors, and tags through serialized fields or other configuration mechanisms.
Performance
To improve performance, it's crucial to cache references whenever possible. Instead of repeatedly calling expensive methods like Camera.main, FindObjectOfType, or GetComponent, store the references in variables during initialization (e.g., Awake, Start or Dependency Injection) and reuse them. While SerializeField references can be convenient, it's important not to overuse them, as they can make the Inspector cluttered.
Unity's built-in collision checks, such as OnTriggerEnter or OnCollisionEnter, are highly optimized and should be used instead of manually calculating distance and check for a threshold. I am childing sensors to my GameObjects and they are being observed from their parent.