Search Unity

Resolved How to destroy a specific gameObject in a list based on a condition?

Discussion in 'Scripting' started by brandonrichards43, Jun 20, 2021.

  1. brandonrichards43

    brandonrichards43

    Joined:
    May 20, 2020
    Posts:
    23
    The problem: I have multiple gameObjects in my world scene that shares a "TriggerBattle".cs script with OnTriggerEnter2D for player. We fade to black, then load my battle scene - then if we won the battle I want to then destroy that specific object once I load back to the world scene.

    It is currently only finding my first hierarchical trigger object and destroys that one.

    Tried: Using DontDestroyOnLoad to retain it into the Battle Scene and disabled the object to 'hide' it, then if I won the battle, a condition I had jerry-rigged up with a simple static bool to switch to true would simply destroy the gameObject. But with my experience it would instead destroy all objects that shared the triggerbattle script opposed to one at random.

    Currently: Using ScriptableObject script that is called "TriggerManager" which I have one asset in the Editor that gets attached to each individual trigger gameObject in my hierarchy. The asset itself stores each "Trigger" GameObject Prefab and a public bool to determine when to call the method inside my triggerbattle.cs

    Finally I have a BattleManager.cs that has an EndBattle method that uses triggerAsset.wasDefeated = true; so this all works, but now I just need to have some guidance / help programmatically to get that specific trigger element, destroy it and keep it destroyed during my game session.

    Expected result: Once I have won a battle, I want that specific gameObject I triggered in my 'World Scene' to persist being deactivated / destroyed in that game session forever. I don't want it to go from the very first list element, but rather find that particular index I triggered into battle and destroy / deactivate that one.

    Code (CSharp):
    1.  
    2. using System.Collections;
    3. using System.Collections.Generic;
    4. using UnityEngine;
    5. using UnityEngine.UI;
    6. using UnityEngine.SceneManagement;
    7.  
    8. public class TriggerBattle : MonoBehaviour
    9. {
    10.     [SerializeField] TriggerManager triggerAsset;
    11.     [SerializeField] GameObject fadeScreenObject;
    12.     [SerializeField] GameObject cameraObject;
    13.     [SerializeField] string sceneManager;
    14.     FadeImage fadeImage;
    15.  
    16.     void Awake()
    17.     {
    18.         fadeImage = FindObjectOfType<FadeImage>();
    19.         cameraObject = FindObjectOfType<Camera>().gameObject;
    20.         fadeScreenObject = FindObjectOfType<FadeImage>().gameObject;
    21.     }
    22.  
    23.     void Start()
    24.     {
    25.         if (triggerAsset.wasDefeated)
    26.         {
    27.             gameObject.tag = "Destroy";
    28.             triggerAsset.DestroyThis();
    29.             Debug.Log(gameObject.tag);
    30.         }
    31.     }
    32.  
    33.     IEnumerator OnTriggerEnter2D(Collider2D other)
    34.     {
    35.         if (other.GetComponent<Unit>().isPlayer)
    36.         {
    37.             Debug.Log("Which one was triggered? " + gameObject.name);
    38.             fadeScreenObject.gameObject.SetActive(true);
    39.             yield return StartCoroutine(fadeImage.FadeSlowly(true));
    40.             yield return new WaitForSeconds(0.5f);
    41.             cameraObject.gameObject.SetActive(false);
    42.             Destroy(cameraObject);
    43.             SceneManager.LoadScene(sceneManager);
    44.         }
    45.     }
    46. }
    47.  
    Code (CSharp):
    1. using System.Collections;
    2. using System.Collections.Generic;
    3. using UnityEngine;
    4.  
    5. [CreateAssetMenu(fileName = "Trigger Asset", menuName = "AssetMenu/Create Trigger")]
    6. public class TriggerManager : ScriptableObject
    7. {
    8.     [SerializeField] public List<GameObject> triggerObjs = new List<GameObject>();
    9.     [SerializeField] public bool wasDefeated = false;
    10.  
    11.     public void DestroyThis()
    12.     {
    13.         if (wasDefeated)
    14.         {
    15.             GameObject go = GameObject.FindGameObjectWithTag("Destroy").gameObject;
    16.             for (int i = 0; i < triggerObjs.Count; i++)
    17.             {
    18.                 if (go.CompareTag("Destroy"))
    19.                 {
    20.                     Destroy(go);
    21.                     triggerObjs.RemoveAt(i);
    22.                 }
    23.             }
    24.             wasDefeated = false;
    25.         }
    26.     }
    27. }
     
    Last edited: Jun 20, 2021
  2. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,749
    Pull all the bits apart and get far up and above the Unity details.

    Identify the data flows:

    - sense the onset of combat itself
    - process that event into a combat event that contains:
    ---> what it needs to be, including:
    -------> who is doing battle
    -------> all subsequent things like the destruction if you win, etc
    ---> go forward and run everything, including the followups as necessary
     
    brandonrichards43 likes this.
  3. brandonrichards43

    brandonrichards43

    Joined:
    May 20, 2020
    Posts:
    23
    Thanks Kurt.

    I am wondering, is there any particular reasoning behind why Unity adds one index for every GameObject that has my TriggerBattle.cs into a list when I'm not using the function FindObjectsOfType<TriggerBattle>() when adding to the list?

    Because they're GameObjects each with the TriggerBattle component is that why?

    Right now this is occuring when I'm in my debugging process.

    Code (csharp):
    1. [SerializeField] public List<TriggerBattle> destroyTrigger = new List<TriggerBattle>();
    I added to my TriggerManager SO script.

    Code (CSharp):
    1.     IEnumerator OnTriggerEnter2D(Collider2D other)
    2.     {
    3.         if (other.GetComponent<Unit>().isPlayer)
    4.         {
    5.             triggerAsset.destroyTrigger.Add(gameObject.GetComponent<TriggerBattle>());
    6.         }
    I simply use the reference of my triggerAsset from my gameObject(s) I activate via my TriggerEnter2D function and instead of simply indexing that particular gameObject.. it's adding every gameObject it can find from my hierarchy that is associated with my triggerbattle script.

    Is that because I have this "TriggerManager triggerAsset" on each individual trigger object from my scene since it lives on my TriggerBattle.cs?

    Cause I have debug.log commands that log what the name of the trigger is, and it's correctly displaying the trigger I'm interacting with on contact but it's still automatically just adding every trigger to my destroyTrigger which is only called once on my TriggerEnter2D.
     
  4. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,749
    It appears to be because of the blurred line between editor code and game code in the Unity editor. You are running OnTriggerEnter2D() as part of your game, but then your .Add() method is actually modifying the serialized scriptable object, changing it on disk. In editor this is permanent, on a build it would only be for the duration of the executable session, perhaps even less.

    So... the missing part here is to understand the difference between runtime data and build-time data, and you have just reached across that boundary inadvertently because of how blurred the line is between those two areas.

    Some strategies to address this are to make a copy of that array in OnEnable() and modify that copy. OR... you can just Instantiate<T>() a fresh copy of that SO that lives only in memory during gameplay, leaving the one on disk unmodified.
     
    path14 likes this.
  5. brandonrichards43

    brandonrichards43

    Joined:
    May 20, 2020
    Posts:
    23
    Thanks again Kurt, I am still fairly new to programming and I'm sure my code fully displays this. Anyways great news to be shared, I was able to find a specific gameObject by creating two tags that I plan to universally implement and re-use (hopefully) in multiple scenes. "t_Battle1" and "t_Battle2" which allows me to uniquely separate each trigger in my first world scene so.. here's my update with that implementation.

    Oh and one other thing, I removed scriptable objects following with what you mentioned above about the way I was permanently modifying the asset. Also on top of that I want to be able to eventually save what triggers should be destroyed or active based on information with things I want to eventually track. If I'm mistaken just let me know and I'll revisit it! :)


    Here's the Improved TriggerBattle script that lives on each enemy world object!
    Code (CSharp):
    1. using System.Collections;
    2. using System.Collections.Generic;
    3. using UnityEngine;
    4. using UnityEngine.UI;
    5. using UnityEngine.SceneManagement;
    6.  
    7. public class TriggerBattle : MonoBehaviour
    8. {
    9.     TriggerManager triggerManager;
    10.     [SerializeField] GameObject fadeScreenObject;
    11.     [SerializeField] GameObject cameraObject;
    12.     [SerializeField] string sceneManager;
    13.     FadeImage fadeImage;
    14.  
    15.     void Awake()
    16.     {
    17.         fadeImage = FindObjectOfType<FadeImage>();
    18.         cameraObject = FindObjectOfType<Camera>().gameObject;
    19.         fadeScreenObject = FindObjectOfType<FadeImage>().gameObject;
    20.         triggerManager = FindObjectOfType<TriggerManager>();
    21.     }
    22.  
    23.     void Start()
    24.     {
    25.         // Scene load in check to see whether object should get destroyed...
    26.  
    27.         CheckForDestruction();
    28.  
    29.         // If the battle was victorious, execute to destroy activated trigger...
    30.  
    31.         if (Unit.playerWonEncounter)
    32.         {
    33.             triggerManager.DestroyThis();
    34.             Unit.playerWonEncounter = false;
    35.         }
    36.     }
    37.  
    38.     void CheckForDestruction()
    39.     {
    40.         if (TriggerManager.destroyedTriggers.Contains("t_Battle1")
    41.             || TriggerManager.destroyedTriggers.Contains("t_Battle2"))
    42.         {
    43.             foreach (string triggerStaticString in TriggerManager.destroyedTriggers)
    44.             {
    45.                 for (int staticIndex = 0; staticIndex < triggerStaticString.Length;
    46.                 staticIndex++)
    47.                 {
    48.                     Destroy(gameObject);
    49.                 }
    50.             }
    51.         }
    52.     }
    53.  
    54.     IEnumerator OnTriggerEnter2D(Collider2D other)
    55.     {
    56.         if (other.GetComponent<Unit>().isPlayer)
    57.         {
    58.             triggerManager.activatedTriggers.Add(gameObject.name);
    59.             Debug.Log(triggerManager.activatedTriggers[0]);
    60.             fadeScreenObject.gameObject.SetActive(true);
    61.             yield return StartCoroutine(fadeImage.FadeSlowly(true));
    62.             yield return new WaitForSeconds(0.5f);
    63.             cameraObject.gameObject.SetActive(false);
    64.             Destroy(cameraObject);
    65.             SceneManager.LoadScene(sceneManager);
    66.         }
    67.     }
    68. }
    Here's the updated TriggerManager script. Are singletons bad for this?
    Code (CSharp):
    1. using System.Collections;
    2. using System.Collections.Generic;
    3. using UnityEngine;
    4.  
    5. public class TriggerManager : MonoBehaviour
    6. {
    7.     // Configuration Parameters
    8.     [SerializeField] public List<string> activatedTriggers = new List<string>();
    9.     [SerializeField] public List<GameObject> triggerObjs = new List<GameObject>();
    10.  
    11.  
    12.     // Persistent Variables & References
    13.     public static List<string> destroyedTriggers = new List<string>();
    14.     public static TriggerManager instance;
    15.  
    16.     void Awake()
    17.     {
    18.         TriggerManagerSingleton();
    19.     }
    20.  
    21.     void TriggerManagerSingleton()
    22.     {
    23.         if (instance == null)
    24.         {
    25.             instance = this;
    26.             DontDestroyOnLoad(this);
    27.         }
    28.         else
    29.         {
    30.             gameObject.SetActive(false);
    31.             Destroy(gameObject);
    32.         }
    33.     }
    34.  
    35.     public void DestroyThis()
    36.     {
    37.         if (Unit.playerWonEncounter)
    38.         {
    39.             GameObject findGameObject = GameObject.FindGameObjectWithTag(activatedTriggers[0]);
    40.             Debug.Log(findGameObject.name);
    41.  
    42.             if (findGameObject.tag == activatedTriggers[0])
    43.             {
    44.                 destroyedTriggers.Add(findGameObject.tag);
    45.                 Destroy(findGameObject.gameObject);
    46.                 activatedTriggers.Clear();
    47.             }
    48.         }
    49.         else
    50.         {
    51.             Debug.Log(findGameObject.tag + " doesn't match any of the existing triggers in 'destroyed triggers' list!");
    52.             return;
    53.         }
    54.     }
    55. }

    So with my implementation I'm able to activate the trigger I ran into, if I was victorious I simply turn my static bool playerWonEncounter to true, then my TriggerBattle script will call the DestroyThis method from TriggerManager which then finds that particular gameObject with the tag stored from the OnTriggerEnter2D.

    Storing a string static reference that is kept between scenes to ensure Triggers always get destroyed whenever I'm loading in and out of other battles or going to and from other world scenes.

    With my current code is there anyway I could improve the code in a big way? Is there anything egregiously misused or out of place?

    I'd love to learn how to clean code up and fix anything major. But thanks again Kurt I believe after reading what you wrote out it got me thinking and finally was able to solve an issue I was struck on for over a week.
     
    Kurt-Dekker likes this.
  6. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,749
    I would identify the mixture of the
    destroyedTriggers
    static variable inside of a class that is actually a singleton-ish construct as being unorthodox. Seems like it should just be an instance field to me, but I'm not inside your head.

    ALSO... for Unity singletons, I like to keep it even simpler than the above.

    Simple Singleton (UnitySingleton):

    Some super-simple Singleton examples to take and modify:

    Simple Unity3D Singleton (no predefined data):

    https://gist.github.com/kurtdekker/775bb97614047072f7004d6fb9ccce30

    Unity3D Singleton with a Prefab (or a ScriptableObject) used for predefined data:

    https://gist.github.com/kurtdekker/2f07be6f6a844cf82110fc42a774a625

    These are pure-code solutions, do not put anything into any scene, just access it via .Instance!

    If it is a GameManager, when the game is over, make a function in that singleton that Destroys itself so the next time you access it you get a fresh one, something like:

    Code (csharp):
    1. public void DestroyThyself()
    2. {
    3.    Destroy(gameObject);
    4.    Instance = null;    // because destroy doesn't happen until end of frame
    5. }
     
    brandonrichards43 likes this.
  7. brandonrichards43

    brandonrichards43

    Joined:
    May 20, 2020
    Posts:
    23
    Yeah you're totally right!

    I wasn't using the "instance" and thought I needed my variable "destroyedTriggers" to be static, but actually it was completely void of any significance. Not only that but I was able to clean up my CheckForDestruction() method inside TriggerBattle.cs by removing the redundant for loop and simply added a check

    "if (TriggerManager.instance.destroyedTriggers.Count >= 1)"
    then inside the foreach loop with another check

    "if (gameObject.tag == triggerStaticString) { Destroy(gameObject); }
    Now I don't have to worry about writing out each individual condition. Thanks again mate :)

    Without getting off topic too much since I did switch this to a resolved question - I'm still trying to learn which path I want to persist data when it comes to serializers. My question is - how do DontDestroyOnLoad / Singletons work with a Save / Load system?

    Are there any pitfalls to them or will they work with any serializer just like any gameObject / components that don't persist between scenes using that method?
     
    Last edited: Jun 25, 2021
  8. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,749
    If you go with an
    ISaveLoadable
    type of interface, eg, an interface that can express values for saving and loading, and that is implemented properly, it shouldn't matter if it is a singleton, or scene-singleton, or game-life singleton.

    The main key is make sure you draw your "boundaries" around when stuff happens:

    - start game (destroy any and all previous singletons)
    - IF you load, then read the save file, inject all the data
    - play game
    - whenever you save, read the data, write it to save file

    Just understand and positively control the lifecycle of your singletons / services. "You go away now at the end of the game," which will cut down confusion with stray stuff lying around.
     
    brandonrichards43 likes this.