Refactoring Unity Spaghetti Code

Given a messy script like this:

CSHARP
public class Player : MonoBehaviour {
    public int hp = 100;
    public float spd = 5f;
    public GameObject bul;
    public float t = 0;
    void Update() {
        t += Time.deltaTime;
        if (Input.GetKey(KeyCode.Space) && t > 0.2f) {
            Instantiate(bul, transform.position, Quaternion.identity);
            t = 0;
        }
        transform.position += new Vector3(Input.GetAxis("Horizontal"), 0, 0) * spd * Time.deltaTime;
    }
}

Refactor into decoupled, named, single-responsibility systems:

CSHARP
public class PlayerHealth : MonoBehaviour {
    [SerializeField] private int maxHealth = 100;
    private int currentHealth;

    public event Action<int, int> OnHealthChanged;

    private void Awake() => currentHealth = maxHealth;

    public void ApplyDamage(int damageAmount) {
        currentHealth = Mathf.Max(0, currentHealth - damageAmount);
        OnHealthChanged?.Invoke(currentHealth, maxHealth);
    }
}

public class PlayerMovement : MonoBehaviour {
    [SerializeField] private float moveSpeed = 5f;

    public void MoveHorizontally(float horizontalInput) {
        transform.position += Vector3.right * horizontalInput * moveSpeed * Time.deltaTime;
    }
}

public class PlayerWeapon : MonoBehaviour {
    [SerializeField] private GameObject projectilePrefab;
    [SerializeField] private float fireCooldownSeconds = 0.2f;
    private float timeSinceLastShot;

    public void TickCooldown() => timeSinceLastShot += Time.deltaTime;

    public bool TryFire() {
        if (timeSinceLastShot < fireCooldownSeconds) return false;
        Instantiate(projectilePrefab, transform.position, Quaternion.identity);
        timeSinceLastShot = 0f;
        return true;
    }
}

public class PlayerController : MonoBehaviour {
    [SerializeField] private PlayerMovement movement;
    [SerializeField] private PlayerWeapon weapon;

    private void Update() {
        movement.MoveHorizontally(Input.GetAxis("Horizontal"));
        weapon.TickCooldown();
        if (Input.GetKey(KeyCode.Space)) weapon.TryFire();
    }
}

Progress:
- [ ] Step 1: Read and map the existing code's actual behavior (don't trust names/comments)
- [ ] Step 2: Identify responsibilities mixed together in each class/method
- [ ] Step 3: List all variables/functions needing renaming; draft clear names
- [ ] Step 4: Extract each responsibility into its own class/component
- [ ] Step 5: Define interfaces/events for cross-component communication
- [ ] Step 6: Replace direct references with dependency injection (SerializeField/constructor)
- [ ] Step 7: Rename everything using Unity/C# naming conventions
- [ ] Step 8: Verify behavior parity (manual test or quick playtest)
- [ ] Step 9: Add minimal XML doc comments on public API only where non-obvious
- [ ] Step 10: Organize files/folders by feature (not by type)

Step 1 — Trace behavior first. Never rename or split code before understanding what it actually does at runtime. Spaghetti code often has misleading names already.

Step 2 — Spot God Objects. Red flags: a Player, GameManager, or Enemy class handling input, physics, UI, audio, and save data simultaneously. Each of these becomes a separate component.

Step 3 — Naming pass. Build a rename table before touching code:

Step 4 — Extract by responsibility, typical splits for common Unity objects:

• XMovement — physics/transform manipulation
• XHealth — hp, damage, death
• XWeapon/XAbility — attack logic
• XInputHandler — raw input to abstract commands
• XController/XCoordinator — orchestrates the above, nothing else
• XView/XPresenter — UI/visual feedback only

Step 5 — Communication patterns, pick based on context:

• Events/Actions (C# event) — one-to-many, decoupled notifications (health changed, died)
• Interfaces (IDamageable, IInteractable) — polymorphic behavior across unrelated types
• ScriptableObject event channels — cross-scene or designer-tunable communication
• Direct reference via [SerializeField] — tightly related sibling components on same GameObject only

Step 6 — Dependency direction. High-level orchestrators (PlayerController) depend on low-level components (PlayerMovement), never the reverse. Low-level components should not know their orchestrator exists.

Example 1: Input:

CSHARP
public class EM : MonoBehaviour {
    public float h=50, s=3; GameObject p;
    public void Dmg(float d){ h-=d; if(h<=0) Destroy(gameObject); }
    void Update(){ transform.position = Vector3.MoveTowards(transform.position, p.transform.position, s*Time.deltaTime); }
}

Output:

CSHARP
public class EnemyHealth : MonoBehaviour {
    [SerializeField] private float maxHealth = 50f;
    private float currentHealth;
    public event Action OnDeath;

    private void Awake() => currentHealth = maxHealth;

    public void ApplyDamage(float damageAmount) {
        currentHealth -= damageAmount;
        if (currentHealth <= 0f) OnDeath?.Invoke();
    }
}

public class EnemyChaseMovement : MonoBehaviour {
    [SerializeField] private float moveSpeed = 3f;
    [SerializeField] private Transform target;

    private void Update() {
        if (target == null) return;
        transform.position = Vector3.MoveTowards(transform.position, target.position, moveSpeed * Time.deltaTime);
    }
}

public class EnemyController : MonoBehaviour {
    [SerializeField] private EnemyHealth health;

    private void Awake() => health.OnDeath += HandleDeath;
    private void HandleDeath() => Destroy(gameObject);
}

Example 2: Input: A single 800-line GameManager.cs handling score, scene loading, pause menu, audio, and enemy spawning via static singleton with public mutable fields. Output: Split into ScoreTracker, SceneLoader, PauseMenuController, AudioManager, EnemySpawner — each a focused MonoBehaviour or plain C# class, coordinated by a thin GameSession class that only wires references (no logic), communicating via events/interfaces instead of static field access.