はじめに

この記事はukyo_rst氏 a.k.a. マグロナちゃんによる以下の記事を元に作成しています。

Unity完全に理解した（バーチャルボルテコン制作記録）｜ukyo_rst｜pixivFANBOX

あとなんか上の素人コードを美しくできる人いたら色々おしえてね。C#なんもわからん。HTMLみたいな感じで書いてる。

面識ないけどお力添えできればいいなぁと思った次第。「美しいコード」には正解はないので、色んな人の意見が上がると凄くいいと思います。

レビューの前に

これだけは言っておきたいんですけど、コードの見た目が悪かろうが何だろうが、「動くものを作った」ことは本当に凄いことです。

分からないなりにできる最善手を尽くしてコードを書いたことは分かります。 BTの性質（今回でいうと各ボタンのON/OFF）を変えるのではなくBT自体を差し替えてしまう等、完成のために柔軟な発想で作業を進めたこと自体が本当に素晴らしいです。プログラムなんて趣味で書く程度なら本当はこんな感じでいいんですよ。ただ、本人が美しくしたいという意思を表明していたので、僭越ながら今回は筆をとった次第です。

もう一点、ここまで言いながらC#での開発経験はありません。なんならC#の開発環境もUnityも入れていません。 JavaやKotlin等、オブジェクト指向言語に関しての開発経験はあるので、エッセンス的な部分は問題ないと思いますが、言語特有の記述やコードフォーマットなどに疎い可能性があります。

動作チェックは今後行いたいですが、もし先にコンパイルエラーや、実行時の動作がおかしくなる等あれば、是非ご教授いただけると幸いです（勿論、他にも気になる点あれば是非ご教授ください）。

レビュー

元のコード

それでは見ていきましょう。まずは元のコードを引用します。

using System;
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using UnityRawInput;

public class ButtonManagerRAW : MonoBehaviour
{

    public GameObject bta, btaoff, btb, btboff, btc, btcoff, btd, btdoff, fxa, fxaoff, fxb, fxboff;
    public bool InterceptMessages;

    private void OnEnable()
    {
        RawKeyInput.Start(true);
        RawKeyInput.OnKeyUp += OnKeyUp;
        RawKeyInput.OnKeyDown += OnKeyDown;
    }

    private void OnDisable()
    {
        RawKeyInput.Stop();
        RawKeyInput.OnKeyUp -= OnKeyUp;
        RawKeyInput.OnKeyDown -= OnKeyDown;
    }

    // Start is called before the first frame update
    void Start()
    {
        bta.SetActive(false);
        btaoff.SetActive(true);
        btb.SetActive(false);
        btboff.SetActive(true);
        btc.SetActive(false);
        btcoff.SetActive(true);
        btd.SetActive(false);
        btdoff.SetActive(true);
        fxa.SetActive(false);
        fxaoff.SetActive(true);
        fxb.SetActive(false);
        fxboff.SetActive(true);
    }

    private void OnKeyUp(RawKey key)
    {
        Debug.Log("Key Up: " + key);

        if (RawKeyInput.IsKeyDown(RawKey.F1))
        {
            bta.SetActive(true);
            btaoff.SetActive(false);
        } else
        {
            bta.SetActive(false);
            btaoff.SetActive(true);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F2))
        {
            btb.SetActive(true);
            btboff.SetActive(false);
        }
        else
        {
            btb.SetActive(false);
            btboff.SetActive(true);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F3))
        {
            btc.SetActive(true);
            btcoff.SetActive(false);
        }
        else
        {
            btc.SetActive(false);
            btcoff.SetActive(true);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F4))
        {
            btd.SetActive(true);
            btdoff.SetActive(false);
        }
        else
        {
            btd.SetActive(false);
            btdoff.SetActive(true);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F5))
        {
            fxa.SetActive(true);
            fxaoff.SetActive(false);
        }
        else
        {
            fxa.SetActive(false);
            fxaoff.SetActive(true);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F6))
        {
            fxb.SetActive(true);
            fxboff.SetActive(false);
        }
        else
        {
            fxb.SetActive(false);
            fxboff.SetActive(true);
        }
    }

    private void OnKeyDown(RawKey key)
    {
        Debug.Log("Key Down: " + key);

        if (RawKeyInput.IsKeyDown(RawKey.F1))
        {
            bta.SetActive(true);
            btaoff.SetActive(false);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F2))
        {
            btb.SetActive(true);
            btboff.SetActive(false);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F3))
        {
            btc.SetActive(true);
            btcoff.SetActive(false);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F4))
        {
            btd.SetActive(true);
            btdoff.SetActive(false);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F5))
        {
            fxa.SetActive(true);
            fxaoff.SetActive(false);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F6))
        {
            fxb.SetActive(true);
            fxboff.SetActive(false);
        }
    }

    // Update is called once per frame
    void Update()
    {

    }
}

似たような処理をくくりだす: 関数化

このコードを見た一番の印象は、「同じようなことを繰り返し書いてしまっているな」というものです。なので、まずはこの繰り返しを関数にしてしまいましょう。最終的にはオブジェクト指向的なクラスを作っていきますが、まずは順序立てて考えていきます。

今回のプログラムでは、このような処理が散見されます。

// ボタンに対応するキーが押された時
bta.SetActive(true);
btaoff.SetActive(false);

このような処理がボタンを変えてあちこちに散りばめられていますね。

この処理に必要な要素は2つです。すなわち、

ボタンが押された時の見た目を示すGameObject
ボタンが離された時の見た目を示すGameObject

の2つになります。

具体例で考えてみましょう。 BT-Aが押された、という状況を考えます。この時、このプログラムで処理すべき内容は、

BT-Aが押された時の見た目を示すGameObject（変数で言えばbta）を有効化（true）する
BT-Aが離された時の見た目を示すGameObject（変数で言えばbtaoff）を無効化（false）する

ここで一段階抽象度を上げます。次の2つのものを考えます。

とあるボタンBTが押された時の見た目を示すGameObject型の変数bton
とあるボタンBTが離された時の見た目を示すGameObject型の変数btoff

この時、具体例で書いた処理は以下のように書けます。

bton.setActive(true);
btoff.setActive(false);

ここまでくれば準備は万全です。ここで関数を作成します。関数に関する説明は色々とあるのですが、今回はその一側面である「処理をまとめる」という部分を利用していきたいと思います。

処理をまとめるだけなので、戻り値はvoidにします。戻り値に関してはここでは解説しません。

このような処理になります。

// ボタンが押された時の処理なので、わかりやすくpushと名付けましょう
public void push(GameObject bton, GameObject btoff)
{
    bton.setActive(true);
    btoff.setActive(false);
}

これをBT-Aについて呼び出すときは、

push(bta, btaoff)

ですね。

また、ボタンを離した時の処理も同じように考えて作成しましょう。以下のようになります。

// ボタンが離された時の処理なので、わかりやすくreleaseと名付けましょう
public void release(GameObject bton, GameObject btoff)
{
    bton.setActive(false);
    btoff.setActive(true);
}

// 使い方: BT-Aが離された時
release(bta, btaoff)

この処理を使って元のコードを改変していきます。わかる人は、そこにそのメソッド（ほぼイコールで関数）はおかしい！と思っていただけると思いますが、現状は修正途中なので大目にみてください。

using System;
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using UnityRawInput;

public class ButtonManagerRAW : MonoBehaviour
{
    public GameObject bta, btaoff, btb, btboff, btc, btcoff, btd, btdoff, fxa, fxaoff, fxb, fxboff;
    public bool InterceptMessages;

    private void OnEnable()
    {
        RawKeyInput.Start(true);
        RawKeyInput.OnKeyUp += OnKeyUp;
        RawKeyInput.OnKeyDown += OnKeyDown;
    }

    private void OnDisable()
    {
        RawKeyInput.Stop();
        RawKeyInput.OnKeyUp -= OnKeyUp;
        RawKeyInput.OnKeyDown -= OnKeyDown;
    }

    // Start is called before the first frame update
    void Start()
    {
        release(bta, btaoff);
        release(btb, btboff);
        release(btc, btcoff);
        release(btd, btdoff);
        release(fxa, fxaoff);
        release(fxb, fxboff);
    }

    private void OnKeyUp(RawKey key)
    {
        Debug.Log("Key Up: " + key);

        if (RawKeyInput.IsKeyDown(RawKey.F1))
        {
            push(bta, btaoff);
        }
        else
        {
            release(bta, btaoff);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F2))
        {
            push(btb, btboff);
        }
        else
        {
            release(btb, btboff);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F3))
        {
            push(btc, btcoff);
        }
        else
        {
            release(btc, btcoff);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F4))
        {
            push(btd, btdoff);
        }
        else
        {
            release(btd, btdoff);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F5))
        {
            push(fxa, fxaoff);
        }
        else
        {
            release(fxa, fxaoff);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F6))
        {
            push(fxb, fxboff);
        }
        else
        {
            release(fxb, fxboff);
        }
    }

    private void OnKeyDown(RawKey key)
    {
        Debug.Log("Key Down: " + key);

        if (RawKeyInput.IsKeyDown(RawKey.F1))
        {
            push(bta, btaoff);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F2))
        {
            push(btb, btboff);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F3))
        {
            push(btc, btcoff);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F4))
        {
            push(btd, btdoff);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F5))
        {
            push(fxa, fxaoff);
        }

        if (RawKeyInput.IsKeyDown(RawKey.F6))
        {
            push(fxb, fxboff);
        }
    }

    // Update is called once per frame
    void Update()
    {

    }

    public void push(GameObject bton, GameObject btoff)
    {
        bton.setActive(true);
        btoff.setActive(false);
    }

    public void release(GameObject bton, GameObject btoff)
    {
        bton.setActive(false);
        btoff.setActive(true);
    }
}

少しやりたいことが見えてきた気がしますね。

似たような処理をくくりだす: クラス化

関数化を通して次のようなことが見えてきました。

btaは常に同じボタンBT-Aに対するGameObjectであるbtaoffと一緒に使われている
逆にbtaは異なるボタンに対するGameObjectであるbtboffやfxaoffと一緒に使われることはない
他のボタンでも同様のことが言える

これはこのように言い換えることもできます。

ボタンはonのGameObjectとoffのGameObjectからなる
各ボタンは別のボタンから独立してる（他のボタンの振る舞いに依存した振る舞いをしない）

ここで使われるのがオブジェクト指向らしいクラスの考え方です。

クラスとは？

世の中には様々なクラスに関しての、ひいてはオブジェクト指向の説明記事がありますが、どうしても抽象的な話が多くなりがちです。これはそもそもクラスという概念自体が抽象化を通して生まれてくるものなので仕方ありません。

この説明記事でクラスについて長々と説明してしまうと、それだけで一記事になってしまうので簡単な説明になりますがお許しください。

クラスとは、データの集まりとその振る舞いをまとめたものです。上で例示したものを流用すると、「ボタンとはonのGameObjectとoffのGameObjectからなる」というのがクラスの「データの集まり」としての側面になります。このデータの集まりに対して、ボタンを押した（push）、離した（release）という動作を定義できるわけです。これが「振る舞い」としての側面になります。

さて、ここで今考えるべきボタンクラスを考えます。とりあえず世の中には色々なボタンがありますが、今回はボルテコンのボタンなので、名前はSdvxButtonとでもしましょう。

まずはクラスの雛形です。

class SdvxButton
{
}

このクラスにはまずデータとしてonのGameObjectとoffのGameObjectが必要でしたね。これを記述します。

class SdvxButton
{
    private GameObject bton, btoff;
}

いい感じですね。 privateに関してはアクセス修飾子と呼びますが、今は気にしなくても大丈夫です。気になるのであれば、別途調べてください。

ではここにpushとreleaseの振る舞いを追加しましょう。先ほど作成した関数を流用します。

class SdvxButton
{
    private GameObject bton, btoff;

    public void push()
    {
        bton.setActive(true);
        btoff.setActive(false);
    }

    public void release()
    {
        bton.setActive(false);
        btoff.setActive(true);
    }
}

先ほどまでは関数に引数が必要でしたが、今回は必要なくなりました。なぜなら、このクラス内の関数（これをメソッドと呼称します。今後は関数ではなくメソッドと呼びます）ではクラスが持つ変数（これはフィールドと呼びます）には自由にアクセスできるからです。これにより、いちいちメソッドに引数を渡す必要がなくなりました。楽チン！

早速このボタンの使い方をみていきましょう。まずはクラスを元にBT-Aに対応するインスタンスを作成します。インスタンスとは、クラスによって定義された振る舞いに、実際のデータを入れて動かせるようにしたものです。 BT-Aなら、btonに変数bta、btoffに変数btaoffを代入すればBT-Aになりそうですね。それでは…どうやって代入しましょう？

というわけで、ここでインスタンスを作成するためにコンストラクタを作成します。コンストラクタはクラス名と同名で、メソッドのように作成するので、

class SdvxButton
{
    private GameObject bton, btoff;

    SdvxButton(GameObject bton, GameObject btoff)
    {
        this.bton = bton;
        this.btoff = btoff;
    }

    public void push()
    {
        bton.setActive(true);
        btoff.setActive(false);
    }

    public void release()
    {
        bton.setActive(false);
        btoff.setActive(true);
    }
}

このようになりますね。 thisというものが出てきていますが、あまり意識しなくてもいいです。 this.btonはクラスのフィールドであるprivate GameObject btonの方のbtonで、何もついていないbtonはコンストラクタの引数であるGameObject btonのbtonだと認識してもらえれば十分です。

これによってBT-Aに対応するインスタンスが作れるようになりました。実際に作って動かす例が以下です。

// BT-Aに対応するSdvxButtonクラスのインスタンスbt_aの作成
SdvxButton bt_a = new SdvxButton(bta, btaoff);

// BT-Aを押す
bt_a.push()

// BT-Aを離す
bt_a.release()

これを実際に元のコードに統合してみましょう。

using System;
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using UnityRawInput;

class SdvxButton
{
    private GameObject bton, btoff;

    SdvxButton(GameObject bton, GameObject btoff)
    {
        this.bton = bton;
        this.btoff = btoff;
    }

    public void push()
    {
        bton.setActive(true);
        btoff.setActive(false);
    }

    public void release()
    {
        bton.setActive(false);
        btoff.setActive(true);
    }
}

public class ButtonManagerRAW : MonoBehaviour
{
    public GameObject bta, btaoff, btb, btboff, btc, btcoff, btd, btdoff, fxa, fxaoff, fxb, fxboff;
    public bool InterceptMessages;

    public SdvxButton bt_a = SdvxButton(bta, btaoff);
    public SdvxButton bt_b = SdvxButton(btb, btboff);
    public SdvxButton bt_c = SdvxButton(btc, btcoff);
    public SdvxButton bt_d = SdvxButton(btd, btdoff);
    public SdvxButton fx_a = SdvxButton(fxa, fxaoff);
    public SdvxButton fx_b = SdvxButton(fxb, fxboff);

    private void OnEnable()
    {
        RawKeyInput.Start(true);
        RawKeyInput.OnKeyUp += OnKeyUp;
        RawKeyInput.OnKeyDown += OnKeyDown;
    }

    private void OnDisable()
    {
        RawKeyInput.Stop();
        RawKeyInput.OnKeyUp -= OnKeyUp;
        RawKeyInput.OnKeyDown -= OnKeyDown;
    }

    // Start is called before the first frame update
    void Start()
    {
        bt_a.release();
        bt_b.release();
        bt_c.release();
        bt_d.release();
        fx_a.release();
        fx_b.release();
    }

    private void OnKeyUp(RawKey key)
    {
        Debug.Log("Key Up: " + key);

        if (RawKeyInput.IsKeyDown(RawKey.F1))
        {
            bt_a.push();
        }
        else
        {
            bt_a.release();
        }

        if (RawKeyInput.IsKeyDown(RawKey.F2))
        {
            bt_b.push();
        }
        else
        {
            bt_b.release();
        }

        if (RawKeyInput.IsKeyDown(RawKey.F3))
        {
            bt_c.push();
        }
        else
        {
            bt_c.release();
        }

        if (RawKeyInput.IsKeyDown(RawKey.F4))
        {
            bt_d.push();
        }
        else
        {
            bt_d.release();
        }

        if (RawKeyInput.IsKeyDown(RawKey.F5))
        {
            fx_a.push();
        }
        else
        {
            fx_a.release();
        }

        if (RawKeyInput.IsKeyDown(RawKey.F6))
        {
            fx_b.push();
        }
        else
        {
            fx_b.release();
        }
    }

    private void OnKeyDown(RawKey key)
    {
        Debug.Log("Key Down: " + key);

        if (RawKeyInput.IsKeyDown(RawKey.F1))
        {
            bt_a.push();
        }

        if (RawKeyInput.IsKeyDown(RawKey.F2))
        {
            bt_b.push();
        }

        if (RawKeyInput.IsKeyDown(RawKey.F3))
        {
            bt_c.push();
        }

        if (RawKeyInput.IsKeyDown(RawKey.F4))
        {
            bt_d.push();
        }

        if (RawKeyInput.IsKeyDown(RawKey.F5))
        {
            fx_a.push();
        }

        if (RawKeyInput.IsKeyDown(RawKey.F6))
        {
            fx_b.push();
        }
    }

    // Update is called once per frame
    void Update()
    {

    }

    // ここに追加したpushメソッドとreleaseメソッドはSdvxButtonクラスに移動しました
}

ButtonManagerRAW内でやりたいことがだいぶスッキリわかるようになってきましたね。

switch文を活用する

このまでコードを整理してきた中で、気になるのはやはりif文の多さです。押したキーを取得したいだけなのになぜこんなに長く…というところだと思います。

これに関しては、おそらくUsageの読み違いが原因でしょう。 Elringus/UnityRawInputを見てみましょう。

Add listeners for the input events.

RawKeyInput.OnKeyUp += HandleKeyUp; RawKeyInput.OnKeyDown += HandleKeyDown; private void HandleKeyUp (RawKey key) { … } private void HandleKeyDown (RawKey key) { … }

> You can also check whether specific key is currently pressed.
> ```
if (RawKeyInput.IsKeyDown(key)) { ... }

ここから、イベントリスナーによるキーの監視はRawKeyInput.IsKeyDown(key)による入力チェックと併用しなくても良いことがわかります。今回の場合、イベントリスナーを使ってキー入力を監視してるわけですから、RawKeyInput.IsKeyDown(key)は不要ですね。押された/離されたキーはRawKey型の変数keyに代入され関数の中で使えます。まずこれを前提としてOnKeyDownを書き換えると、

private void OnKeyDown(RawKey key)
{
    Debug.Log("Key Down: " + key);

    if (key == RawKey.F1)
    {
        bt_a.push();
    }

    if (key == RawKey.F2)
    {
        bt_b.push();
    }

    if (key == RawKey.F3)
    {
        bt_c.push();
    }

    if (key == RawKey.F4)
    {
        bt_d.push();
    }

    if (key == RawKey.F5)
    {
        fx_a.push();
    }

    if (key == RawKey.F6)
    {
        fx_b.push();
    }
}

このようになりますね。

しかし、このようなif文を書く必要はありません。このような時のために、C#にはswitch文という形式が用意されています。このif文と等価な処理は以下のように書けます。

private void OnKeyDown(RawKey key)
{
    Debug.Log("Key Down: " + key);

    switch (key)
    {
        case RawKey.F1:
            bt_a.push();
            break;
        case RawKey.F2:
            bt_b.push();
            break;
        case RawKey.F3:
            bt_c.push();
            break;
        case RawKey.F4:
            bt_d.push();
            break;
        case RawKey.F5:
            fx_a.push();
            break;
        case RawKey.F6:
            fx_b.push();
            break;
        default:
            break;
    }
}

OnKeyUpの方も見ていきましょう。 OnKeyUpではおそらく離されている探すために、一度if (RawKeyInput.IsKeyDown(RawKey.F1))として押されていることを確認し、else節内で本当にやりたかったrelease処理をしているのだと思います。しかし、OnKeyUpでは、keyとして離されたキーが渡されてくるため、この処理はだいぶ削ることができます。と言うことでOnKeyDownと同じように処理したものがこちらです。

private void OnKeyUp(RawKey key)
{
    Debug.Log("Key Up: " + key);

    switch (key)
    {
        case RawKey.F1:
            bt_a.release();
            break;
        case RawKey.F2:
            bt_b.release();
            break;
        case RawKey.F3:
            bt_c.release();
            break;
        case RawKey.F4:
            bt_d.release();
            break;
        case RawKey.F5:
            fx_a.release();
            break;
        case RawKey.F6:
            fx_b.release();
            break;
        default:
            break;
    }
}

元のコードに統合しましょう。

using System;
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using UnityRawInput;

class SdvxButton
{
    private GameObject bton, btoff;

    SdvxButton(GameObject bton, GameObject btoff)
    {
        this.bton = bton;
        this.btoff = btoff;
    }

    public void push()
    {
        bton.setActive(true);
        btoff.setActive(false);
    }

    public void release()
    {
        bton.setActive(false);
        btoff.setActive(true);
    }
}

public class ButtonManagerRAW : MonoBehaviour
{
    public GameObject bta, btaoff, btb, btboff, btc, btcoff, btd, btdoff, fxa, fxaoff, fxb, fxboff;
    public bool InterceptMessages;

    public SdvxButton bt_a = SdvxButton(bta, btaoff);
    public SdvxButton bt_b = SdvxButton(btb, btboff);
    public SdvxButton bt_c = SdvxButton(btc, btcoff);
    public SdvxButton bt_d = SdvxButton(btd, btdoff);
    public SdvxButton fx_a = SdvxButton(fxa, fxaoff);
    public SdvxButton fx_b = SdvxButton(fxb, fxboff);

    private void OnEnable()
    {
        RawKeyInput.Start(true);
        RawKeyInput.OnKeyUp += OnKeyUp;
        RawKeyInput.OnKeyDown += OnKeyDown;
    }

    private void OnDisable()
    {
        RawKeyInput.Stop();
        RawKeyInput.OnKeyUp -= OnKeyUp;
        RawKeyInput.OnKeyDown -= OnKeyDown;
    }

    // Start is called before the first frame update
    void Start()
    {
        bt_a.release();
        bt_b.release();
        bt_c.release();
        bt_d.release();
        fx_a.release();
        fx_b.release();
    }

    private void OnKeyUp(RawKey key)
    {
        Debug.Log("Key Up: " + key);

        switch (key)
        {
            case RawKey.F1:
                bt_a.release();
                break;
            case RawKey.F2:
                bt_b.release();
                break;
            case RawKey.F3:
                bt_c.release();
                break;
            case RawKey.F4:
                bt_d.release();
                break;
            case RawKey.F5:
                fx_a.release();
                break;
            case RawKey.F6:
                fx_b.release();
                break;
            default:
                break;
        }
    }

    private void OnKeyDown(RawKey key)
    {
        Debug.Log("Key Down: " + key);

        switch (key)
        {
            case RawKey.F1:
                bt_a.push();
                break;
            case RawKey.F2:
                bt_b.push();
                break;
            case RawKey.F3:
                bt_c.push();
                break;
            case RawKey.F4:
                bt_d.push();
                break;
            case RawKey.F5:
                fx_a.push();
                break;
            case RawKey.F6:
                fx_b.push();
                break;
            default:
                break;
        }
    }

    // Update is called once per frame
    void Update()
    {

    }
}

だいぶ見通しが良くなってきましたね。

キーとボタンを対応させる

ここまででも十分コードの見通しは良くなってきました。ここで終えてしまってもいいのですが、もう一息いきましょう。

現状、キーとボタンの対応を記述している部分がばらけています。このままでは、例えばBT-Aと対応するキーをF1からF7にしようとすると、OnKeyUpとOnKeyDownの2箇所を正確に直さないとバグが起きてしまいます。そのくらい忘れないでしょと思いがちですが、それは今コードを書いたばかりだからです。明日の自分はこれを忘れます。

なので、キーとボタンの対応を一元化し、一元化された対応を元にOnKeyUpとOnKeyDownを動かすように改良していきましょう。今回必要なのは、RawKey型の引数から、対応するSdvxButton型のインスタンスを呼び出す関数です。関数のシグネチャを先に作ってしまいましょう。以下のようになります。

private SdvxButton getSdvxButton(RawKey key)
{
}

サクッと書いてしまいましょう。

private SdvxButton getSdvxButton(RawKey key)
{
    switch (key)
    {
        case RawKey.F1:
            return bt_a;
        case RawKey.F2:
            return bt_b;
        case RawKey.F3:
            return bt_c;
        case RawKey.F4:
            return bt_d;
        case RawKey.F5:
            return fx_a;
        case RawKey.F6:
            return fx_b;
        default:
            /* ??? */
    }
}

はい、ここで問題がひとつ見えてきました。対応するキー以外が押されてしまったらどうしましょうか。

これに関しては場合によるかと思います。簡単に考えられる案としては、押し離ししても画面に影響を与えないダミーオブジェクトを作成する、nullを返す、例外を吐く等々ありますが、どうしましょう。

この場合の個人的なベストプラクティスは、SdvxButtonをインターフェースとしてしまい、ダミーオブジェクトのシングルトンを作ってそれを返すことですね。 nullは扱いたくないですし、これは例外とはとても言えない状況ですからね。

面倒ですがこれに沿ってやっていきましょう。まずはSdvxButtonのインターフェースを作成します。

interface SdvxButton
{
    void push();
    void release();
}

インターフェースとは、先ほどのクラスでいうところの「振る舞い」のみを抽出したものです。この「振る舞い」ができるクラスは全て同一のインターフェースとして扱えるという優れものです。 SdvxButtonにはpushとreleaseの振る舞いがありましたから、これをインターフェースとして定義するわけです。

インターフェースと同名のクラスは作成できないので、旧SdvxButtonクラスをRealSdvxButtonクラスに変更しましょう。また、SdvxButtonインターフェースを実装していることを表明します。いい名前が思いつかなかったので代案あれば教えてください。

class RealSdvxButton : SdvxButton
{
    private GameObject bton, btoff;

    SdvxButton(GameObject bton, GameObject btoff)
    {
        this.bton = bton;
        this.btoff = btoff;
    }

    public void push()
    {
        bton.setActive(true);
        btoff.setActive(false);
    }

    public void release()
    {
        bton.setActive(false);
        btoff.setActive(true);
    }
}

合わせて、ButtonManagerRAWクラス内で作成したインスタンスの部分も修正しましょう。

public SdvxButton bt_a = RealSdvxButton(bta, btaoff);
public SdvxButton bt_b = RealSdvxButton(btb, btboff);
public SdvxButton bt_c = RealSdvxButton(btc, btcoff);
public SdvxButton bt_d = RealSdvxButton(btd, btdoff);
public SdvxButton fx_a = RealSdvxButton(fxa, fxaoff);
public SdvxButton fx_b = RealSdvxButton(fxb, fxboff);

RealSdvxButtonクラスのインスタンスはSdvxButtonインターフェースのインスタンスとして扱えれば十分なので、左側は修正する必要はありません。 RealSdvxButtonクラスのインスタンスとして扱いたい！という時は修正しましょう。

さて、ダミーのSdvxButtonインターフェースの実装も作りましょう。名前はわかりやすくDummySdvxButtonで、

class DummySdvxButton : SdvxButton
{
    public void push()
    {
    }

    public void release()
    {
    }
}

ダミーなので、pushもreleaseもやることはありません。中身は空で大丈夫です。本来はシングルトンにしたいのですが、シングルトンにしないと困ることもないので、今回は省略します。

さあ、ここまできたらゴールは近いです。デフォルトの場合DummySdvxButtonを返せばいいだけなので、

private SdvxButton getSdvxButton(RawKey key)
{
    switch (key)
    {
        case RawKey.F1:
            return bt_a;
        case RawKey.F2:
            return bt_b;
        case RawKey.F3:
            return bt_c;
        case RawKey.F4:
            return bt_d;
        case RawKey.F5:
            return fx_a;
        case RawKey.F6:
            return fx_b;
        default:
            return new DummySdvxButton();
    }
}

これで大丈夫です。

このgetSdvxButtonメソッドは、SdvxButtonインターフェースを実装したクラスであれば、RealSdvxButtonクラスのインスタンスでも、DummySdvxButtonクラスのインスタンスでも、あるいは新たに作成した他のクラスのインスタンスでも返せます。なので、このように複数クラスのインスタンスが混在してても大丈夫なのです。

このgetSdvxButtonメソッドを使用したOnKeyUpメソッドおよびOnKeyDownメソッドは以下のようになります。

private void OnKeyUp(RawKey key)
{
    Debug.Log("Key Up: " + key);

    getSdvxButton(key).release();
}

private void OnKeyDown(RawKey key)
{
    Debug.Log("Key Down: " + key);

    getSdvxButton(key).push();
}

何をしているのか一目瞭然ですね。

さあ、全てを元のコードに統合しましょう。

最終結果

using System;
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using UnityRawInput;

interface SdvxButton
{
    void push();
    void release();
}

class RealSdvxButton : SdvxButton
{
    private GameObject bton, btoff;

    SdvxButton(GameObject bton, GameObject btoff)
    {
        this.bton = bton;
        this.btoff = btoff;
    }

    public void push()
    {
        bton.setActive(true);
        btoff.setActive(false);
    }

    public void release()
    {
        bton.setActive(false);
        btoff.setActive(true);
    }
}

class DummySdvxButton : SdvxButton
{
    public void push()
    {
    }

    public void release()
    {
    }
}

public class ButtonManagerRAW : MonoBehaviour
{

    public GameObject bta, btaoff, btb, btboff, btc, btcoff, btd, btdoff, fxa, fxaoff, fxb, fxboff;
    public bool InterceptMessages;

    public SdvxButton bt_a = SdvxButton(bta, btaoff);
    public SdvxButton bt_b = SdvxButton(btb, btboff);
    public SdvxButton bt_c = SdvxButton(btc, btcoff);
    public SdvxButton bt_d = SdvxButton(btd, btdoff);
    public SdvxButton fx_a = SdvxButton(fxa, fxaoff);
    public SdvxButton fx_b = SdvxButton(fxb, fxboff);

    private void OnEnable()
    {
        RawKeyInput.Start(true);
        RawKeyInput.OnKeyUp += OnKeyUp;
        RawKeyInput.OnKeyDown += OnKeyDown;
    }

    private void OnDisable()
    {
        RawKeyInput.Stop();
        RawKeyInput.OnKeyUp -= OnKeyUp;
        RawKeyInput.OnKeyDown -= OnKeyDown;
    }

    // Start is called before the first frame update
    void Start()
    {
        bt_a.release();
        bt_b.release();
        bt_c.release();
        bt_d.release();
        fx_a.release();
        fx_b.release();
    }

    private void OnKeyUp(RawKey key)
    {
        Debug.Log("Key Up: " + key);

        getSdvxButton(key).release();
    }

    private void OnKeyDown(RawKey key)
    {
        Debug.Log("Key Down: " + key);

        getSdvxButton(key).push();
    }

    private SdvxButton getSdvxButton(RawKey key)
    {
        switch (key)
        {
            case RawKey.F1:
                return bt_a;
            case RawKey.F2:
                return bt_b;
            case RawKey.F3:
                return bt_c;
            case RawKey.F4:
                return bt_d;
            case RawKey.F5:
                return fx_a;
            case RawKey.F6:
                return fx_b;
            default:
                return new DummySdvxButton();
        }
    }

    // Update is called once per frame
    void Update()
    {

    }
}

これで終わりです！だいぶいいコードになったんじゃないでしょうか？

Q&A

ここまでやって何がいいの？処理があちこちに行っただけじゃない？

一番大きいのは、今後このコードを編集するときに、どこを変更すればいいのかが一目瞭然になることです。

例えば、現状はOnのボタンとOffのボタンを入れ替えることでボタンの表示を変えていましたが、ひとつのボタンでOn状態のテクスチャとOff状態のテクスチャを切り替えよう！となったとします。このとき、昔のコードではどこをどう直すべきかわかりますか？そしてその作業を迅速かつ安全に終わらせられますか？こう考えると意外と難しい・手間がかかることに気づいていだだけるかなと思います。

一方、変更したコードでは、必要な処置はRealSdvxButtonクラスの中身のみを変更することです。あるいは、SdvxButtonインターフェースを実装した新しいクラスを作ることでも問題ないです。いずれにせよ、絡みついた処理が分離したことで、機能を変更・拡充することが容易になりました。

少し前に話題になった@t_wadaさんの「質とスピード」の発表に目を通していだたけるとまた知見があるかもしれません。

pushとreleaseを分けたのはどうして？引数にbool変数を一個含めればまとめられるよね？

現状ではpushとreleaseは全く逆の処理なのでそれでもいいんですが、今後仕様が変わりpushのみで行う処理、releaseのみで行う処理が追加された場合、その一つの関数では押された場合には〜、離された場合には〜、と2つの責務を持つことになります。シグネチャの変更は何度もしたくないので、先に分けてしまうことにしました。

あるいはpushとreleaseをインターフェースとしてprivateな実装をしてしまうならばいいかも。こんな感じ。

class RealSdvxButton : SdvxButton
{
    private GameObject bton, btoff;

    RealSdvxButton(GameObject bton, GameObject btoff) {
        this.bton = bton;
        this.btoff = btoff;
    }

    public void push() {
        activate(true);
    }

    public void release() {
        activate(false);
    }

    private void activate(bool pushed) {
        bton.setActive(pushed);
        btoff.setActive(!pushed);
    }
}

switch文じゃなくて辞書配列使えよ

~~めっちゃわかる。~~

これに関しては悩んだんですけど、直感的にわかりやすいのは辞書をこねくり回すよりもswitchかな〜と思い、今回はswitch文を採用しました。

キーとの対応をボタン自体に持たせちゃダメなの？

ちょっと考えたんですが、コードが無駄に複雑になりそうのでやめておきました。いいコード例あれば教えてください。

アクセス修飾子ガバガバすぎない？

本当だよ。やる気あんの？

FX-A、FX-BじゃなくてFX-L、FX-Rですよw

お前それマグロナちゃんの前でも同じこと言えんの？

マグロナちゃん制作ボルテコンのコードレビュー