Unityのアンチパターンの記:Script編:第一部 - Kakoのいろいろやったこと記

対象
なぜ書こうと思ったか
内容
- 前提
PlayerAntiScript
直してみたPlayer
説明
- 動作に関わるもの
- 個人的な好み
終わりに

対象

ゲームをなんとか自力で作れそうな段階。

なぜ書こうと思ったか

今まで本やチュートリアルをなぞってこなかったので、考えや知識の間違いを確認したいことと、初学者の身としてはアンチパターンを知っていきたい、という気持ちがあるので、どっかのなんかの誰かのためになるかな、と思ったことによる。

内容

見たことのあるちょっと良くなさそうなもので初期に改善できそうなものを突っ込んで、実際に改造してみる。
自作自演まさかり。

前提

動作に関わるもの
書きやすさ、読みやすさを主とした好みに関わるもの

を書きます。命名規則「自体」はスルーします。人によるので!

また、間違いがあったらすぐに直しますので、ご指摘いただけると幸いです。

PlayerAntiScript

数字が、良くない番号です。

using UnityEngine;

public class PlayerAntiScript : MonoBehaviour
{
    //1
    private const int Tsuyosa = 5;

    //2
    public Rigidbody Rigidbody => GetComponent<Rigidbody>();

    //3
    private int _hp = default;


    //4
    private GameObject _enemy;

    //5
    private void Start()
    {
    }


    private void Update()
    {
        //4,6
        _enemy = FindObjectOfType<EnemyScript>().gameObject;

        //7
        if (Input.GetMouseButton(0))
        {
            //2,8,9
            Rigidbody.AddForce(transform.forward * 30);
        }
    }

    private void OnCollisionEnter(Collision other)
    {
        //10
        if (other.gameObject.TryGetComponent<EnemyScript>(out var result))
        {
            //11
            result.GetComponent<EnemyScript>().Hp--;
            if (result.GetComponent<EnemyScript>().Hp < 1)
            {
                Destroy(result);
            }
        }
    }

    //1,3,4,12
    private int MaxEnemyNum = 30;
}

直してみたPlayer

using UnityEngine;

public class Player : MonoBehaviour
{
    //使ってないけど修正参考で上に出しておいた感じです
    static readonly int MaxEnemyNum = 30;

    private const int PlayerPower = 1;
    static readonly float Acceleration = 30f;

    private Rigidbody _rigidbody;

    private void Start()
    {
        _rigidbody = GetComponent<Rigidbody>();
    }

    private void FixedUpdate()
    {
        if (!Input.GetMouseButton(0)) return;

        _rigidbody.AddForce(transform.forward * Acceleration);
    }

    private void OnCollisionEnter(Collision other)
    {
        if (!other.gameObject.TryGetComponent<EnemyScript>(out var result)) return;

        //ここでは意味が無いけれどboolを返したら便利そう。名前が長いのは実はやばいけど全容が無いので第一部ではご容赦
        var isAlive = result.AddDamageAndCheckIsAlive(PlayerPower);
    }
}

説明

動作に関わるもの

1 : 後々変わりそうな数字でconstは警戒

constについて、詳しくは調べて頂いたらいいかな、と思いますが、簡単に書くと「進め方によっては数字を変更した際にバグる」です。
constは、事前に組み込まれるのでとても軽い部類なのですが、事前に組み込まれるがゆえに、以前組み込んだ数字のまま動くことがある、という現象があります。
なので「将来的に、まだ調整しそうな定数にしたい数値」はstatic readonly などを使ってあげると、気にしなくて良くなります。
最終的に、完全に確定した際には心置きなくconstを使う、のがいいと思います。

2 : 実質毎フレーム取得と不要なpublic

この宣言の仕方はとても便利なのですが、呼ばれ得る度に取得しているので、Updateで使うと実質毎回参照を取ります。大変です。

また、publicは割と怖くて、他のクラス、オブジェクトからも操作できてしまう、という問題があります。
「それの何が悪いの?気にしときゃいいじゃん」って思うかもしれませんが、例えば3つのオブジェクトが参照、変更をかけている時、同時に変更がかかったりしたらどうなるでしょう?やばいです。
実際、既存の有名Asset等でも、これで競合しちゃってUnityエディタがフリーズする、というのが発生することがあります(特にRefresh系)。
少なくとも手元では、その状況をそもそも発生させないためにも、不要なpublicを消すのが好ましいかな、と。
なので、不要な場合には、無しか、privateにしてあげましょう。

4 : 取得する形と違う必要性をなくせそうだし、これ使ってない……?

6で取得していますが、取得→gameObjectに変換、としていますが、だったらEnemyScriptの型で宣言しちゃってもいいのかな、と。
そもそも使ってないし(説明のために書いたんですけどね)。
仮にgameObject経由で取ってきたい情報があるなら、EnemyScriptの中で取得、こっちに公開して流したり。
もうちょっと進むとInterface切り分けとかも出てきそうな感じありますね。

6 : 毎フレームFindは重い!

この言葉のみです。重い!キャッシュする、とか言いますが、一度取得、どこかにしまっておく、いわゆるキャッシュをすることで、不要な参照を避けられ、軽くなります。これは変更後のRigidbodyの部分でやっています。

8 : AddForceってなんだろう?Updateで使わないように

AddForceといえば「FixedUpdateで使おう」とかよく言われます。なんでなんでしょうか?
これは、UnityのPhisics系が、基本FixedUpdateの時間の流れ方(フレーム間の幅)を単位として動いてくれるので、なにも気にしなくても時間という単位に関して解決してくれるためです。
docs.unity3d.com

Updateで使うなら明示的に時間を管理する必要があったりなかったりするので、そもそもFixedUpdateを使う、とすると幸せになれます。
詳しくやると運動方程式とかをちょっとだけやることになります。

10 : これだと消せないかも?

resultに対して取得したものが入っているんですが、今回はEnemyScriptが入っています。これだと、GameObject自体は消せないので、下のほうにあるDestroyがうまくきかないんじゃないかな?ってなりますね。
もちろん、将棋みたいに、やられたら敵という属性がなくなる、とかなら使いみちはあると思います。

11 : 他人が他人の体力をいじれるの、ちょっと怖い

2とも関連します。公開されている数値は、公開の仕方によっては、他人が直接いじることができます。
あくまでも相手にダメージを通知して、相手自身がダメージを処理、結果だけ返したりする、というのが平和ではないでしょうか。
また、ここではPlayerだけがいじっていますが、他の要因、ダメージ床もある、フレンドリーファイアもある、などとなった場合にどんどん変な事になっていきます。
最初のうちは、なるべくなら動作する本人が処理する、のが平和に思えます。

個人的な好み

3 : 名前が変?

命名規則の種類の話ではなく、Script内での名前の規則性がなくて混乱する、というやつです。結構見ます。
Camelがどうとかいうのではなく、最低限内部では揃っていたほうがやりやすいのかな?と思います。

5 : 使わないなら消したい

これだけです。テンプレートを変えない場合、基本出てくるので、残っているのをよく見かけますが、使わないなら消したほうがいいのかなぁ、と思います。
ただ、今回は改造後で使っています。

7 : 早期リターンなんてものもあったり

これは好みですが、条件が1つしかなかったりする場合は、早期リターン、ガード節というのがあります。これもうまくいかない要因を排除できたり、ほんの僅かに軽くなったりがあるっぽいので好きでおすすめです。もちろん場合によります。
そして、今回は段階的に無視しましたが、FixedUpdateでキー入力をみるの、なんか危なかったり、なかったり。