Search Unity

  1. Unity 6 Preview is now available. To find out what's new, have a look at our Unity 6 Preview blog post.
    Dismiss Notice
  2. Unity is excited to announce that we will be collaborating with TheXPlace for a summer game jam from June 13 - June 19. Learn more.
    Dismiss Notice
  3. Dismiss Notice

Question inventory troubles

Discussion in 'Scripting' started by union3d, May 12, 2024.

  1. union3d

    union3d

    Joined:
    Dec 8, 2012
    Posts:
    15
    hello so tldr to keep items persistence in my project I've attached a dontdestroy script to them my problem arises from the fact I can pick one test item up in the room and it shows up the first inventory slot but as I go to the door and load a new scene turn around and go back to the item room and try picking the other test item up it does not show up in the 2nd inventory slot as intended I've don't some debugging its not the pickup script but its the OnpickupItem function in my script

    help would be appreciated as I have no clue on what to due

    Code (CSharp):
    1. using System.Collections;
    2. using System.Collections.Generic;
    3. using UnityEngine;
    4. using TMPro;
    5. using UnityEngine.EventSystems;
    6. using FMODUnity;
    7.  
    8. public class InventorySelectController : MonoBehaviour
    9. {
    10.  
    11.  
    12.    
    13.    [SerializeField] private GameObject _inventoryView;
    14.    [SerializeField] private TMP_Text _itemNameText;
    15.    [SerializeField] private TMP_Text _itemDescriptionText;
    16.    [SerializeField] private EventReference OpenInventorySound;
    17.    [SerializeField] private EventReference CloseInventorySound;
    18.    [SerializeField] private List<ItemSlot> _slots;
    19.  
    20.    [SerializeField] private Playermovement _Playermovement;
    21.    [SerializeField] private ScreenFader _fader;
    22.  
    23.    private enum State
    24.    {
    25.    menuClosed,
    26.    menuOpen,
    27.    };
    28.  
    29.    private State _state;
    30.  
    31.  
    32.    private void OnEnable()
    33.    {
    34.     EventBus.Instance.onPickUpItem += OnItemPickedUp;
    35.    }
    36.  
    37.  
    38.    private void OnDisable()
    39.    {
    40.     EventBus.Instance.onPickUpItem -= OnItemPickedUp;
    41.    }
    42.  
    43.    private void OnItemPickedUp(ItemData itemData)
    44.    {
    45.     foreach (var slot in _slots)
    46.     {
    47.       if (slot.isEmpty())
    48.       {
    49.         slot.itemData = itemData;
    50.         Debug.Log("slot has been filled");
    51.         break;
    52.       }
    53.     }
    54.    }
    55.  
    56.    public void OnSlotSelected(ItemSlot selectedSlot)
    57.    {
    58.      if (selectedSlot.itemData == null)
    59.      {
    60.      _itemNameText.ClearMesh();  
    61.      _itemDescriptionText.ClearMesh();
    62.    
    63.      return;
    64.      }
    65.       _itemNameText.SetText(selectedSlot.itemData.Name);
    66.       _itemDescriptionText.SetText(selectedSlot.itemData.Description[0]);
    67.    
    68.      if (selectedSlot == null)
    69.      {
    70.       Debug.Log("slot selected");
    71.      
    72.      }  
    73.    }
    74.  
    75. private void Update()
    76. {
    77. if (Input.GetKeyDown(KeyCode.Tab))
    78. {
    79.    if (_state == State.menuClosed)
    80.    {
    81.     EventBus.Instance.PauseGamePlay();
    82.    Debug.Log(_state);
    83.    _fader.FadeToBlack(0.3f, FadeFromMenuCallback);
    84.    _state = State.menuOpen;
    85.    }
    86.    else if (_state == State.menuOpen)
    87.    {
    88.     EventBus.Instance.ResumeGamePlay();
    89.   Debug.Log(_state);
    90.    _fader.FadeToBlack(0.3f, FadeToMenuCallback);
    91.    _state = State.menuClosed;
    92.    }
    93. }
    94.  
    95. }
    96. private void FadeFromMenuCallback()
    97. {
    98. AudioManager.instance.PlayOneShot(OpenInventorySound, this.transform.position);
    99. _inventoryView.SetActive(true);
    100. _fader.FadeFromBlack(0.3f, null);
    101. //Debug.Log(_state);
    102. }
    103. private void FadeToMenuCallback()
    104. {
    105. AudioManager.instance.PlayOneShot(OpenInventorySound, this.transform.position);
    106. _inventoryView.SetActive(false);
    107. _fader.FadeFromBlack(0.3f, null);
    108. //Debug.Log(_state);
    109. }
    110.  
    111.  
    112. }
    113.  
     
  2. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    8,381
    I mean your OnItemPickedUp method is exceedingly simple. And at glance I don't see how it could be the issue, unless there is something wrong with your
    isEmpty()
    method. What debugging have you done to make you think it's the issue? It sounds like you have more debugging to do.
     
  3. union3d

    union3d

    Joined:
    Dec 8, 2012
    Posts:
    15
    in the on pick up method the debug log tells me if the slots have been filled as you can see here after picking up an item before leaving the room the slots filled but when I come back and pick up the other object the log will not fire off

    also here is an image of the hierarchy for persistent objects

    upload_2024-5-12_1-17-47.png
    upload_2024-5-12_1-14-36.png
    here is the log after I picked up the item that was now a persistent object on dontdestroyonload
    upload_2024-5-12_1-20-57.png
    the pickup script work and destroyed the game object but the debof log for slot filled did not fire off
     
  4. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    8,381
    Right, that much is obvious. Like I said, the method is exceedingly simple. It alone in isolation is probably not the problem. Have you check your
    isEmpty()
    method? Have you confirmed there is actually more than one slot? Is there something wrong with your DDoL implementation? Honestly, using DDoL for persistent items in an inventory system raises alarm bells, as does that "there is more than one manager" log message. You need to keep debugging.
     
  5. union3d

    union3d

    Joined:
    Dec 8, 2012
    Posts:
    15

    added a debug log to the isEmpy method nothing showed up upon picking up an item upload_2024-5-12_1-52-45.png
     
  6. ArachnidAnimal

    ArachnidAnimal

    Joined:
    Mar 3, 2015
    Posts:
    1,936
    How do you handle the situation when all slots are full?
     
  7. union3d

    union3d

    Joined:
    Dec 8, 2012
    Posts:
    15
    like in resident evil 1 there will be a storage box
     
  8. ArachnidAnimal

    ArachnidAnimal

    Joined:
    Mar 3, 2015
    Posts:
    1,936
    Your code is assuming there will be an empty slot.

    What is DDoL? Google search finds 0 results for this acronym.
     
  9. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    8,381
    Which means the code isn't running. Which means something is probably up with your inventory slots. Which means you have your next point to investigate. Don't just add one more Debug.Log and keeping coming back to us. Keep investigating. We can't debug your code for you.

    DontDestroyOnLoad.
     
    ArachnidAnimal likes this.
  10. union3d

    union3d

    Joined:
    Dec 8, 2012
    Posts:
    15
    DonDestroyOnLoad
     
  11. Ryiah

    Ryiah

    Joined:
    Oct 11, 2012
    Posts:
    21,672
    The
    return
    keyword exits the method as soon as its reached. Since you're calling
    return
    before the
    Debug.Log
    the logging will never happen even when it should have.
     
    spiney199, union3d and Kurt-Dekker like this.
  12. union3d

    union3d

    Joined:
    Dec 8, 2012
    Posts:
    15
    ok still debugging but heres a video to give a better example of what happening


    edit: not sure if this also helps but upon starting the game the persistent stuff for both player and items become DDOL and functions fine with collection and viewing items until you leave and come back so im wondering if its were there some sort of data separation or mishandling im not seeing
     
    Last edited: May 12, 2024
  13. union3d

    union3d

    Joined:
    Dec 8, 2012
    Posts:
    15
    I will rewrite this Debug.log today when I get home and see what happens thank you :)
     
  14. union3d

    union3d

    Joined:
    Dec 8, 2012
    Posts:
    15
    ok i just finish debugging in
    in the itemslot script when I pick up items before leaving the room the log fires off but if I leave and comeback it does as I've described its doesn't fire off
     
  15. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    8,381
    If you haven't identified the source of the problem, then your debugging process hasn't finished yet. Debugging involves finding the source of your problem, rather than just describing it. Use what you've found so far to keep digging.
     
    Ryiah, Kurt-Dekker and ArachnidAnimal like this.
  16. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    39,321
    Yes, exactly what Spiney says.

    Turn this statement:

    into this conversation in your head:

    "I expect when I leave and come back, mechanism X should cause the code to be running.

    However, mechanism X isn't happening so the code doesn't run.

    What is wrong with mechanism X?"


    Keep in mind "mechanism X" may be a complex series of steps, such as loading the scene, activating other pieces of code, etc., I don't know what all you're doing.

    But that is the mindset you need to debug.
     
    Ryiah and spiney199 like this.
  17. union3d

    union3d

    Joined:
    Dec 8, 2012
    Posts:
    15
    think i found the source of the problem game uses and eventbus
    if I'm correct on how events work as soon as I pick up and item

    pickupitem is called which invokes the onitempickup in the inventory

    now the question is why does this happen when the items that children DDOL


    "Inventory Controller"
    Code (CSharp):
    1. using System.Collections;
    2. using System.Collections.Generic;
    3. using UnityEngine;
    4. using TMPro;
    5. using UnityEngine.EventSystems;
    6. using FMODUnity;
    7.  
    8. public class InventorySelectController : MonoBehaviour
    9. {
    10.  
    11.  
    12.    
    13.    [SerializeField] private GameObject _inventoryView;
    14.    [SerializeField] private TMP_Text _itemNameText;
    15.    [SerializeField] private TMP_Text _itemDescriptionText;
    16.    [SerializeField] private EventReference OpenInventorySound;
    17.    [SerializeField] private EventReference CloseInventorySound;
    18.    [SerializeField] private List<ItemSlot> _slots;
    19.  
    20.    [SerializeField] private Playermovement _Playermovement;
    21.    [SerializeField] private ScreenFader _fader;
    22.  
    23.    private enum State
    24.    {
    25.    menuClosed,
    26.    menuOpen,
    27.    };
    28.  
    29.    private State _state;
    30.  
    31.  
    32.    private void OnEnable()
    33.    {
    34.     EventBus.Instance.onPickUpItem += OnItemPickedUp;
    35.    }
    36.  
    37.  
    38.    private void OnDisable()
    39.    {
    40.     EventBus.Instance.onPickUpItem -= OnItemPickedUp;
    41.    }
    42.  
    43.    private void OnItemPickedUp(ItemData itemData)
    44.    {
    45.     foreach (var slot in _slots)
    46.     {
    47.       if (slot.isEmpty())
    48.       {
    49.         slot.itemData = itemData;
    50.         //Debug.Log("slot has been filled");
    51.         break;
    52.       }
    53.     }
    54.    }
    55.  
    56.    public void OnSlotSelected(ItemSlot selectedSlot)
    57.    {
    58.      if (selectedSlot.itemData == null)
    59.      {
    60.      _itemNameText.ClearMesh();  
    61.      _itemDescriptionText.ClearMesh();
    62.    
    63.      return;
    64.      }
    65.       _itemNameText.SetText(selectedSlot.itemData.Name);
    66.       _itemDescriptionText.SetText(selectedSlot.itemData.Description[0]);
    67.    
    68.      if (selectedSlot == null)
    69.      {
    70.       Debug.Log("slot selected");
    71.      
    72.      }  
    73.    }
    74.  
    75. private void Update()
    76. {
    77. if (Input.GetKeyDown(KeyCode.Tab))
    78. {
    79.    if (_state == State.menuClosed)
    80.    {
    81.     EventBus.Instance.PauseGamePlay();
    82.    Debug.Log(_state);
    83.    _fader.FadeToBlack(0.3f, FadeFromMenuCallback);
    84.    _state = State.menuOpen;
    85.    }
    86.    else if (_state == State.menuOpen)
    87.    {
    88.     EventBus.Instance.ResumeGamePlay();
    89.   Debug.Log(_state);
    90.    _fader.FadeToBlack(0.3f, FadeToMenuCallback);
    91.    _state = State.menuClosed;
    92.    }
    93. }
    94.  
    95. }
    96. private void FadeFromMenuCallback()
    97. {
    98. AudioManager.instance.PlayOneShot(OpenInventorySound, this.transform.position);
    99. _inventoryView.SetActive(true);
    100. _fader.FadeFromBlack(0.3f, null);
    101. //Debug.Log(_state);
    102. }
    103. private void FadeToMenuCallback()
    104. {
    105. AudioManager.instance.PlayOneShot(OpenInventorySound, this.transform.position);
    106. _inventoryView.SetActive(false);
    107. _fader.FadeFromBlack(0.3f, null);
    108. //Debug.Log(_state);
    109. }
    110.  
    111.  
    112. }
    113.  

    "EventBus"
    Code (CSharp):
    1. using System;
    2. using System.Collections;
    3. using System.Collections.Generic;
    4. using UnityEngine;
    5.  
    6. public class EventBus : MonoBehaviour
    7. {
    8.     public static EventBus Instance {get; private set;}
    9.     public event Action onOpenInventory;
    10.     public event Action onCloseInventory;
    11.    
    12.     public event Action<ItemData> onPickUpItem;
    13.  
    14.     public event Action onGameplayePaused;
    15.     public event Action onGameplayeResumed;
    16.     public void OpenInventory()
    17.     {
    18.         onOpenInventory?.Invoke();
    19.     }
    20.     public void CloseInventory()
    21.     {
    22.         onCloseInventory?.Invoke();
    23.     }
    24.  
    25.     public void PickUpItem(ItemData itemData)
    26.     {
    27.        if(onPickUpItem == null)
    28.        Debug.LogError("nothing subscribed to onPickUpItem");
    29.        onPickUpItem?.Invoke(itemData);
    30.     }
    31.    
    32.   public void EquipItem(ItemData itemData)
    33.     {
    34.         //onEquipItem?.Invoke(itemData);
    35.     }
    36.  
    37.   public void StoreItem(ItemData itemData)
    38.     {
    39.         //onStoreItem?.Invoke(itemData);
    40.     }
    41. public void PauseGamePlay()
    42.     {
    43.        onGameplayePaused?.Invoke();
    44.     }
    45.  
    46.     public void ResumeGamePlay()
    47.     {
    48.         onGameplayeResumed?.Invoke();
    49.     }
    50.  
    51.  
    52.     // Update is called once per frame
    53.     private void Awake()
    54.     {
    55.         Instance = this;
    56.     }
    57. }
    58.  
     
  18. TeaJunkie

    TeaJunkie

    Joined:
    Nov 23, 2016
    Posts:
    24
    I'm not sure if im missing something here but i assume your DDoL scripts are in the scene you start with, do you check to see if you have duplicates which would happen when you leave the scene and return. In which case you would need to do a check and make sure only 1 version of these scripts is running at a time. I'm not seeing this in any of the scripts that you have posted so far.
     
    Ryiah likes this.
  19. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    39,321
    Building on what @TeaJunkie suggests above, your EventBus looks like a horribly defective singleton implementation.

    If you are expecting that to be DontDestroyOnLoad() then you absolutely CANNOT connect it to ANYTHING else that is not also marked DDoL. If you do so, then those other objects will go away while this object persists and refers to them, and now you have errors.

    And if you place this in a scene and reload that scene, your Awake() function will WIPE OUT the old instance reference and now you will have two of them. There is no lifetime our uniqueness control in your code whatsoever.

    Ideally, do not mark ANYTHING as DontDestroyOnLoad()

    If you absolutely must, then NEVER place it in any scene, EVER. It's that bad to do so, even if every single tutorial out there shows you to do that. Those tutorials all assume you never reload the scene, and this error happens all the time in here.

    Instead, use this pattern if you absolutely must have a DontDestroyOnLoad() object, and NEVER place it in a scene.

    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

    Alternately you could start one up with a
    RuntimeInitializeOnLoad
    attribute.
     
    Ryiah likes this.
  20. union3d

    union3d

    Joined:
    Dec 8, 2012
    Posts:
    15

    no the EventBus is connected to anything in scene if I'm reading your post correctly
    I'm gonna be honest I got most of this code from an incomplete tutorial series and I've been doing my best to foot it on my own from there

    but the event bus isn't in the scene whatso ever as far as I see it this is how the script sequence of events goes

    pickup.cs fires Pickupitem() =====> eventbus fires of Pickupitem which invokes onPickupitem which the inventory controller subscribes to

    honestly if it is the don't destroyonload that's causing problems I think I'm better off doing what I initially planned and code a save/load system for rooms

    I also don't know if this helps even more but this is the dontdestory script
    Code (CSharp):
    1. using System.Collections;
    2. using System.Collections.Generic;
    3. using UnityEngine;
    4.  
    5. public class DontDestroy : MonoBehaviour
    6. {
    7. private static GameObject[] persistentObjects = new GameObject[3];
    8. public int objectIndex;
    9.   void Awake()
    10.   {
    11.      if(persistentObjects[objectIndex]== null)
    12.      {
    13.      persistentObjects[objectIndex] = gameObject;
    14.      DontDestroyOnLoad(gameObject);
    15.      }
    16.  
    17.      else if (persistentObjects[objectIndex] != gameObject)
    18.      {
    19.        Destroy(gameObject);
    20.      }
    21.  
    22.    }
    23.  
    24. }
    25.  
     
  21. union3d

    union3d

    Joined:
    Dec 8, 2012
    Posts:
    15

    Edit: ok after messing around and tinkering after making some new rooms and puzzles I've come to find out that the error log fires off in the EventBus even if I remove the parent objects dontdestroyonload script

    the problem is as far as I can tell from the debugging and investigating I've done so far only happening after reloading rooms
     
  22. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    39,321
    Then what are all those events connected to? Because if those events are hooked to things in the scene and those things go away, bam, it's gone.

    Either way, as I pointed out, it's a defective singleton: there's nothing to prevent the subsequent loading and overwriting of the instance variable.

    Look, this lifetime control stuff is hard to get correct under the BEST of circumstances. If you haven't yet fully completed a few games in Unity and understand 100% of what that tutorial has taught you, your chances of making this work satisfactory area rather low.

    Your only guard against this stuff malfunctioning is to debug, debug, debug, debug. You HAVE to understand what happens to things when they are loaded / unloaded in a scene, and I can't make you understand that by typing here.
     
    Ryiah likes this.
  23. union3d

    union3d

    Joined:
    Dec 8, 2012
    Posts:
    15
    I looked at the first link you posted
    I came up with this and it seems to have solved the issue :)

    Code (CSharp):
    1.  private void Awake() /// persistent between rooms
    2.     {  
    3.         if (Instance != null && Instance != this)
    4.         {
    5.             Destroy(gameObject);
    6.             return;
    7.         }
    8.         Instance = this;
    9.         DontDestroyOnLoad(gameObject);
    10.     }
    11. }
     
  24. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    8,381
    EventBus could honestly just be a static class, and the events/delegates and methods static. There's no reason for it to be a monobehaviour.
     
    Ryiah likes this.