Question There must be a more efficient way to do this

My question is: is there a way to do all of this once in code? None of it is executing at the same time, so it's not exactly consuming more written this way and it works as intended, but I wonder if there's a better way of doing this. If you have any questions let me know!

Code (CSharp):

1.  
2.  
3. private bool PreviousArrayContainsAudioClipIndex()
4.     {
5.         for (int i = 0; i < previousArray.Length; i++)
6.         {
7.             if (previousArray[i] == audioClipIndex)
8.             {
9.                 return true;
10.             }
11.         }
12.         return false;
13.     }
14.     private AudioClip GetClipFromTextureSound(TextureSound TextureSound)
15.     {
16.  
17.         // Initialize
18.         if (previousArray == null)
19.         {
20.  
21.             previousArray = new int[TextureSound.Clips.Length / 2];
22.         }
23.         if (previousArray.Length == 0)
24.         {
25.             // If the the array length is 0 it returns null
26.             return null;
27.         }
28.         else
29.         {
30.             // Psuedo random remembering previous clips to avoid repetition
31.             do
32.             {
33.                 audioClipIndex = Random.Range(0, TextureSound.Clips.Length);
34.             } while (PreviousArrayContainsAudioClipIndex());
35.             // Adds the selected array index to the array
36.             previousArray[previousArrayIndex] = audioClipIndex;
37.             // Wrap the index
38.             previousArrayIndex++;
39.             if (previousArrayIndex >= previousArray.Length)
40.             {
41.                 previousArrayIndex = 0;
42.             }
43.         }
44.         // Returns the randomly selected clip
45.         return TextureSound.Clips[audioClipIndex];
46.     }
47.     private AudioClip GetClipFromTextureSoundForJumpLand(TextureSound TextureSound)
48.     {
49.         int clipIndex = Random.Range(0, TextureSound.JumpClips.Length);
50.         return TextureSound.JumpClips[clipIndex];
51.     }
52.     private AudioClip GetClipNullClips()
53.     {
54.  
55.         // Initialize
56.         if (previousArray == null)
57.         {
58.  
59.             previousArray = new int[NullClips.Length / 2];
60.         }
61.         if (previousArray.Length == 0)
62.         {
63.             // If the the array length is 0 it returns null
64.             return null;
65.         }
66.         else
67.         {
68.             // Psuedo random remembering previous clips to avoid repetition
69.             do
70.             {
71.                 audioClipIndex = Random.Range(0, NullClips.Length);
72.             } while (PreviousArrayContainsAudioClipIndex());
73.             // Adds the selected array index to the array
74.             previousArray[previousArrayIndex] = audioClipIndex;
75.             // Wrap the index
76.             previousArrayIndex++;
77.             if (previousArrayIndex >= previousArray.Length)
78.             {
79.                 previousArrayIndex = 0;
80.             }
81.         }
82.         // Returns the randomly selected clip
83.         return NullClips[audioClipIndex];
84.     }
85.     private AudioClip GetClipNullClipsForJumpLand()
86.     {
87.         int clipIndex = Random.Range(0, NullClipsJumpLand.Length);
88.         return NullClipsJumpLand[clipIndex];
89.     }

 

Well I didn't read all the code but at least you can use IndexOf: https://learn.microsoft.com/fr-fr/dotnet/api/system.array.indexof?view=net-7.0

However instead of remembering you can set the indexes then use those and generate more when it's over.

 

It seems like your methods GetClipFromTextureSound and GetClipNullClips do exactly the same, they just perform the actions on different AudioClip arrays.

As you want to avoid code duplication, you could pass an AudioClip array to the function as for the method, it does not care where the array comes from.

The same with GetClipFromTextureSoundForJumpLand and GetClipNullClipsForJumpLand.

Like GetClip(AudioClip[] clips) and GetClipsForJumpLand(AudioClips[] clips)

 

Yes, they correctly suggest that you need to move the logic of the two methods into a separate method.
Something like this.

Code (CSharp):

1.  
2. //-----------------------------------
3. private AudioClip GetRandomClip(AudioClip[] clips)
4. {
5.     int clipIndex = Random.Range(0, clips.Length);
6.     return clips[clipIndex];
7. }
8.  
9. private AudioClip GetClipFromTextureSoundForJumpLand()
10. {
11.     return GetRandomClip(TextureSound.JumpClips);
12. }
13.  
14. private AudioClip GetClipNullClipsForJumpLand()
15. {
16.     return GetRandomClip(NullClipsJumpLand);
17. }
18.  
19. //-----------------------------------
20.  
21. private AudioClip GetClipFromTextureSound()
22. {
23.     return GetClipsFrom(TextureSound.Clips);
24. }
25. private AudioClip GetClipNullClips()
26. {
27.     return GetClipsFrom(NullClips);
28. }
29.  
30. private AudioClip GetClipFrom(AudioClip[] clips)
31. {
32.     if (previousArray == null)
33.     {
34.         previousArray = new int[clips.Length / 2];
35.     }
36.     if (previousArray.Length == 0)
37.     {
38.         // If the the array length is 0 it returns null
39.         return null;
40.     }
41.     else
42.     {
43.         // Psuedo random remembering previous clips to avoid repetition
44.         do
45.         {
46.             audioClipIndex = Random.Range(0, clips.Length);
47.         } while (PreviousArrayContainsAudioClipIndex());
48.         // Adds the selected array index to the array
49.         previousArray[previousArrayIndex] = audioClipIndex;
50.         // Wrap the index
51.         previousArrayIndex++;
52.         if (previousArrayIndex >= previousArray.Length)
53.         {
54.             previousArrayIndex = 0;
55.         }
56.     }
57.     // Returns the randomly selected clip
58.     return clips[audioClipIndex];
59. }
60.  
61. //-----------------------------------

 

Implement a shuffle algorithm. Fisher–Yates can be written in a few lines of code. Then initialization becomes:

Code (CSharp):

1.  
2. nextIndex = 0;
3. List<AudioClip> sounds = new (TextureSound);
4. Shuffle(sounds);
5.  

Usage becomes:

Code (CSharp):

1.  
2. AudioClip toPlay = sounds[nextIndex++ % sounds.Count];
3.  

This has other advantages too:

• Constant runtime for choosing a sound.
• No chance of an infinite loop if the previously played clip array contains all clips.

 

Thank you! I forgot to thank you, this absolutely works!