Chương 10: Quy Tắc & Checklist Code Review Chuẩn

13 Quy Tắc Bắt Buộc (Kèm Lý Do & Cặp Code Đối Chiếu)

Quy tắc 1: Không dùng Singleton (static Instance) & Không FindObjectOfType

LÝ DO (WHY): Việc dùng public static MyClass Instance tạo ra sự liên kết chặt chẽ (tight coupling) khó kiểm soát, phụ thuộc ẩn, không thể mock để viết unit test, và dễ gây rò rỉ bộ nhớ khi chuyển scene (nếu instance trỏ tới một GameObject trong Scene). Khi nhiều singleton gọi chéo nhau trong Awake(), bạn sẽ gặp lỗi không xác định thứ tự khởi tạo (race condition). Còn FindObjectOfType quét toàn bộ hệ phân cấp Scene gây tụt FPS nghiêm trọng.

Giải pháp: Đăng ký service vào ServiceLocator lúc khởi động (trong Bootstrapper hoặc GameEntry) và truy cập thông qua facade mỏng không trạng thái App.X hoặc GameApp.X.

❌ BAD CODE: Dùng Singleton static tự tạo hoặc FindObjectOfType

// BAD: Static singleton pattern creates hidden dependencies and lifecycle issues
public class GameManager : MonoBehaviour
{
    public static GameManager Instance { get; private set; }

    private void Awake()
    {
        if (Instance == null) Instance = this;
        else Destroy(gameObject);
    }
}

// In some UI button handler:
void OnBuyClick()
{
    // BAD: Direct call to singleton or FindObjectOfType
    GameManager.Instance.AddCoins(100);
    // BAD: Extremely slow scene hierarchy search
    var wallet = FindObjectOfType<GameWallet>();
}

✅ GOOD CODE: Đăng ký với ServiceLocator và truy cập qua GameApp/App

// GOOD: Plain C# service registered in GameEntry.Init()
// public sealed class GameWallet { public void AddCoins(int amount) { ... } }
// Registered in GameEntry: ServiceLocator.Register(new GameWallet());

// Accessed via clean facade forwarder
void OnBuyClick()
{
    // GOOD: Forwarder directly resolves registered service from ServiceLocator
    GameApp.Wallet.AddCoins(100);
}

Quy tắc 2: Luôn hủy đăng ký sự kiện (Unsubscribe) giữa các vòng đời khác nhau

LÝ DO (WHY): Các service nền tảng (như App.Iap, App.Ads, App.RemoteConfig) sống suốt vòng đời ứng dụng (App-lifetime, nằm trong root [Core] với DontDestroyOnLoad). Các component giao diện MonoBehaviour (như MainScreen, ShopDialog) lại sống theo vòng đời Scene hoặc đóng/mở popup (Scene-lifetime). Nếu MonoBehaviour đăng ký lắng nghe event của Service mà không hủy đăng ký khi bị hủy (Destroyed), Service sẽ giữ tham chiếu mạnh (strong reference) tới MonoBehaviour đã chết. Kết quả: Garbage Collector không thể giải phóng GameObject đó (gây rò rỉ RAM), và khi event kích hoạt sẽ ném ra lỗi MissingReferenceException!

❌ BAD CODE: Đăng ký sự kiện nhưng không hủy khi GameObject bị hủy

// BAD: Subscribes to app-lifetime service but never unsubscribes
public class ShopView : MonoBehaviour
{
    private void Start()
    {
        // Core service lives forever; ShopView will leak in memory when scene changes!
        App.Iap.Ready += OnIapReady;
    }

    private void OnIapReady()
    {
        // MissingReferenceException occurs if fired after this GameObject is destroyed!
        UpdatePrices();
    }
}

✅ GOOD CODE: Ghép cặp OnEnable/OnDisable hoặc OnOpened/OnClosed

// GOOD: Perfect pairing of subscription lifecycle
public class ShopDialog : DialogBase
{
    protected override void OnOpened()
    {
        base.OnOpened();
        // Subscribe when dialog opens
        App.Iap.Ready += OnIapReady;
        if (App.Iap.IsReady) OnIapReady();
    }

    protected override void OnClosed()
    {
        base.OnClosed();
        // Crucial: Unsubscribe immediately when dialog closes to prevent memory leaks
        App.Iap.Ready -= OnIapReady;
    }

    private void OnIapReady()
    {
        UpdatePrices();
    }
}

Quy tắc 3: Chỉ cấp phần thưởng khi kết quả là RewardedResult.Rewarded và ĐÃ LƯU

LÝ DO (WHY): Người chơi có thể đóng quảng cáo giữa chừng, bấm nút bỏ qua (Skip), hoặc gặp lỗi mất mạng khi đang xem video. Nếu bạn cấp thưởng mà không kiểm tra kết quả hoặc cấp thưởng cả khi r == RewardedResult.Skipped, studio sẽ bị thất thoát doanh thu quảng cáo nghiêm trọng và tạo lỗ hổng hack vô hạn quà. Hơn nữa, phải gọi lưu dữ liệu (Persist) ngay lập tức để tránh người chơi tắt app đột ngột rồi bị mất quà.

❌ BAD CODE: Cấp thưởng bừa bãi không kiểm tra kết quả

// BAD: Grants reward regardless of whether user watched the entire ad!
App.Ads.ShowRewarded("x2_coin", result =>
{
    // Fatal: Player might have skipped or ad failed, but coins are still added!
    GameApp.Wallet.AddCoins(100);
});

✅ GOOD CODE: Kiểm tra nghiêm ngặt kết quả Rewarded

// GOOD: Strict verification of reward completion
App.Ads.ShowRewarded(GameKeys.PlacementDoubleReward, result =>
{
    if (result == RewardedResult.Rewarded)
    {
        // Safe: Content is granted and persisted inside Wallet service
        GameApp.Wallet.AddCoins(100);
        App.Overlay.Toast("Received 100 bonus coins!");
    }
    else
    {
        CoreLog.Info("Ads", $"Rewarded not granted, result={result}");
    }
});

Quy tắc 4: Handler IAP chỉ trả về true sau khi ĐÃ CẤP VÀ LƯU TRỮ thành công

LÝ DO (WHY): Khế ước PurchaseGrantHandler quy định: khi trả về true, Unity IAP sẽ gửi xác nhận tiêu thụ giao dịch (confirm/finish transaction) lên Apple/Google. Nếu bạn trả về true trước khi lưu hoặc khi gặp lỗi, người chơi bị trừ tiền nhưng không nhận được quà. Nếu trả về false, giao dịch sẽ được lưu trong PendingOrderStore và tự động cấp phát lại vào lần mở game sau. Tuyệt đối không cấp quà trong callback của App.Iap.Purchase(...) vì callback đó chỉ phục vụ phản hồi giao diện UI!

❌ BAD CODE: Cấp quà trong callback Purchase hoặc trả về true khi chưa lưu

// BAD: Granting in the purchase callback is vulnerable to app crash before save
App.Iap.Purchase("coins_pack", result =>
{
    if (result == PurchaseResult.Success)
    {
        // DANGEROUS: If app dies here, pending store won't retry and user loses items!
        coins += 500;
    }
});

✅ GOOD CODE: Đăng ký handler duy nhất và trả về true sau khi lưu

// GOOD: Registered once via App.Iap.SetGrantHandler(GrantProduct) in GameEntry
public bool GrantProduct(string productId)
{
    switch (productId)
    {
        case GameKeys.ProductCoinsSmall:
            AddCoins(100);
            Save(); // Must persist to disk first!
            return true; // Tells Unity IAP it is safe to confirm order
        default:
            CoreLog.Warn("IAP", $"Unknown product: {productId}");
            return false; // Keeps order pending for retry
    }
}

Quy tắc 5: Tuyệt đối không dùng LINQ, không cấp phát rác, không GetComponent trong Update

LÝ DO (WHY): Ở tốc độ 60 khung hình/giây (FPS), game chỉ có 16.6 mili-giây cho mỗi frame. Việc gọi LINQ (.Where(), .Select(), .ToList()) hoặc tạo closure/lambda sinh ra các đối tượng tạm thời trên heap. Gọi GetComponent<T>() liên tục gây tốn CPU tra cứu. Hệ quả: Garbage Collector (GC) của C# phải kích hoạt liên tục để dọn dẹp, gây ra hiện tượng khựng giật khung hình (GC Spikes / Frame Drops) làm trải nghiệm chơi game rất tệ. Tham khảo Unity Garbage Collection Best Practices.

❌ BAD CODE: Dùng LINQ, new object và GetComponent trong Update

// BAD: Generates megabytes of garbage allocations every second!
void Update()
{
    // Allocates enumerator and delegate closure every single frame
    var aliveEnemies = enemies.Where(e => e.IsAlive).ToList();

    // Constant GetComponent overhead on the main thread
    GetComponent<Rigidbody2D>().velocity = Vector2.up * speed;

    // String concatenation allocates temporary strings
    scoreText.text = "Score: " + currentScore;
}

✅ GOOD CODE: Cache component trong Awake, dùng vòng for không rác

// GOOD: Zero allocations per frame
private Rigidbody2D _rb;
private readonly List<Enemy> _aliveEnemies = new List<Enemy>(32);

private void Awake()
{
    // Cache reference once
    _rb = GetComponent<Rigidbody2D>();
}

private void Update()
{
    _aliveEnemies.Clear();
    for (int i = 0; i < enemies.Count; i++)
    {
        if (enemies[i].IsAlive) _aliveEnemies.Add(enemies[i]);
    }

    _rb.velocity = Vector2.up * speed;
    // Update UI text only when score actually changes, not every frame
}

Quy tắc 6: Mọi key lưu trữ và cấu hình phải nằm trong GameKeys / CoreConfig

LÝ DO (WHY): Hardcode các chuỗi string như "coins", "level_1", hay key AdMob "ca-app-pub-..." rải rác trong code C# dẫn đến lỗi chính tả (typo) cực kỳ khó phát hiện. Ngoài ra, việc để lộ key thật trong code sẽ kích hoạt chốt chặn kiểm tra bản build (Build Guard) làm fail build release, đồng thời gây rò rỉ thông tin bí mật lên Git.

❌ BAD CODE: Rải chuỗi string tùy tiện trong code

// BAD: Magic strings are error-prone and untracked
PlayerPrefs.SetInt("coin_amount", 50); // Typo elsewhere: "coin_amounts" will silently fail!
App.Ads.TryShowInterstitial("level_finish"); // Unstandardized placement

✅ GOOD CODE: Khai báo tập trung trong GameKeys với tiền tố game.*

// GOOD: Declared in GameKeys.cs
// public const string Coins = "game.coins";
// public const string PlacementLevelEnd = "level_end";

LocalStorage.SetInt(GameKeys.Coins, 50);
App.Ads.TryShowInterstitial(GameKeys.PlacementLevelEnd);

Quy tắc 7: Chỉ chuyển Scene thông qua App.Scenes.Load(SceneId.X)

LÝ DO (WHY): Gọi thẳng SceneManager.LoadScene("Main") bằng chuỗi string vừa dễ gõ sai, vừa không qua kiểm tra index trong Build Settings, không có hiệu ứng fade che mờ chuyển cảnh của UIOverlay, và không có cơ chế chặn người chơi nhấn nhiều lần cùng lúc gây xung đột luồng nạp scene.

❌ BAD CODE: Gọi trực tiếp SceneManager bằng chuỗi

// BAD: Bypasses template fade overlay and validation
UnityEngine.SceneManagement.SceneManager.LoadScene("Main");

✅ GOOD CODE: Gọi SceneLoader qua Facade App.Scenes

// GOOD: Typesafe enum, automated fade in/out, and concurrent load protection
App.Scenes.Load(SceneId.Main);

Quy tắc 8: Mọi Coroutine phải có chú thích điều kiện dừng (Stop Condition)

LÝ DO (WHY): Một coroutine chạy ngầm mà không có điều kiện dừng rõ ràng sẽ biến thành "Zombie Coroutine", tiếp tục chạy vô tận ngay cả khi GameObject hoặc Scene đã bị hủy, gây rò rỉ bộ nhớ, tốn pin và văng lỗi NullReferenceException liên tục. Với class C# thuần không phải MonoBehaviour, luôn dùng CoreHost.Run() hoặc CoreHost.Delay().

❌ BAD CODE: Coroutine lặp vô tận không kiểm soát

// BAD: Unbounded coroutine with no clear termination condition
private IEnumerator TickLoop()
{
    while (true)
    {
        DoWork();
        yield return new WaitForSeconds(1f);
    }
}

✅ GOOD CODE: Điều kiện dừng được định nghĩa rõ ràng kèm comment

// GOOD: Stop condition clearly documented and checked every iteration
// Stop condition: runs until timer expires or round is marked as finished
private IEnumerator LevelTimerRoutine(float duration)
{
    float elapsed = 0f;
    while (elapsed < duration && !_isRoundFinished)
    {
        elapsed += Time.deltaTime;
        yield return null;
    }
    OnTimerExpired();
}

Quy tắc 9: Tách biệt rõ ràng giữa View (Giao diện) và Logic (Nghiệp vụ)

LÝ DO (WHY): Đưa code tính toán điểm, luật thắng/thua, lưu file trực tiếp vào các component UI (như MainScreen hay ResultDialog) sẽ biến UI thành "God Object" khổng lồ, rối rắm và không thể tái sử dụng. View chỉ nên làm nhiệm vụ hiển thị hình ảnh và phát ra event khi người chơi bấm nút; toàn bộ luật chơi phải nằm trong class C# thuần như LevelSession.

❌ BAD CODE: Nút UI tự xử lý toàn bộ logic tính điểm và lưu trữ

// BAD: UI component directly executes game rules and business persistence
public class WinButtonHandler : MonoBehaviour
{
    public void OnClick()
    {
        int bonus = CalculateBonus();
        int coins = PlayerPrefs.GetInt("coins") + bonus;
        PlayerPrefs.SetInt("coins", coins);
        SceneManager.LoadScene("NextLevel");
    }
}

✅ GOOD CODE: View chỉ phát event, Logic nằm trong Controller/Session

// GOOD: View only exposes user intent via events
public class MainScreen : MonoBehaviour
{
    public event Action WinClicked;
    public void NotifyWin() => WinClicked?.Invoke();
}

// Logic handled in GameplayController / LevelSession
public class GameplayController : MonoBehaviour
{
    private void OnWinClicked()
    {
        EndLevel(won: true); // Manages session, rewards, and dialogs cleanly
    }
}

Quy tắc 10: Thư mục Core TUYỆT ĐỐI KHÔNG tham chiếu sang Game

LÝ DO (WHY): TapBrain.Core.asmdef được thiết kế để làm lõi nền tảng dùng chung cho hàng chục game prototype khác nhau của studio. Nếu code Core gọi trực tiếp tới class của Game, Core sẽ bị phụ thuộc vòng và compiler sẽ báo lỗi ngay lập tức. Core chỉ giao tiếp với Game qua interface, event hoặc delegate (như PurchaseGrantHandler, NotificationService.ScheduleRequested).

❌ BAD CODE: Core gọi trực tiếp code trong thư mục Game

// In Assets/_Project/Core/Services/IAP/IapService.cs:
// BAD: Core references game-specific wallet class! Core asmdef cannot compile this.
TapBrain.Game.GameWallet.Instance.AddCoins(100);

✅ GOOD CODE: Core định nghĩa delegate trừu tượng để Game đăng ký vào

// In Core: Define abstract delegate contract
public delegate bool PurchaseGrantHandler(string productId);
public void SetGrantHandler(PurchaseGrantHandler handler) => _grantHandler = handler;

// In Game (GameEntry.cs): Connect the game wallet implementation to Core
App.Iap.SetGrantHandler(wallet.GrantProduct);

Quy tắc 11: Không tạo file asmdef bên trong thư mục Assets/_Project/Game/

LÝ DO (WHY): Khi một thư mục có file asmdef, mã nguồn trong đó không thể truy cập mã nguồn của Assembly-CSharp. Khi làm prototype nhanh, các bạn lập trình viên thường tải nhiều asset hay plugin từ Unity Asset Store (như DOTween, Odin, Spine...) và phần lớn các plugin này không có asmdef riêng mà nằm ở Assembly-CSharp. Nếu tạo asmdef cho Game/, bạn sẽ bị chặn không thể dùng các asset này nếu không tự viết asmdef cho từng asset. Giữ Game/ trong Assembly-CSharp là quyết định kiến trúc có chủ đích để thân thiện nhất với các bạn dev mới.

Quy tắc 12: Thận trọng với các menu TapBrain > Setup (Ghi đè Scene & Prefab)

LÝ DO (WHY): Các menu như Rebuild Scenes hay Run Full Template Setup được viết ra để tự động dựng lại toàn bộ cấu trúc dự án mẫu từ đầu. Chúng sẽ xóa và ghi đè toàn bộ 3 scene Loading, Home, Main và các Dialog prefab. Nếu dự án của bạn đã tùy biến level, kéo thả art mới thì việc chạy menu này sẽ xóa sạch công sức làm việc mà không thể Undo!

Quy tắc 13: Tuyệt đối không commit Keystore, file bí mật hoặc Private Key lên Git

LÝ DO (WHY): File *.keystore chứa chữ ký số bảo mật để phát hành ứng dụng lên Google Play. Nếu keystore bị lộ lên Git public/shared, kẻ xấu có thể mạo danh studio để tung ra các bản cập nhật độc hại. Nếu mất mật khẩu hoặc mất keystore, bạn sẽ không bao giờ có thể cập nhật lại game trên Google Play được nữa. Hãy luôn lưu keystore bên ngoài thư mục dự án và lưu trữ an toàn trong trình quản lý mật khẩu của studio.

2. Bảng Checklist Code Review Sao Chép Được (Copyable)

Dưới đây là mẫu Checklist chuẩn định dạng Markdown. Khi tạo Pull Request trên GitHub hoặc GitLab, hãy copy toàn bộ khối dưới đây dán vào phần mô tả PR:

### 📋 TapBrain PR Code Review Checklist

#### 1. 🏗️ Kiến trúc & Thiết kế (Architecture)
- [ ] Không có Singleton (`static Instance`), không có `FindObjectOfType` cho services.
- [ ] Mọi service truy cập qua `App.X` (Core) hoặc `GameApp.X` (Game) hoặc `ServiceLocator.Get()`.
- [ ] Core hoàn toàn không tham chiếu sang Game (Core asmdef độc lập).
- [ ] Không tạo thêm file `asmdef` trong thư mục `Assets/_Project/Game/`.
- [ ] Chuyển scene duy nhất bằng `App.Scenes.Load(SceneId.X)`.

#### 2. ⚡ Hiệu năng & Bộ nhớ (Performance & Memory)
- [ ] Không dùng LINQ, không cấp phát `new`, không `GetComponent` trong hàm `Update/FixedUpdate`.
- [ ] Các component MonoBehaviour hủy đăng ký sự kiện (Unsubscribe) đầy đủ trong `OnDisable` hoặc `OnClosed`.
- [ ] Các Coroutine đều có chú thích điều kiện dừng (Stop Condition) rõ ràng.
- [ ] Không có tài nguyên ảnh/âm thanh chưa nén hoặc kích thước quá lớn không cần thiết.

#### 3. 💰 Doanh thu & Dịch vụ (Monetization & Services)
- [ ] Rewarded Ad chỉ cấp quà khi `RewardedResult.Rewarded` và đã lưu vào disk (`Save()`).
- [ ] IAP Grant Handler chỉ trả về `true` sau khi vật phẩm đã được cấp phát và lưu an toàn.
- [ ] Không cấp vật phẩm IAP trong callback của `App.Iap.Purchase(...)`.
- [ ] Giãn cách quảng cáo Interstitial tuân thủ chính sách của `AdsPolicy`.

#### 4. 🛡️ Tính ổn định & An toàn (Stability & Safety)
- [ ] Mọi key lưu trữ nằm trong `GameKeys` (tiền tố `game.*`), cấu hình SDK trong `CoreConfig`.
- [ ] Game khởi động và chơi bình thường khi ngắt toàn bộ kết nối mạng (Offline test).
- [ ] Mở trực tiếp scene `Main` hoặc `Home` rồi bấm Play vẫn khởi động an toàn.
- [ ] Không commit file `.keystore`, `.pem`, Service Account JSON hay secret key lên Git.

#### 5. 🧹 Vệ sinh mã nguồn (Clean Code)
- [ ] Comment và Log viết bằng tiếng Anh chuẩn.
- [ ] Console không có bất kỳ thông báo lỗi biên dịch (`error CS`) nào.
- [ ] Không chạy menu `Rebuild Scenes` làm ghi đè các tùy biến scene của dự án.