From 02e0ead3c2232b726498738942339d5df911282e Mon Sep 17 00:00:00 2001
From: Devin Braune <devin@devinbraune.de>
Date: Tue, 2 Feb 2021 01:31:15 +0100
Subject: [PATCH] Improve self-managed observers, remove when inactive

---
 Assets/Scripts/Shop/Model/BuyModel.cs         |  2 +-
 Assets/Scripts/Shop/View/ShopView.cs          | 14 ++++++++++++--
 Assets/Scripts/Shop/View/ShopViewGrid.cs      |  4 ++--
 Assets/Scripts/Shop/View/ViewItemContainer.cs |  8 +++++++-
 4 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/Assets/Scripts/Shop/Model/BuyModel.cs b/Assets/Scripts/Shop/Model/BuyModel.cs
index 5279035..9096317 100644
--- a/Assets/Scripts/Shop/Model/BuyModel.cs
+++ b/Assets/Scripts/Shop/Model/BuyModel.cs
@@ -20,7 +20,7 @@ public class BuyModel : ShopModel
     {
         inventory.RemoveItemByIndex(selectedItemIndex);    // Before removing the item from the model's actual inventory
         OnRemove(GetSelectedItem());    // If there's a view subscribed, this will probably remove the item from it
-        SelectItemByIndex(selectedItemIndex > inventory.GetItemCount() ? --selectedItemIndex : selectedItemIndex);
+        SelectItemByIndex(selectedItemIndex >= inventory.GetItemCount() ? --selectedItemIndex : selectedItemIndex);
     }
 
 }
diff --git a/Assets/Scripts/Shop/View/ShopView.cs b/Assets/Scripts/Shop/View/ShopView.cs
index e06b4d5..3ee1061 100644
--- a/Assets/Scripts/Shop/View/ShopView.cs
+++ b/Assets/Scripts/Shop/View/ShopView.cs
@@ -31,16 +31,26 @@ public abstract class ShopView : MonoBehaviour, IShopModelObserver<Item>
     protected ShopModel other; // Other model in MVC pattern (our own inventory)
     private ShopController shopController; //Controller in MVC pattern
 
-    protected virtual void Start()
+    // Set up the view's necessary stuff before the view is enabled
+    protected virtual void Awake()
     {
         model = new BuyModel(2f, 16, 500); //Right now use magic values to set up the shop
         shopController = gameObject.AddComponent<MouseController>().Initialize(model);//Set the default controller to be the mouse controller
         SetupItemIconView(); //Setup the grid view's properties
-        PopulateItemIconView(); //Display items
         InitializeButtons(); //Connect the buttons to the controller
         //model.Subscribe(this);
     }
 
+    private void OnEnable()
+    {
+        PopulateItemIconView(); //Display items
+    }
+
+    private void OnDisable()
+    {
+        ClearIconView();
+    }
+
     //------------------------------------------------------------------------------------------------------------------------
     //                                                  SetupItemIconView()
     //------------------------------------------------------------------------------------------------------------------------        
diff --git a/Assets/Scripts/Shop/View/ShopViewGrid.cs b/Assets/Scripts/Shop/View/ShopViewGrid.cs
index 0000b3d..3d865ff 100644
--- a/Assets/Scripts/Shop/View/ShopViewGrid.cs
+++ b/Assets/Scripts/Shop/View/ShopViewGrid.cs
@@ -15,12 +15,12 @@ public class ShopViewGrid : ShopView
     //views.
     
     // Start is called before the first frame update
-    protected override void Start()
+    protected override void Awake()
     {
         viewConfig = Resources.Load<ViewConfig>("ViewConfig");//Load the ViewConfig scriptable object from the Resources folder
         Debug.Assert(viewConfig != null);
         Debug.Assert(_gridLayoutGroup != null);
-        base.Start();
+        base.Awake();
         print("ShopView Grid Initialised");
     }
 
diff --git a/Assets/Scripts/Shop/View/ViewItemContainer.cs b/Assets/Scripts/Shop/View/ViewItemContainer.cs
index 2806750..4397f7d 100644
--- a/Assets/Scripts/Shop/View/ViewItemContainer.cs
+++ b/Assets/Scripts/Shop/View/ViewItemContainer.cs
@@ -82,7 +82,7 @@ public class ViewItemContainer : MonoBehaviour, IItemContainer, IShopModelObserv
     public void OnRemoved(Item item)
     {
         if (item != Item) return;    // Well we only want this if the item removed from the model is actually ours!
-        _unsubscriber.Dispose();    // Careful! If we don't make this thing aware that it manages itself, and we forget to manage it, we have a memory leak
+        IsSelected = false;          // Kind of pointless, but won't hurt
         Destroy(gameObject);
     }
 
@@ -90,4 +90,10 @@ public class ViewItemContainer : MonoBehaviour, IItemContainer, IShopModelObserv
     {
         throw new NotImplementedException();
     }
+    
+    // The reason we do this in OnDestroy() is so we don't have a memory leak when this gets removed externally somehow
+    private void OnDestroy()
+    {
+        _unsubscriber?.Dispose();    // Careful! If we don't make this thing aware that it manages itself, and we forget to manage it, we have a memory leak
+    }
 }