123
84

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 1 year has passed since last update.

UniRxアンチパターン集

Last updated at Posted at 2022-02-12

はじめに

いままでいくつか「UniRxをこう使うといいよ」に着目していろいろ記事を書いてきました。
その結果としてUniRxがかなり一般的に使われるライブラリとなったため個人的にはとても喜ばしいと感じていました。

その一方で、「無茶苦茶なUniRxの使い方」が広まってしまっているとも感じています。
というのも「こう使うといいよ」については触れてきたが、「こう使うべきではない」点についてはあまり触れてこなかったためです。

そこで今回はUniRxにおけるアンチパターンをいくつか紹介し、その回避方法について説明します。

※ 文中に「Observable」という単語がでてきますが、ニュアンスとしては「≒UniRx」だと思って読んでください。

アンチパターンとは

アンチパターンとは、「一般的に避けるべきといわれる、好ましくない使い方」を指します。
そのパターンを用いることにメリットが無く、むしろデメリットのほうが大きいようなものを指します。
(ハッキリいってしまうと、「UniRxを用いたクソコード」です)

なぜそれがアンチパターンなのかを理解し、覚悟を持った上であえて採用する分には何もいうことはありません。
ですが知らずのうちにアンチパターンへ陥ってるような場合は早急に改善が必要です。

UniRxアンチパターン集

アンチパターン: Disposeしてない

DisposeとはIDisposableインタフェースから提供されるメソッドです。
Disposeを呼び出すことでオブジェクトが正常終了し、メモリの解放の準備などが実行されます。
これの呼び出しを忘れた場合、メモリリークしてしまったり場合によってはクラッシュの原因になることもあります。
そのため**IDisposableなオブジェクトを触る場合は常にDisposeもセットで行うことを意識する必要があります。**

UniRxにおいてもIDisposableなオブジェクトはいくつか存在します。
それらを大まかに分けると次の2つに分類できます。

  • Subscribeに対するDispose
  • ストリームソースに対するDispose

それぞれのDispose方法や、それを忘れた時に何が起きるのかについて解説します。

Subscribeに対するDispose

概要

SubscribeとはUniRxにおいて、ストリーム(Observable)を購読する時に実行するメソッドです。
このSubscribeIObservable<T>インタフェースに、次のように定義されています。

namespace System
{
    public interface IObservable<out T>
    {
        IDisposable Subscribe(IObserver<T> observer);
    }
}

このように、**Subscribeの返り値はIDisposable**になっています。
これは「SubscribeするとObservableに対する購読が始めるが、この購読を中止したい場合はDisposeを呼んでね」という意味になっています。

Disposeを忘れるとどうなるのか

SubscribeのDisposeを忘れた場合、裏で購読状態がずっと残ってしまいます。

Disposeを忘れていたとしてもすぐには問題にならないことが多く、質が悪いです。
(何かしらのGameObjectを破棄したタイミングで突然エラーが出たり、またはずっと裏で動いたままCPUを無駄に消費し続けたり)

そもそもUniRxは「Observable側がSubscribe側に対してメッセージを投げる」という仕組みになっています。
そのため「メッセージを受け取る側がもう存在しないのにSubscribeが残ったままになっている」という状況になってしまうと、
NullReferenceExceptionなどの例外が飛んでエラーだらけになってしまったりします。

たとえば、次のような実装を見てみましょう。
Observable.EveryFixedUpdateを用いているにもかかわらず、Disposeをし忘れているパターンです。

using UniRx;
using UnityEngine;

namespace AntiPatterns.NoDispose
{
    public class Enemy : MonoBehaviour
    {
        void Start()
        {
            var rigidBody = GetComponent<Rigidbody>();
            
            // 毎フレーム移動する
            Observable
                .EveryFixedUpdate()
                .Subscribe(_ =>
                {
                    // Rigidbodyにアクセス
                    rigidBody.AddForce(Vector3.forward, ForceMode.VelocityChange);
                });
        }

        /// <summary>
        /// 何かに衝突したら消える
        /// </summary>
        private void OnCollisionEnter(Collision _)
        {
            Destroy(gameObject);
        }
    }
}

このようにオブジェクトがDestroyされたタイミングで例外が出てしまいました。
例外の内容は「Destroyされたオブジェクト(Rigidbody)に対してアクセスが発生してしまった」です。

なぜこうなったかというと、「オブジェクトが破棄された際にObservable.EveryFixedUpdateの購読を止めなかった」からです。

対策

SubscribeDispose漏れ」の対策としては使い終わったタイミングでDisposeを徹底するです。
ひとまずMonoBehaviour上であればAddTo(this)を使えばなんとかなるため、MonoBehaviourを使っているならAddToをつけることを意識するとよいでしょう。

using UniRx;
using UnityEngine;

namespace AntiPatterns.NoDispose
{
    public class Enemy : MonoBehaviour
    {
        void Start()
        {
            var rigidBody = GetComponent<Rigidbody>();
            
            // 毎フレーム移動する
            Observable
                .EveryFixedUpdate()
                .Subscribe(_ =>
                {
                    // Rigidbodyにアクセス
                    rigidBody.AddForce(Vector3.forward, ForceMode.VelocityChange);
                })
                // AddTo(this)することで、このGameObjectが破棄された時に
                // 自動的にDisposeを呼び出してくれる
                .AddTo(this); 
        }

        /// <summary>
        /// 何かに衝突したら消える
        /// </summary>
        private void OnCollisionEnter(Collision _)
        {
            Destroy(gameObject);
        }
    }
}

もしSubscribeの呼び出し元がMonoBehaviourを使っていない場合は、そのクラスそのものをIDisposableにしてしまい、
Dispose()されたときにSubscribeも同時にDispose()するなどすると良いでしょう。

using System;
using UniRx;
using UnityEngine;

namespace AntiPatterns.NoDispose
{
    // Pureなクラス上で、何か別クラスのObservableを購読したとした場合
    public sealed class PureClassObserver : IDisposable
    {
        private readonly IDisposable _disposable;

        public PureClassObserver()
        {
            _disposable = Observable.EveryUpdate()
                .Subscribe(_ =>
                {
                    Debug.Log("なにか処理してる");
                });
        }

        // このクラスがDisposeされたら購読も止める
        public void Dispose()
        {
            _disposable.Dispose();
        }
    }
}

また、AddToCompositeDisposableCancellationTokenに対しても紐付けることができます。
CancellationTokenへの紐付けはUniTaskが必要)
まとめて複数の購読を中止したい場合はこちらも活用すると良いでしょう。

using System;
using System.Threading;
using Cysharp.Threading.Tasks; //いる
using UniRx;
using UnityEngine;

namespace AntiPatterns.NoDispose
{
    public sealed class PureClassObserver : IDisposable
    {
        // CancellationTokenSource
        private readonly CancellationTokenSource _cancellationTokenSource 
            = new CancellationTokenSource();

        public PureClassObserver()
        {
            Observable.EveryUpdate()
                .Subscribe(_ =>
                {
                    Debug.Log("なにか処理してる");
                })
                // CancellationTokenにAddTo
                .AddTo(_cancellationTokenSource.Token);
        }

        // このクラスがDisposeされたら購読も止める
        public void Dispose()
        {
            _cancellationTokenSource.Cancel();
            _cancellationTokenSource.Dispose();
        }
    }
}

他にも直接Disposeを呼ばずに、OnCompletedメッセージを経由して間接的にDisposeを実行するといった手法もあったりします。
TakeUntilDestroyを使うなど)

ストリームソースに対するDispose

Subscribeに対するDisposeについては、AddToを使っている人が多いのでわりとどんなコードでも徹底されている印象はあります。
しかしその一方でストリームソースに対するDisposeはケアしていない人が多い印象を受けます。

ストリームソースとは、Observableの源流となるオブジェクトを指します。
たとえば次のようなオブジェクトです。

  • Subject<T>
  • ReactiveProperty<T>
  • IConnectableObservable<T>

これらオブジェクトはすべてIDisposableとして定義されており、使い終わった時にDispose()を呼び出すことが推奨されます。

using System;
using System.Threading;
using Cysharp.Threading.Tasks;
using UniRx;
using UnityEngine;

namespace AntiPatterns.NoDispose
{
    /// <summary>
    /// ReactivePropertyのDispose忘れている例
    /// </summary>
    public sealed class TimerProvider : MonoBehaviour
    {
        // どこにもReactivePropertyのDisposeの記述がない
        private readonly ReactiveProperty<int> _timer 
            = new ReactiveProperty<int>(30);
        
        /// <summary>
        /// 現在のタイマー値
        /// </summary>
        public IReadOnlyReactiveProperty<int> CurrentTime => _timer;

        private void Start()
        {
            // 1秒ごとにカウントダウンする
            LoopAsync(this.GetCancellationTokenOnDestroy()).Forget();
        }

        private async UniTaskVoid LoopAsync(CancellationToken token)
        {
            while (!token.IsCancellationRequested)
            {
                if (_timer.Value <= 0) break;
                
                _timer.Value--;
                await UniTask.Delay(TimeSpan.FromSeconds(1));
            }
        }
    }
}

Disposeを忘れるとどうなるのか

Observable側でDisposeを実行すると、そのObservableに対する購読をまとめてDisposeすることができます。
そのためObservable側のDisposeを行うことで、購読をすべて止めることができるため、より安全にObservableの破棄ができます。

なおReactivePropertyのみ特殊で、これは「Dispose()されたタイミングでOnCompletedメッセージを発行する」という挙動になっています。
そのためReactivePropertyを使っているならなおさらDisposeの扱いには注意する必要があります。

Observable側のDispose漏れはそれがただちに致命的な問題に繋がることは多くありません。
せいぜいGCによる回収が遅延する程度で、致命的にメモリリークしたりすることは多くないです。
ですがIDisposableなオブジェクトを放置すること自体がC#の作法としてよろしくないので、徹底してDispose()する癖をつけることを推奨します。

対策

SubjectReactivePropertyも忘れずに使い終わったタイミングでDispose()するようにしましょう。
実はこれらもAddToが使えるので、Awake()Start()あたりでAddToしておいてもOKです。

using System;
using System.Threading;
using Cysharp.Threading.Tasks;
using UniRx;
using UnityEngine;

namespace AntiPatterns.NoDispose
{
    /// <summary>
    /// Dispose忘れているやつ
    /// </summary>
    public sealed class TimerProvider : MonoBehaviour
    {
        // どこにもReactivePropertyのDisposeの記述がない
        private readonly ReactiveProperty<int> _timer 
            = new ReactiveProperty<int>(30);
        
        /// <summary>
        /// 現在のタイマー値
        /// </summary>
        public IReadOnlyReactiveProperty<int> CurrentTime => _timer;

        private void Awake()
        {
            // OnDestroy()で _timer.Dispose() するのと同義
            _timer.AddTo(this);
        }

        private void Start()
        {
            // 1秒ごとにカウントダウンする
            LoopAsync(this.GetCancellationTokenOnDestroy()).Forget();
        }

        private async UniTaskVoid LoopAsync(CancellationToken token)
        {
            while (!token.IsCancellationRequested)
            {
                if (_timer.Value <= 0) break;
                
                _timer.Value--;
                await UniTask.Delay(TimeSpan.FromSeconds(1));
            }
        }
    }
}

アンチパターン: なんでもUniRxで書こうとする

これは2022年現在におけるUniRxの使いみちでも解説したのですが、
「UniRxで書くべき部分」と「UniRxを使わないほうがいい部分」を意識しましょうという話です。

詳しくは上記の記事を読んでほしいのですが、ざっくりとUniRxを使うべきでない部分を挙げると次になります。

  • 複雑なロジックを無理やりオペレータチェインで書こうとしている
  • 非同期処理をObservableで扱おうとしている

とくに「ロジックを無理やりオペレータチェインで書こうとしている」はUniRx使い始めの方がよく陥る現象です。
「UniRxの万能感を信じて、すべてのコードをUniRxで書けばきっとキレイになる」という状態です。
そもそもUniRxは決して万能ではなく、むしろその得意不得意をしっかり意識して使わないといけないライブラリです。

「せっかくUniRxを導入したのだからすべてをUniRxで書こう」ではなく、「本当にここでUniRxを使うべきなのか」を考えるようにするとよいでしょう。

アンチパターン: オペレータの使い方がおかしい(副作用を書く)

※このミスをやらかす初心者が非常に多いです。

間違ったオペレータの使い方にもいくつかあるのですが、とくに注意してほしいのは次の1点です。

  • オペレータ内で副作用を起こしている
  • たとえば、Whereの中でフィールド変数のList<T>に書き込んでたり
  • たとえば、Select内で他のSubject.OnNextをキックしてたり

これは絶対にNGな行為なので、UniRxを使うときはしっかりとこの点を意識するようにしてください。
オペレータ内で副作用はダメ、ゼッタイ。

そもそも「副作用」とは

そもそも「副作用」が何を意味するかわからない方もいるでしょう。
プログラミングにおける「副作用」の意味は、「関数を実行した時にその関数の外の状態を変化させてしまうこと」です。
もっと簡単にいうと、「処理の途中で外の変数を上書きする」といった処理を指します。

たとえば次のようなAdd関数があったとします。
これはうけた引数を用いて計算して、その結果を返すだけなので副作用はありません。

/// <summary>
/// 足し算する関数
/// これは副作用なし
/// </summary>
public int Add(int x, int y)
{
    return x + y;
}

一方で、次のようなAdd2関数があったとします。
これは関数の挙動が外部変数に依存しており、関数を実行するたびに_sumの値が変動します。
そのためこの関数は同じ引数でも呼び出すたびに異なる結果を返すことになります。

/// <summary>
/// 合計値を保持するフィールド変数
/// </summary>
private int _sum = 0;

/// <summary>
/// 足し算する関数
/// 外部変数の状態を書き換えるので副作用あり
/// </summary>
public int Add2(int a)
{
    _sum += a;
    return _sum;
}

そもそも「副作用」はオブジェクト指向プログラミングにおいて避けて通れないものです。
「オブジェクト」に「状態」を内包させ「メソッド」経由で副作用を起こしてオブジェクトの状態を変化させる、がオブジェクト指向の基本的な考え方です。
そのため副作用そのものが悪いわけではありません。

副作用はそれが許される場所と許されない場所がある」ということです。

オペレータ内で副作用がなぜ許容されないか

たとえば、次のような副作用を含んだUniRxのコードがあったとします。
とくにWhereの使い方に注目してください。

using System;
using UniRx;
using UniRx.Triggers;
using UnityEngine;

namespace AntiPatterns.SideEffects
{
    public sealed class Player : MonoBehaviour
    {
        private Enemy _lastHitEnemy;

        private void Start()
        {
            // 衝突判定
            this.OnCollisionEnterAsObservable()
                // 敵にぶつかったか判定し、敵だったらフィールドに保存しておく
                .Where(x =>
                {
                    if (x.gameObject.TryGetComponent<Enemy>(out var e))
                    {
                        // フィールドに最後にぶつかった敵を記憶する
                        _lastHitEnemy = e;
                        return true;
                    }

                    return false;
                })
                // 衝突したら1秒間無敵
                .ThrottleFirst(TimeSpan.FromSeconds(1))
                .Subscribe(x =>
                {
                    // ぶつかった場所にエフェクト再生
                    PlayHitEffect(x.contacts[0].point);
                })
                .AddTo(this);
        }

        /// <summary>
        /// 指定座標にエフェクトを再生する
        /// </summary>
        private void PlayHitEffect(Vector3 point)
        {
            // 省略
        }
    }
}

Whereの中で外部の変数_lastHitEnemyを上書きする処理を実行しています。
これは紛れもなく「副作用」であり、NGな使い方です。

オペレータの処理内に副作用を混ぜてしまうとそのオペレータのもつ責務が曖昧となり、可読性が著しく落ちてしまいます。

本来Whereオペレータは「OnNextメッセージを条件にしたってフィルタリングする」という責務をもたせるために用います。
これは「コードを書いた人間」と「そのコードを読む人間」とで共通認識であるべきです。
あとから他人がコードを読んだ時に「Whereを使ってるということは、ここは条件判定だけしてるんだな」と、Whereという名前を見ただけで一瞬で判断できる状態になっているべきです。

同様に他のオペレータにもそれぞれ応じた責務があります。

Selectということは、ここで型変換するんだな」
Delayということは、ここで遅延を挟むんだな」
Take(1)ということは、1回だけ処理を走らせるつもりなんだな」

といったように、オペレータ名を見るだけで中身をちゃんと読まなくても全体の流れがすぐ把握できるようになっていることが可読性を保つ上で重要な点です。

たとえばさきほどのこのコードですが。

// 衝突判定
this.OnCollisionEnterAsObservable()
    // 敵にぶつかったか判定し、敵だったらフィールドに保存しておく
    .Where(x =>
    {
        if (x.gameObject.TryGetComponent<Enemy>(out var e))
        {
            // フィールドに最後にぶつかった敵を記憶する
            _lastHitEnemy = e;
            return true;
        }

        return false;
    })
    // 衝突したら1秒間無敵
    .ThrottleFirst(TimeSpan.FromSeconds(1))
    .Subscribe(x =>
    {
        // ぶつかった場所にエフェクト再生
        PlayHitEffect(x.contacts[0].point);
    })
    .AddTo(this);

まだラムダ式でWhere内に直接書いてある分には副作用が隠れていることにまだ気づけます。

これがもし、Whereの中身が別のメソッドに切り出してあったらどうなるでしょうか。

private void Start()
{
    // 衝突判定
    this.OnCollisionEnterAsObservable()
        // 敵にぶつかったか判定し、敵だったらフィールドに保存しておく
        .Where(CheckEnemy)
        // 衝突したら1秒間無敵
        .ThrottleFirst(TimeSpan.FromSeconds(1))
        .Subscribe(x =>
        {
            // ぶつかった場所にエフェクト再生
            PlayHitEffect(x.contacts[0].point);
        })
        .AddTo(this);
}

private bool CheckEnemy(Collision x)
{
    if (x.gameObject.TryGetComponent<Enemy>(out var e))
    {
        // フィールドに最後にぶつかった敵を記憶する
        _lastHitEnemy = e;
        return true;
    }

    return false;
}

このコードを読んだ時、「Where実行時に副作用がある」とすぐ気付けるでしょうか。
それぞれのオペレータが呼び出しているメソッドをすべて精査して副作用が隠れていないかを確認しないとこのコードの挙動を完全に把握することができなくなります。
今回は呼び出してるメソッドがCheckEnemy()だけですが、もっと複雑なコードで呼び出しているメソッドが多岐に渡る場合、把握はより一層困難なものとなります。

そしてこのコードの把握を困難にしている最大の原因は「オペレータ内で副作用を起こしている」からです。
そのためオペレータ内での副作用を禁止すれば困難さは勝手に解消されます

もう一度いいます。オペレータ内に副作用を書くな。マジ。

副作用との付き合い方

実装する上でどうしても副作用が必要になるケースはありえます。
そういった場合、副作用との付き合い方は2つあります。

  • 副作用はSubscribe()の中のみに限定する
  • どうしようもないときの最終手段としてDo()を使う

副作用との付き合い方:Subscribeの中のみに限定する

とくにひねったような解決策でもなく、「副作用はSubscribe()の中まで我慢しろ」という話です。
たとえば、さきほどの副作用を含んでいたコードも次のように書き直すことができます。

using System;
using UniRx;
using UniRx.Triggers;
using UnityEngine;

namespace AntiPatterns.SideEffects
{
    public sealed class Player : MonoBehaviour
    {
        private Enemy _lastHitEnemy;

        private void Start()
        {
            // 衝突判定
            this.OnCollisionEnterAsObservable()
                // 型を変換
                .Select(x => (Collision: x, Enemy: x.gameObject.GetComponent<Enemy>()))
                // 衝突相手がEnemyだったか
                .Where(t => t.Enemy != null)
                // 衝突したら1秒間無敵
                .ThrottleFirst(TimeSpan.FromSeconds(1))
                .Subscribe(t =>
                {
                    var (collision, enemy) = t;

                    // 最後にぶつかった敵を保存(副作用)
                    _lastHitEnemy = enemy;
                    
                    // ぶつかった場所にエフェクト再生
                    PlayHitEffect(collision.contacts[0].point);
                })
                .AddTo(this);
        }

        /// <summary>
        /// 指定座標にエフェクトを再生する
        /// </summary>
        private void PlayHitEffect(Vector3 point)
        {
            // 省略
        }
    }
}

Select -> Where と多段になっているところに若干のコストを感じるかもしれませんが、この程度でパフォーマンスに影響がでることはありません
効果があるのかよくわからない最適化を求めて副作用だらけのコードを書くくらいなら、多少のコストを払って可読性が高いコードを書いたほうが遥かにマシです。

副作用との付き合い方:Doを使う

どうしてもObservableの途中で副作用が必要になった時、その救済策としてDo()というオペレータが用意されています。
ですが「Do()を使ってれば副作用は起こしまくってOK」ではありません

Do()はいわば「マジでどうしようもないときの最終手段」であり、本来使うべきではありません。
オペレータとしてDo()が存在はするが、使うべきではないです。

(デバッグ用途でログを出すならDebug()オペレータ使ったほうが楽です)

アンチパターン: Doを多用する

安易にDo()を使うな

さきほど、「副作用と付き合うためにはDo()を使うしかない」と言いましたが、これは決して**「Do()を使ってるなら副作用コードは書いてもOK」という意味ではありません。**
そもそもの副作用を含んだ処理自体を敬遠すべきであり、Do()を使ってようが使ってまいがそもそも副作用を含んだコードをUniRxで書くなです。

たとえば次のDo()を使ってがんばってる次のコード。
やりたいことはわかるし、そうせざるを得なかった気持ちもわかります。

using System;
using UniRx;
using UniRx.Triggers;
using UnityEngine;

namespace AntiPatterns.DoWoTsukauna
{
    public sealed class Player : MonoBehaviour
    {
        /// <summary>
        /// 無敵フラグ
        /// </summary>
        public bool IsInvincible { get; private set; }

        private void Start()
        {
            // 敵との衝突処理
            // Doを使ってるあまり良いとは言えないコード
            this.OnCollisionEnterAsObservable()
                // 無敵状態じゃない
                .Where(_ => !IsInvincible)
                // ぶつかった相手が敵
                .Where(x => x.gameObject.TryGetComponent<Enemy>(out _))
                // 無敵状態にしてダメージ処理を実行
                .Do(_ =>
                {
                    IsInvincible = true;
                    OnDamage();
                })
                // 3秒無敵
                .Delay(TimeSpan.FromSeconds(3))
                // 無敵解除
                .Subscribe(_ => IsInvincible = false)
                .AddTo(this);
        }

        /// <summary>
        /// ダメージを受けた処理
        /// </summary>
        private void OnDamage()
        {
            // なんかする
        }
    }
}

Observableの途中で副作用を起こして、さらにそれをDelayなどで遅延させて、継続して別の副作用を起こすというもの。
たしかに実装していてこういうパターンは出てきますし、Do()を使えばスマートに書けた気持ちになれます。

ですが「そもそもこれ別にDo()使わなくても書けるんじゃないか」という観点で一度見て頂きたいです。
Do()を使わずにもっとスマートに書けるならそれに越したことはないです。

Do()を回避する方法

回避策1:Subscribe()に書く

Observableの途中にDo()を挟んでそこに副作用コードを書くことはNGです。
しかし末端部分であるSubscribe()内であれば副作用コードは許容されるので、副作用をここに寄せてしまうやり方です。

using System;
using UniRx;
using UniRx.Triggers;
using UnityEngine;

namespace AntiPatterns.DoWoTsukauna
{
    public sealed class Player : MonoBehaviour
    {
        /// <summary>
        /// 無敵フラグ
        /// </summary>
        public bool IsInvincible { get; private set; }

        private void Start()
        {
            // 敵との衝突処理
            // Doを使ってるあまり良いとは言えないコード
            this.OnCollisionEnterAsObservable()
                // 無敵状態じゃない
                .Where(_ => !IsInvincible)
                // ぶつかった相手が敵
                .Where(x => x.gameObject.TryGetComponent<Enemy>(out _))
                .Subscribe(_ =>
                {
                    // 無敵状態にしてダメージ処理を実行
                    IsInvincible = true;
                    OnDamage();

                    // 3秒後に無敵フラグ解除
                    Observable.Timer(TimeSpan.FromSeconds(3))
                        .TakeUntilDestroy(this)
                        .Subscribe(_ =>
                        {
                            IsInvincible = false;
                        });
                })
                .AddTo(this);
        }

        /// <summary>
        /// ダメージを受けた処理
        /// </summary>
        private void OnDamage()
        {
            // なんかする
        }
    }
}

Subscribe()内で別のObservableSubscribe()するのは問題ありません。
Do()を使って無理やり1本のObservableにするよりかは、多少ダサくても多重Subscribe()になっていた方が見通しはよいです。

(それでもDo()の方がスマートだ!と思うのなら止めはしませんが…)

回避策2:async/awaitで書く

そもそも副作用が入り乱れるコードをObservableで扱うことが無理であったりします。
そういう場合はUniTaskasync/awaitで手続き的に書いてしまうのも有効です。

using System;
using System.Threading;
using Cysharp.Threading.Tasks;
using Cysharp.Threading.Tasks.Triggers;
using UnityEngine;

namespace AntiPatterns.DoWoTsukauna
{
    public sealed class Player : MonoBehaviour
    {
        /// <summary>
        /// 無敵フラグ
        /// </summary>
        public bool IsInvincible;

        private void Start()
        {
            // チェックループ起動
            CheckEnemyCollisionAsync(this.GetCancellationTokenOnDestroy()).Forget();
        }

        // 衝突チェックループ
        private async UniTaskVoid CheckEnemyCollisionAsync(CancellationToken token)
        {
            // 衝突判定取得用のAsyncHandler
            var handler = this.GetAsyncCollisionEnterTrigger()
                .GetOnCollisionEnterAsyncHandler(token);

            while (!token.IsCancellationRequested)
            {
                // 衝突待機
                var hit = await handler.OnCollisionEnterAsync();

                // 無敵状態ならスルー
                if (IsInvincible) continue;

                // Enemyじゃないならスルー
                if (!hit.gameObject.TryGetComponent<Enemy>(out _)) continue;

                // 無敵状態にしてダメージ処理を実行
                IsInvincible = true;
                OnDamage();

                // 3秒間無敵維持
                await UniTask.Delay(TimeSpan.FromSeconds(3), cancellationToken: token);

                // 無敵解除
                IsInvincible = false;
            }
        }


        /// <summary>
        /// ダメージを受けた処理
        /// </summary>
        private void OnDamage()
        {
            // なんかする
        }
    }
}

ちなみに、このコードはawait foreachが使える環境ならこっちのほうがキレイに書けます。

using System;
using System.Threading;
using Cysharp.Threading.Tasks;
using Cysharp.Threading.Tasks.Triggers;
using UnityEngine;

namespace AntiPatterns.DoWoTsukauna
{
    public sealed class Player : MonoBehaviour
    {
        /// <summary>
        /// 無敵フラグ
        /// </summary>
        public bool IsInvincible { get; private set; }

        private void Start()
        {
            // チェックループ起動
            CheckEnemyCollisionAsync(this.GetCancellationTokenOnDestroy()).Forget();
        }

        // 衝突チェックループ
        private async UniTaskVoid CheckEnemyCollisionAsync(CancellationToken token)
        {
            // 衝突イベントをUniTaskAsyncEnumerableとして取得
            await foreach (var hit in this.GetAsyncCollisionEnterTrigger())
            {
                token.ThrowIfCancellationRequested();

                // 無敵状態ならスルー
                if (IsInvincible) continue;

                // Enemyじゃないならスルー
                if (!hit.gameObject.TryGetComponent<Enemy>(out _)) continue;

                // 無敵状態にしてダメージ処理を実行
                IsInvincible = true;
                OnDamage();

                // 3秒間無敵維持
                await UniTask.Delay(TimeSpan.FromSeconds(3), cancellationToken: token);

                // 無敵解除
                IsInvincible = false;
            }
        }

        /// <summary>
        /// ダメージを受けた処理
        /// </summary>
        private void OnDamage()
        {
            // なんかする
        }
    }
}

「そもそも購読したい対象が最初からObservableなんだが」というパターンもあるでしょう。
そういう場合もObservableからUniTaskに変換して、そこからawaitしてしまえばOKです。

using System;
using System.Threading;
using Cysharp.Threading.Tasks;
using UniRx;
using UnityEngine;

namespace AntiPatterns.DoWoTsukauna
{
    /// <summary>
    /// 敵
    /// 10秒ごとに1秒間無敵になる
    /// </summary>
    public class Enemy : MonoBehaviour
    {
        /// <summary>
        /// 無敵フラグ
        /// </summary>
        public bool IsInvincible { get; private set; }

        [SerializeField] private TimerProvider _timerProvider;


        private void Start()
        {
            var token = this.GetCancellationTokenOnDestroy();
            CheckLoopAsync(token).Forget();
        }

        private async UniTaskVoid CheckLoopAsync(CancellationToken token)
        {
            // 10秒ごとに1秒無敵になる
            // (よくわからん挙動だが、まぁ例ということで…)

            while (!token.IsCancellationRequested)
            {
                // _timerProvider.CurrentTime は ReactiveProperty(Observable)
                // ObservableをUniTaskに変換してawait
                await _timerProvider.CurrentTime
                    // 末尾ゼロ秒のときのみ
                    .Where(x => x % 10 == 0)
                    // Observable -> UniTask
                    .ToUniTask(useFirstValue: true, token);

                // 無敵化
                IsInvincible = true;
                // 1秒待機
                await UniTask.Delay(TimeSpan.FromSeconds(1), cancellationToken: token);
                // 無敵解除
                IsInvincible = false;
            }
        }
    }
}

アンチパターン: イベントフローが整理されていない

「なぜObservableを使っているのか書いた本人が理解できてない」というパターンです。
Observableを使うメリットとして「依存関係を整理してイベントフローを一方通行にできる」というものがあります。

しかし、このようなデータフローに対する理解が浅いままObservableを使うと次のようなコードを書いてしまうことがあります。

  • 普通にメソッドコールすれば十分な場面ですらなぜかObservableを経由している
  • 外のクラス相手にSubject<T>を渡す処理があったりする

たとえば次のコード。
PlayerPlayerManagerが、Observableを通じて連動していますがそこの定義方法が怪しいです。

using UniRx;
using UnityEngine;

namespace AntiPatterns.TakeObserver
{
    public sealed class PlayerManager : MonoBehaviour
    {
        // PlayerのPrefab
        [SerializeField] private Player _playerPrefab;

        // 今存在しているPlayerの実体
        private Player _currentPlayer;

        private readonly Subject<Unit> _resetSubject = new Subject<Unit>();

        private void Start()
        {
            _resetSubject.AddTo(this);
            CreatePlayer();
        }

        private void OnPlayerDead()
        {
            // Playerが死んだ時の処理がここに
            _currentPlayer = null;

            // 新しいPlayerを生成する
            CreatePlayer();
        }

        private void CreatePlayer()
        {
            // ※ダメな書き方
            
            // Playerを生成
            _currentPlayer = Instantiate(_playerPrefab);

            // 死亡イベント通知用
            var deadSubject = new Subject<Unit>();

            // 通知用のSubjectを渡して初期化
            _currentPlayer.Initialize(deadSubject, _resetSubject);

            // 死亡イベントを取得したら処理を走らせる
            deadSubject.Subscribe(_ => OnPlayerDead());
        }

        // ゲームリセット
        public void GameReset()
        {
            _resetSubject.OnNext(Unit.Default);
        }
    }
}
using System;
using UniRx;
using UnityEngine;

namespace AntiPatterns.TakeObserver
{
    public sealed class Player : MonoBehaviour
    {
        // 死んだ時に通知する用のSubject
        private Subject<Unit> _onDeadSubject;

        /// <summary>
        /// 初期化
        /// </summary>
        public void Initialize(Subject<Unit> deadSubject, IObservable<Unit> resetObservable)
        {
            // 相手から渡されたSubjectを保持しておく
            _onDeadSubject = deadSubject;

            // リセットイベントが来たら位置リセット
            resetObservable
                .Subscribe(_ => Reset())
                .AddTo(this);
        }

        private void Update()
        {
            // 落下したら死亡する
            if (transform.position.y < 0)
            {
                OnDead();
            }
        }

        // リセット処理
        private void Reset()
        {
            // 位置をリセット
            transform.SetPositionAndRotation(Vector3.zero, Quaternion.identity);
        }

        // 死亡処理
        private void OnDead()
        {
            // 死亡結果を相手に通知する
            _onDeadSubject.OnNext(Unit.Default);
            _onDeadSubject.OnCompleted();

            // 破棄
            Destroy(gameObject);
        }
    }
}

普通にメソッドコールすればいいものをなぜか無駄にObservable経由になってたり、
相手から通知をうける部分もなぜかManager側でObservableを用意してたりといろいろダメなコードです。

対策:イベントフローを整理する

じゃあどうすればいいのかですが、一番確実なのはクラス図を書いて関係を俯瞰してみることです。

実際、クラス図を書いてみると今のコードはこうなっています。

一瞬見て分かるレベルでぐっちゃぐちゃです。
どうしてこうなっているかというと、「オブジェクトの主従関係がハッキリしていないから」です。
どちらか一方に参照をよせ、「片方は片方を一方的に知っている」「片方は片方のことを一切知らない」という形にしてあげると関係がスッキリします。

なので、これを目指してコードを修正します。

using UniRx;
using UnityEngine;

namespace AntiPatterns.TakeObserver
{
    public sealed class PlayerManager : MonoBehaviour
    {
        // PlayerのPrefab
        [SerializeField] private Player _playerPrefab;

        // 今存在しているPlayerの実体
        private Player _currentPlayer;

        private void Start()
        {
            CreatePlayer();
        }

        private void OnPlayerDead()
        {
            // Playerが死んだ時の処理がここに
            _currentPlayer = null;

            // 新しいPlayerを生成する
            CreatePlayer();
        }

        private void CreatePlayer()
        {
            // Playerを生成
            _currentPlayer = Instantiate(_playerPrefab);

            // 死亡イベントを取得したら処理を走らせる
            _currentPlayer.OnDeadAsync.Subscribe(_ => OnPlayerDead());
        }

        // ゲームリセット
        public void GameReset()
        {
            // 直接メソッド呼び出しすればOK
            _currentPlayer.Reset();
        }
    }
}
using System;
using UniRx;
using UnityEngine;

namespace AntiPatterns.TakeObserver
{
    public sealed class Player : MonoBehaviour
    {
        // 死んだ時に通知する用のSubject
        // Player側にSubject定義もたせたほうがスマート
        private readonly AsyncSubject<Unit> _onDeadSubject = new AsyncSubject<Unit>();

        public IObservable<Unit> OnDeadAsync => _onDeadSubject;
        
        private void Awake()
        {
            _onDeadSubject.AddTo(this);
        }

        private void Update()
        {
            // 落下したら死亡する
            if (transform.position.y < 0)
            {
                OnDead();
            }
        }

        // リセット処理
        public void Reset()
        {
            // 位置をリセット
            transform.SetPositionAndRotation(Vector3.zero, Quaternion.identity);
        }

        // 死亡処理
        private void OnDead()
        {
            // 死亡結果を相手に通知する
            _onDeadSubject.OnNext(Unit.Default);
            _onDeadSubject.OnCompleted();

            // 破棄
            Destroy(gameObject);
        }
    }
}

はい、ということで普通に書いたらこうなるでしょ、というコードに落ち着きました。

UniRxを使い始めた初心者の人は「何が何でもUniRxで書かないとダメ」という観念にとらわれ、シンプルに書けばいいのに無駄にややこしく書く傾向があったりします。
UniRxを導入しいてもそれを無理に使う必要はないので、原点に立ち返って素直なコードを書けばOKです。
(なんでそこまでしてUniRxにこだわるの?という場面が多すぎる)

アンチパターン: メッセージの発行タイミングが管理しきれていない

これは「イベントフローが整理されていない」に通じるものがあります。
「誰が」「いつ」「どこで」、イベントを発行するかが管理できてないものです。
ひどいパターンだと「Observableの途中でDo()を使って他のSubjectにイベントを流し込んでいたり」とかありえます。

こればかりはプロジェクト規模や作ろうとしているものの規模感によるので、対策がムズカシイ部分もあります。
ひとまず、次のことを意識して実装を進めると比較的マシな状況にはできます。

  • Subject<T>はクラス内にprivateで隠蔽し、外に公開するものはIObservable<T>のみにする
  • Subject.OnNext()を呼び出す場所はできるだけ1つのメソッドに集約する
  • イベントの発行に順序関係をもたせる
  • Observableから他のObservableにイベントが飛び火するような入れ子関係をできるだけ避ける
  • ファサードパターンを使うなど、そもそも外部モジュールからアクセスするときの口を集約する

まとめ

  • 何がアンチパターンなのか、どうしてアンチパターンといわれるかを理解しよう
  • アンチパターンを避けるためにはどうするのか、他の選択肢を選べるようになろう
  • UniRxは万能でもなんでもないので、ややこしいと思ったら一切使わないのも手

今回、巷のいろいろなUniRxコードを見て思った感想を書いた記事になってます。
「オイオイ、どういう発想したらそういうコードになるんじゃ?????」というコードを今まで数多く見てきました。
そういったクソUniRxコードがなぜクソなのか、の要素を整理した結果が今回の内容になっています。

UniRxは使う機会が減ったとはいえど、まだ現役で使うことはあります。
もしUniRxを使うことがあるならばこのあたりを意識してみるとよいでしょう。

123
84
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
123
84

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?