[BUGFIX] Prevented infinite recursion on freeplay when a song doesn't have the normal difficulty

[AppleHair]

Any attempt to create a song without the normal difficulty will result in a silent crash when opening freeplay. After some debugging, I discovered that the crash was caused by an infinite recursion, which gets triggered at the start of FreeplaySongData's updateValues method.

Code context

In Constants:

/**
 * Default difficulty for charts.
 */
public static final DEFAULT_DIFFICULTY:String = 'normal';

In FreeplaySongData:

public var currentDifficulty(default, set):String = Constants.DEFAULT_DIFFICULTY

function set_currentDifficulty(value:String):String
{
  currentDifficulty = value;
  updateValues(displayedVariations);
  return value;
}

function updateValues(variations:Array<String>):Void
{
  this.songDifficulties = song.listDifficulties(null, variations, false, false);
  if (!this.songDifficulties.contains(currentDifficulty)) currentDifficulty = Constants.DEFAULT_DIFFICULTY;
  //...

Infinite recursion explanation

When updateValues gets called for the first time on a FreeplaySongData object, if its song doesn't have the difficulty specified in its currentDifficulty property (normal), the currentDifficulty property will be set to Constants.DEFAULT_DIFFICULTY(normal), which will trigger its setter-method, which will trigger updateValues again in the same conditions as before, meaning the same thing will keep happening until the program decides to exit.

Usually, the compiler ensures that this kind of field access does not go through the accessor method when made from within the accessor method itself, thus avoiding infinite recursion. However, because the field access happens in a separate method, the compiler can't know if we're nested inside the scope of the accessor method or not, which means he won't be able to prevent infinite recursion in this case.

Solution

So to actually prevent the infinite recursion, I added another condition, which prevents the currentDifficulty property from being set to Constants.DEFAULT_DIFFICULTY(normal) if it's already set to Constants.DEFAULT_DIFFICULTY(normal). Additionally, I added a return expression after we set the currentDifficulty property because, as I already described, this field access triggers the setter-method, which calls the updateValues again with the new value of the currentDifficulty property, which means we don't need to continue the function, because we already went over the rest of the function in a nested call.

[EliteMasterEric]

This describes a similar problem to, but modifies different code from, #2712. Are these different issues?

If #2712 (which has already been approved and merged internally) fixes the issue, this PR is redundant.

[AppleHair]

This describes a similar problem to, but modifies different code from, #2712. Are these different issues?

If #2712 (which has already been approved and merged internally) fixes the issue, this PR is redundant.

It seems #2712 does fix part of the same issue. The only thing it doesn't address is updateValues running twice when the currentDifficulty property actually changes in value, which is what the return expression was added for.

What I can do is remove the second condition I added (currentDifficulty != Constants.DEFAULT_DIFFICULTY) and then this PR should behave as an extension of #2712. The only differece is that the return expression will still run if currentDifficulty = Constants.DEFAULT_DIFFICULTY ended up not changing the value and not running updateValues again, but that's fine, because this also means the current difficulty is still not included in the song, so the method would have returned regardless.