koobas.hobune.stream
(Closed)My Android game freezes after 7-12 minutes because of this code
So I've been experiencing freezes on my Android game, only on my device not in the editor.
The game just freezes. The background music continues to play, some sound effects like rain and thunder continue to play but game itself looks like an image; it's just frozen.
I've successfully been able to pinpoint what method is causing it but I can't figure out why. On a more interesting note this only started happening when I upgraded to Unity 4.2. With no modification this worked without a problem on 4.1.
[NOTE]
I also want to point out that I have script debugging on and I'm using LogCat to monitor any errors my phone spits out. When the freezing happens there is NOTHING printed out. According to Unity/my device there are no errors.
This is the code that causes the game to freeze after 7-12 minutes of playing:
public IEnumerator MusicRoutine(){
while(true){
if(playingMusic){
musicSource.Stop();
if(shuffle){
int randomNumber = Random.Range(0, (musicList.Length - 1));
lookingForASong = true;
while(lookingForASong){
for(int i = 0;i < whatSongsHaveWePlayedShuffle.Length;i++){
if(randomNumber == whatSongsHaveWePlayedShuffle[i]){
i = 100;
tempbool = true;
}
if(tempbool == false){
nothingMatched = true;
}else{
nothingMatched = false;
tempbool = false;
}
}
if(nothingMatched){
lookingForASong = false;
nothingMatched = false;
musicSource.clip = null;
yield return null;
yield return null;
musicSource.clip = musicList[randomNumber];
yield return null;
yield return null;
whatSongsHaveWePlayedShuffle[tempIndexNumber] = randomNumber;
tempIndexNumber++;
if(tempIndexNumber >= musicList.Length){
tempIndexNumber = 0;
resetShuffleArrayForWhatSongsHaveBeenPlayed();
}
}else{
randomNumber = Random.Range(0, (musicList.Length - 1));
}
}
}
else{
musicSource.clip = null;
yield return null;
yield return null;
musicSource.clip=musicList[currentTrackID];
yield return null;
yield return null;
currentTrackID+=1;
if(currentTrackID == musicList.Length) {
currentTrackID=0;
}
}
yield return new WaitForSeconds(0.5f);
yield return null;
yield return null;
yield return null;
yield return null;
yield return null;
yield return new WaitForSeconds(1f);
yield return null;
yield return null;
yield return null;
//we must have a hand full of delays to make sure the clip is playable
if(musicSource.clip.isReadyToPlay){ //im not exactly sure how this check works so we'll keep the delays
musicSource.Play(); //start playing
yield return null;
yield return new WaitForSeconds(musicSource.clip.length + 2.3f);
}else{
//im not sure why we wouldn't be ready but if we still aren't ready lets wait
yield return null;
yield return new WaitForSeconds(3f);
yield return null;
if(musicSource.clip.isReadyToPlay){ //one more check for shiggles
musicSource.Play(); //start playing
yield return null;
yield return new WaitForSeconds(musicSource.clip.length + 2.3f);
}else{
//it's still not ready so something is wrong, lets break out;
break;
}
}
yield return null;
}else{
yield return null;
yield return null;
yield return null;
yield return null;
yield return new WaitForSeconds(2f);
yield return null;
yield return null;
yield return null;
}
}
}
It does not matter if shuffle is true or false.
On a more interesting note if I set playingMusic to false so that it doesn't actually execute any of the music playing it does NOT freeze. This value was set to false at STARTUP before it was ever able to execute. (It's a PlayerPref).
Can anyone think of why this is causing my game to freeze?
Thanks
you could try adding some more logs in there to get a sense of in what part of your code he's crashing... if it is always after the same log or not.
It doesn't freeze when you play it in the editor?
One thing though, if you combine yield return null with WaitForSeconds... just wait a few 10th of a second longer and leave out the other yields...
That isn't a bad idea the problem is it doesn't freeze while executing these. Or at least I don't think it does.
Every time it crashes it is in the middle of playing a song. In my code right after I play the song I do
yield return new WaitForSeconds(musicSource.clip.length + 2.3f);
So if it was freezing on some part of the script it wouldn't play the song. If it's in the middle of playing a song it is currently at this yield.
Every time it has froze a song was currently playing.
If for some reason the yield finished before the song was over (seems impossible though) the first thing I do is
musicSource.Stop();
Therefore even if it was still playing it would stop playing the song, then hit the point of code where it "freezes" assu$$anonymous$$g your comment was correct.
And no it has never froze in the editor.
I'm guessing it is not always the same song playing when it crashes?
How is your memory doing?
btw: Your second example would never compile since you don't have a yield inside your function. You need at least one yield or the function is not a coroutine. Also the yield has to be inside the loop or it would crash as well.
@Bunny83 I fixed my main post as you probably already read a few times.
@Joyrider my memory usage is fine. I peak at maybe 200mb. Before I did performance increases I was getting around 320mb on my phone so I know there is plenty of memory left to use. (Using Samsung Galaxy S3 w\ 2gb of ram)
All songs are also streamed from disc not held in memory.
So I figured it out due to a comment below from Bunney83. He stated that Random.Range when using it to return ints it does not include the last number.
So on line 6 and 38 I do:
randomNumber = Random.Range(0, (musicList.Length - 1));
Because of this the last index on the array never gets put as a random number causing my for loop to go into a infinite loop once all songs have been played but the last one. I removed the - 1 and it doesn't freeze.
This doesn't explain why it still froze when shuffle was false. Some more testing will be required...
Let me sum up your script with one word: horrible!
Some more details:
It seems you're missing some fundamental keywords like "break". Your i=100 "solution" is prone to error and makes it really hard to understand your code.
Random.Range comes in two versions, one that takes floats and return floats and one that takes ints and return an int. You use the int version and the "max" parameter is exclusive, so substracting 1 from Length will skip the last one.
Your way of selecting a random song is the most inefficient and your actual problem.
Your "while(lookingForASong)" loop won't exit once each possible song has been played. Since you don't have a yield inside the loop in this case you're caught in an infinite loop. Since you never clear / resets the members of your "whatSongsHaveWePlayedShuffle" array once each song is in that array, none can be replayed. Without a yield you will never exit this loop ==> crash
You should split this code into some meaningful functions since this is a mess. Something like this:
// C#
private int lastPlayedSong = -1;
List<int> GenerateShuffleList(int aCount, int aMakeThisLast)
{
var tmp = new List<int>(aCount);
for (int i = 0; i < aCount; i++)
if (i != aMakeThisLast)
tmp.Add(i);
var result = new List<int>(aCount);
while(tmp.Count > 0)
{
int index = Random.Range(0, tmp.Count);
result.Add(tmp[index]);
tmp.RemoveAt(index);
}
if (aMakeThisLast>=0 && aMakeThisLast < aCount)
result.Add(aMakeThisLast);
return result;
}
IEnumerator PlaySong(int aIndex)
{
aIndex = aIndex % musicList.Length;
lastPlayedSong = aIndex;
musicSource.Stop();
musicSource.clip = musicList[aIndex];
musicSource.Play();
yield return new WaitForSeconds(musicList[aIndex].Length + 0.5f);
}
public IEnumerator MusicRoutine()
{
var shuffleList = GenerateShuffleList(musicList.Length, -1);
while(true)
{
if(playingMusic)
{
if(shuffle)
{
if (shuffleList.Count == 0)
{
shuffleList = GenerateShuffleList(musicList.Length, lastPlayedSong);
}
int index = shuffleList[0];
shuffleList.RemoveAt(0);
yield return StartCoroutine(PlaySong(index));
}
else
{
yield return StartCoroutine(PlaySong(lastPlayedSong + 1));
}
}
else
{
yield return null;
}
}
}
Note: I've written this from scratch an haven't tested it, so there might be some syntax errors ;) You need to add a "using System.Collections.Generic;" at the top.
Thanks for the great constructive criticism! Let me key you in on a few things though.
*First I was unaware that the int version of Random.Range didn't include the last number so thanks!
*Using Random.Range might not be the most efficient but it isn't the true problem. It could be a problem but not THE problem. I can prove this because if shuffle is set to false, therefore it skips all the randomization, it still freezes after x amount of $$anonymous$$utes.
*Also note my app does not CRASH it FREEZES. There is a difference.
*Something I lied about that has now bit me in the ass is my second example (which is now corrected in my main post) about commenting out the code, was a lie. I never tried that. I was at work and needed a quick solution so I copy and pasted. The truth was I set the variable playing$$anonymous$$usic to false so it would just execute the else which was simply some yields. Doing so caused the app to not freeze after ~2 hours of playing.
*You state that I never clear out the array whatSongsHaveWePlayedShuffle and that is wrong. I do. I don't post the method but you can see on line 35 I have this method:
resetShuffleArrayForWhatSongsHaveBeenPlayed();
This method sets all values of the array to -5, therefore "clearing" it.
I'm going to point out that having yield return null; over and over again probably isn't helping.
I doubt having lots of yields would specifically generate a crash. It's an infinite loop, so even if he only had one yield it would accomplish just as much yields in 7-12 $$anonymous$$utes...
Like Joyrider stated having one yield or 500 won't and shouldn't generate a crash. It's simply just waiting for a frame to pass by.
On a side note I added the yields AFTER I realized this is what was causing the freeze. Initially there wasn't any yields or waits.
If you're going to do that, at least throw it in a $$anonymous$$ethod.
public void YieldNumberOfTimes(int numberOfTimes){
for(int i = 0; i < numberOfTimes){
yield return null;
}
}
Better yet, coroutine with WaitForSeconds() as suggested...
@jacobschellenberg: Your function wouldn't compile. Since your function doesn't return an IEnumerator you can't use the yield keyword.
@jacobschellenberg I could of made a method to do so, that is correct. After you spend x amount of hours trying to debug something that should work you get a bit sloppy. I was adding random things all over the place.
I don't plan on keeping them though once this is resolved.
On a side note I lied about the commenting out portion due to not having time and needed a quick copy and paste, I set "playing$$anonymous$$usic" to false is what I was really doing. Setting playing$$anonymous$$usic at startup caused my app not to not crash. I have fixed my main post.
