Evaluate my step sound script

Community Showcases
1

Hi!
It’s not a question or a problem, I just need an outside perspective on whether I’m doing the right thing.
So, I created two scripts for the character steps. In both, I call PlayStepsSound() from the animation event. In both cases on each scene I have a script that sets the groundType(enum) of steps for that scene (outside ground, indoors wood, etc.), in both cases I have a set of lists, containing the step sounds from which I randomly play.

In the first case I go the easy way and make a script similar to the one in the Unity demo. What I didn’t like about it: every time we call PlayStepsSound() we check the type of steps. In my game, there is one type of step in every scene.

using System.Collections.Generic;
using UnityEngine;

public class PlayerStepsSound : MonoBehaviour
{
    [SerializeField] private AudioSource stepsAudio;
    [SerializeField] private List<AudioClip> groundClips = new List<AudioClip>();
    [SerializeField] private List<AudioClip> woodenFloorClips = new List<AudioClip>();
    [SerializeField] private List<AudioClip> groundMareClips = new List<AudioClip>();
   

    public enum GroundType
    {
        Ground,
        WoodenFloor,
        GroundMare,
    }

    public GroundType groundType;

    public void PlayStepsSound()
    {
        if(groundType == GroundType.Ground)
            stepsAudio.PlayOneShot(groundClips[Random.Range(0, groundClips.Count)]);
        else if(groundType == GroundType.WoodenFloor)
            stepsAudio.PlayOneShot(woodenFloorClips[Random.Range(0, woodenFloorClips.Count)]);
        else if(groundType == GroundType.GroundMare)
            stepsAudio.PlayOneShot(groundMareClips[Random.Range(0, groundMareClips.Count)]);

    }

In the second case, I use a setter for enum to change the steps type not every time we call it, but only when it actually changes. I also use a void delegate to store the current step method, and in PlayStepSound() I simply invoke this delegate.

using System.Collections.Generic;
using UnityEngine;

public class PlayerStepsSound : MonoBehaviour
{
    [SerializeField] private AudioSource stepsAudio;
    [SerializeField] private List<AudioClip> groundClips = new List<AudioClip>();
    [SerializeField] private List<AudioClip> woodenFloorClips = new List<AudioClip>();
    [SerializeField] private List<AudioClip> groundMareClips = new List<AudioClip>();
   
    private delegate void StepsSoundMethod();
    private StepsSoundMethod stepsSoundMethod;

    public enum GroundType
    {
        Ground,
        WoodenFloor,
        GroundMare,
    }
    private GroundType _groundType;
    public GroundType groundType
    {
        get
        {
            return _groundType;
        }
        set
        {
            _groundType = value;
            if(_groundType == GroundType.Ground)
                stepsSoundMethod = StepsGround;
            else if (_groundType == GroundType.WoodenFloor)
                stepsSoundMethod = StepsWoodenFloor;
            else if (_groundType == GroundType.GroundMare)
                stepsSoundMethod = StepsGroundMare;
        }
    }

    public void PlayStepsSound()
    {
        if(stepsSoundMethod != null)
        {
            stepsSoundMethod();
        }
    }
   
    private void StepsGround()
    {
        stepsAudio.PlayOneShot(groundClips[Random.Range(0, groundClips.Count)]);
    }
    private void StepsWoodenFloor()
    {
        stepsAudio.PlayOneShot(woodenFloorClips[Random.Range(0, woodenFloorClips.Count)]);
    }
    private void StepsGroundMare()
    {
        stepsAudio.PlayOneShot(groundMareClips[Random.Range(0, groundMareClips.Count)]);
    }
}

So, since I haven’t learned how to profile code yet - can you tell me if I’m doing it right? I understand that maybe the second way is more complicated than necessary, but I am self-taught and like to learn things by trial and error.
Thanks!

2

Seems pretty unneccesary, why don’t you just have a reference to which List you want to access?

List<AudioClip> playTheseClips;
public void SetGroundType(GroundType newGroundType)
{
  if (newGroundType == GroundType.Ground)
    playTheseClips = groundClips;
  // etc.
}

public void PlayStepsSound()
{
  stepsAudio.PlayOneShot(playTheseClips[Random.Range(0, groundClips.Count)]);
}
1 Like