Reload on Death Script fails on second death.

Unity Engine
1

I have a script that I attach to a persistent object that observes the player’s HP to determine if the camera needs to fade to black, then perform a reload on the current scene.

The problem is, for some reason this code will behave properly on the first death, but on the second will get stuck inside the coroutine loop, “Fading the screen out…” In fact, the preceding Fader.FadeOut() method dosen’t even do anything as far as I can tell.

I’m clearly missing something here. Since it’s a Coroutine, do I have to somehow reset it to a default state or something? I’m not knowledgeable about IEnumerators, and I suspect that’s where my problem lies.

HealthSystem.cs
This behavior goes on the player.

using UnityEngine;

public class HealthSystem : MonoBehaviour 
{
    #region Variables / Properties

    public int HP;
    public int MaxHP;

    #endregion Variables / Properties

    // SNIP Unnecessary items to this question.
}

Fader.cs
This behavior goes on the main camera.

using UnityEngine;

public class Fader : MonoBehaviour 
{
	#region Constants
	
	private const float _OpaqueEnough = 0.99f;
	
	#endregion Constants
	
	#region Variables / Properties
	
	public Texture2D FadeImage;
	public float FadeRate = 0.1f;
	public Color Tint = new Color(0,0,0, 1);
	public Color TargetTint = new Color(0,0,0, 0);
	
	public bool ScreenHidden
	{
		get { return Tint.a >= _OpaqueEnough; }
	}
	
	public bool ScreenShown
	{
		get { return Tint.a < _OpaqueEnough; }
	}
	
	#endregion Variables / Properties
	
	#region Engine Hooks

    void OnGUI()
	{
		GUI.color = Tint;
		GUI.depth = -1;
		GUI.DrawTexture(new Rect(0,0, Screen.width,Screen.height), FadeImage);
	}
	
	void FixedUpdate() 
	{
		Tint = Color.Lerp(Tint, TargetTint, FadeRate * Time.deltaTime);
	}
	
	#endregion Engine Hooks
	
	#region Methods
	
	public void FadeOut()
	{
		Tint = new Color(0,0,0, 0);
		TargetTint = new Color(0,0,0, 1);
	}
	
	public void FadeIn()
	{
		Tint = new Color(0,0,0, 1);
		TargetTint = new Color(0,0,0, 0);
	}
	
	#endregion Methods
}

TransitionManager.cs
This behavior goes on the persistent object.

sing UnityEngine;
using System;
using System.Linq;
using System.Collections.Generic;

public class TransitionManager : ManagerMonoBehavior
{
	#region Variables / Properties

	public string DebugCreateTime;
	
	public Vector3 spawnPosition;
	public Vector3 spawnRotation;
	public int targetSceneID = -1;
	public string targetSceneName = string.Empty;
	public GameObject playerPiece;
	
	public DateTime CreateTime { get; private set; }
	
    public static TransitionManager Instance
	{
		get { return TransitionManager.FindOldestInstance(); }
	}
	
	#endregion Variables / Properties
	
	#region Engine Hooks
	
	#endregion Engine Hooks
	
	#region Base Class Overrides
	
	public override void SelfDestructIfOthersExist()
	{
		TransitionManager[] objects = (TransitionManager[]) FindObjectsOfType(typeof(TransitionManager));
		IEnumerable<TransitionManager> destructables = objects.Where(o => o.IsInitialInstance == false);
		
		if(DebugMode)
			Debug.LogWarning(destructables.Count() + " other managers were found!  Destroying them.");
		
		foreach(TransitionManager current in destructables)
		{
			Destroy(current.gameObject);
		}
	}
	
	#endregion Base Class Overrides
	
	#region Methods
	
	public static TransitionManager FindOldestInstance()
	{	
		TransitionManager[] objects = (TransitionManager[]) FindObjectsOfType(typeof(TransitionManager));
		if(objects.Length == 0)
			throw new Exception("TransitionManager could not find any Transition Managers in the scene!");
		
		TransitionManager oldest = objects.FirstOrDefault(a => a.IsInitialInstance == true);
		if(oldest == default(TransitionManager))
		{
			oldest = objects.First();
			oldest.IsInitialInstance = true;
		}
		
		return oldest;
	}
	
	public void ChangeScenes()
	{	
		if(targetSceneID == -1)
		{
			Application.LoadLevel(targetSceneName);
		}
		else
		{
			Application.LoadLevel(targetSceneID);
		}
	}
	
	// Information:
	// -------------------------------------------------------------------------------
	// The following methods are responsible for the player piece being copied to the 
	// destination scene when it has loaded, via the SceneSpawner script (this script
	// is pre-set on the GameCamera prefab!)
	//
	// In the prior scene, the player's piece is acquired by the manager, which means 
	// a clone is saved to the class.
	//
	// In the target scene, when it is loaded, the SceneSpawner will Instantiate the 
	// piece, then lock the camera to it.
	
	public void AcquirePlayerPiece(GameObject piece)
	{
		if(piece == null)
			throw new ArgumentNullException("Must specify a game object to copy and transition to the new scene.");
		
		playerPiece = (GameObject) GameObject.Instantiate(piece, piece.transform.position, piece.transform.rotation);
		playerPiece.name = piece.name;
		
		UnityEngine.Object.Destroy(piece);
		UnityEngine.Object.DontDestroyOnLoad(playerPiece);
	}
	
	// Information:
	// -------------------------------------------------------------------------------
	// The following methods are used when the transition is ready, for instance, when
	// the screen is fully faded and the player cannot see it.
	//
	// You give these methods the position, rotation, and scene info for the scene you
	// want to perform the transition to.  ChangeScene() will automatically call that
	// information and perform the scene swap.
	
	public void PrepareTransition(Vector3 targetPos, Vector3 targetRot, int sceneID)
	{
		if(DebugMode)
			Debug.Log("Prepping a transition to: " + Environment.NewLine
				      + "Location: " + targetPos + Environment.NewLine
				      + "Rotation: " + targetRot + Environment.NewLine
				      + "Scene ID: " + sceneID);
		
		SetLocationRotation(targetPos, targetRot);
		targetSceneID = sceneID;
		targetSceneName = string.Empty;
	}
	
	public void PrepareTransition(Vector3 targetPos, Vector3 targetRot, string sceneName)
	{
		if(DebugMode)
			Debug.Log("Prepping a transition to: " + Environment.NewLine
				      + "Location: " + targetPos + Environment.NewLine
				      + "Rotation: " + targetRot + Environment.NewLine
				      + "Scene: " + sceneName);
		
		SetLocationRotation(targetPos, targetRot);
		targetSceneName = sceneName;
		targetSceneID = -1;
	}
	
	private void SetLocationRotation(Vector3 targetPos, Vector3 targetRot)
	{
		spawnPosition = targetPos;
		spawnRotation = targetRot;
	}
	
	#endregion Methods
}

PlayerDeathReloader.cs
This behavior goes on the persistent object.

using UnityEngine;
using System.Collections;

public class PlayerDeathReloader : MonoBehaviour 
{
	#region Variables / Properties
	
	public bool DebugMode = false;
	public GameObject Player;
	public int LastHP;

	private Fader _fader;
	private HealthSystem _health;
	private TransitionManager _sceneChange;
	
	#endregion Variables / Properties
	
	#region Engine Hooks
	
	public void Start()
	{
		_sceneChange = TransitionManager.Instance;
		_fader = (Fader) FindObjectOfType(typeof(Fader));

		AcquirePlayer();
	}
	
	public void Update()
	{
		if(_health.HP == LastHP)
			return;

		if(DebugMode)
			Debug.Log("The player's HP has changed from " + LastHP + " to " + _health.HP);

		LastHP = _health.HP;
		if(_health.HP > 0)
			return;

		if(DebugMode)
			Debug.Log("The player has died!");

		StartCoroutine(ReloadLevelSequence());
	}

	public void OnLevelWasLoaded()
	{
		AcquirePlayer();
	}

	#endregion Engine Hooks
	
	#region Methods

	private void AcquirePlayer()
	{
		Player = GameObject.FindWithTag("Player");
		_health = Player.GetComponent<HealthSystem>();
		LastHP = _health.HP;
	}
	
	private IEnumerator ReloadLevelSequence()
	{
		if(DebugMode)
			Debug.Log("Performing level reload...");
		
		_fader.FadeOut();
		while(_fader.ScreenShown)
		{
			if(DebugMode)
				Debug.Log("Fading the screen out...");

			yield return 0;
		}
		
		_sceneChange.ChangeScenes();
	}
	
	#endregion Methods
}
2

When you write persistent object do you mean you set it to ‘DontDestroyOnLoad’? If so do you also set the Fader to a persistent object?

If the Fader isn’t a persistent object I’m thinking you should get an exception in you’re ReloadLevelSequence method because the _fader object isn’t valid anymore after the new scene is loaded. The Start() method of you’re PlayerDeathReloader is called only the first time the object is created and not each time a scene is loaded.

P.S.: Thanks for the code. I’m on the search for something simillar to fade to a picture during scene changes :wink:

3

Out of interest, how exactly are you doing this? (In the middle of something, can’t read all that code right now.)

Edit: Ok, turns out I do have time to read that code while I wait for an upload. I notice that you’re polling the health value from another object. While it’s probably not the cause of the issue here, it’s a pretty inefficient way to do things. If I may suggest an improvement, whenever the player’s health is changed check if it’s <= 0 (make sure it’s private and change it via a property or a setter function so you can track it easily). If it is, then fire an event (call a delegate, broadcast a message, whatever). The fader can listen for that event and kick in all on its own, without the need for a 3rd party object that just sits there chewing up a little CPU time and making your program a little more complicated than it needs to be.

As a bonus, it removes the opportunity for bugs in the “watcher” code, and means you can have fewer persistent objects. Which may in fact solve the problem, depending on where it’s coming from.

4

angrypenguin, thanks for the idea!

First, the persisting object is a DontDestroyOnLoad object.

Next, I implemented a ‘OnHealthChanged(int[ ] hpArgs)’ method in both the death-watcher, and the system that presents the player’s HP. I told the HealthSystem, when it receives damage or healing to send a message to these two things, with the hp and max hp wrapped up in the integer array. I tried dying twice, but the problem still remains. I’m confident the problem is not in the receipt of the HP changed event, or health system code. I suspect the coroutine still.

Here’s my updated Death watcher code:

using UnityEngine;
using System;
using System.Collections;

public class PlayerDeathReloader : MonoBehaviour 
{
	#region Variables / Properties
	
	public bool DebugMode = false;

	private Fader _fader;
	private HealthSystem _health;
	private TransitionManager _sceneChange;
	
	#endregion Variables / Properties
	
	#region Engine Hooks
	
	public void Start()
	{
		_sceneChange = TransitionManager.Instance;
		_fader = (Fader) FindObjectOfType(typeof(Fader));
	}

	public void OnLevelWasLoaded()
	{
		StandardDebugMessage("Level was successfully reloaded.");
		_fader = (Fader) FindObjectOfType(typeof(Fader));
	}

	#endregion Engine Hooks
	
	#region Methods

	public void OnHealthChanged(int[] hpArgs)
	{
		int hp = hpArgs[0];
		StandardDebugMessage("HP has changed!");

		if(hp > 0)
			return;

		StartCoroutine(ReloadLevelSequence());
	}

	private IEnumerator ReloadLevelSequence()
	{
		StandardDebugMessage("Reloading level...");
		
		_fader.FadeOut();
		while(_fader.ScreenShown)
		{
			StandardDebugMessage("Fading screen out...");
			yield return 0;
		}
		
		_sceneChange.ChangeScenes();
	}

	private void StandardDebugMessage(string message)
	{
		if(! DebugMode)
			return;

		Debug.Log(string.Format("({0}) {1}", DateTime.Now.ToString("HH:mm:ss"), message));
	}
	
	#endregion Methods
}

EDIT: Woo-hoo I figured it out! I wasn’t re-acquiring the fader on level load. Fixed the above code. :slight_smile:

5

Nice. Now for some further advancement. :slight_smile:

That’s called “tightly coupled” code because the calling object has to be aware of the called object’s type. It’ll work fine, so don’t sweat for now, but consider alternate approaches in the future as this makes the code less portable and harder to maintain. I’d suggest looking up “loose coupling” and seeing how you can improve your designs. But, like I said, don’t let it worry you at the moment - one step at a time.

6

One of the things I’ve done since this, is creating an “HealthChangedEvent” object (I had to add some stuff so that enemy HP changes don’t display on the HUD!) that the Message takes as an argument.

Ideally, I would also assign these messengable objects (the observer and HUD) an interface so I could instead just do a FindObjectsOfType(typeof(IWatchesHealth));, but that dosen’t work, since types have to extend from MonoBehaviour, and such an interface would very much be stand-alone (it would specifically enforce the ‘void OnHealthChanged(HealthChangedEvent arg)’ as its contract.).

Oh yeah - I improved this implementation by creating a HealthChangedEvent object that tells what changed, and from what tag (that way, my HUD dosen’t draw enemy HP updates.)

It also occurs to me, I could implement a static Instance property for the Fader, since there is usually only ever one Fader in existence in any given scene.

7

You can extend from MonoBehaviour and implement interfaces at the same time. Interfaces don’t define behaviour, so they avoid the whole diamond-of-death thing that multiple-inheritance introduces.

Depending on the contents of your HealthChangedEvent object I’d suggest making it a struct. Creating objects means heap allocation, which means garbage collection. Or, the approach that I use, is to use delegates and pass all of the relevant data in its arguments. (Note that delegates can cause hidden allocation if you’re not careful with them, though. For best results stick to usage patterns where allocation is done at load time.)

8

Oh no, I’m well aware of this (in fact in my codebase I do just that in a couple of places.) The problem is, trying to do FindObjectOfType against an interface won’t work, because the type that is being searched for has to derive from MonoBehaviour. Interfaces can implement other interfaces, but to my knowledge an interface cannot extend a base class.

Didn’t know about delegates causing allocations. Does this apply to 1st Class functional types like Action<> and Func<>? And, why do structs not cause heap allocation?

9

Those are things you’re going to have to look into for yourself. :wink: