UPPER_BOUNDS = 3
There are 3 models loaded, indexed 0,1,2
Whenever loadModel() or unloadModel() is called and count is greater than 2 you will get “index out of range error”.
Lines 53, 54:
count = UPPER_BOUNDS; //(i.e. count = 3)
unloadModel();
=> Count = 3, unloadModel() but no model exists at index 3 => error
Lines 71, 72:
If count < 3 (UPPER_BOUNDS) this will execute. So, when count = 2, it will execute but will increase to 3
count++; //increases count to 3
loadModel(); //error no model at index 3
Make sure that loadModel() and unloadModel() never get called with count greater than 2, or the maximum number of models in clones.
The following code for the FileLoader script should do what you want. It’s quite different -and much simpler- from your original code but should do the job fine. I provided plenty of comments in each part of the code, so you can understand the changes I made, as well as the functionality. A major change was removing the clones List, and replacing it with a currentModel GameObject, which also makes very easy referencing this from the GameController. In effect, a ‘currentIndex’ replaced ‘count’, and this always points to the element in the models List that is loaded in currentModel, and made available to the GameController through a readonly property.
One important thing that I want to say is that although this code provides the functionality you specify, it will not perform so well. The reason for this is that as the user goes forwards and backwards in the list of models, the Garbage Collector will be called all the time to clear all objects that are destroyed (or lose their reference). When the Garbage Collector kicks in, the performance of the application will drop, until the Garbage Collector finishes its job. A better way to implement this is to have all models inside the scene and disabled. Then instead of Instantiating and Destroying, you just disable the previous and enable the next model. Such a solution would perform much better because there is no Garbage Collector involved. To implement this, create an Empty GameObject in the scene and add to it -as children- all the models you have in Assets/Models. Then disable all of the children.
Then, all the GameObjects inside the models List will be instantiated GameObjects (and disabled), rather than prefabs. So, Inside the loadModel() method you don’t need to use Instantiate(), but currentModel.SetActive(false) to disable and currentModel.SetActive(true) to enable. I have included the relevant code for these changes, but I have commented it out, so you decide what you want.
So here is the code:
using UnityEngine;
using System.Collections;
using System.Collections.Generic;
public class FileLoader : MonoBehaviour {
//It seems that the only use of the following two fields is to get a reference to the Main Camera (SceneCamera)
//and then assing it to the GameController's TouchRotation (assigned in Start())
//Since FileLoader has no other use of either SceneCamera or TouchRotation, it is better to remove these fields from FileLoader
//and have a public Camera field directly in TouchRotation
public Camera SceneCamera; //should be moved to TouchRotation script
public TouchRotation tRotation; //remove
public List<GameObject> models = new List<GameObject>(); // three game objects loaded in unity
//You don't need this!
//You just need ONE currentModel field where GameObjects from the models List are loaded
//as you move to the next or previous element in Models
//so, you don't even need an Unload method. Every time you move in "models" the relative model will be loaded
//as the currentModel
//private List<GameObject> clones = new List<GameObject>(); // Remove!!!
//private GameObject temp; //no need for this, so remove
//Renamed UPPER_BOUNDS to highestIndex and made private
//This will always point to the highest index in the models List,
//because the number of elements in the models List determines the highest possible index
//for the availability of an element to be loaded as currentModel
private int highestIndex;
//This is the index of the element in models List that has already been loaded to currentModel
private int currentIndex = -1; // initalize to -1, because no model loaded at start
//Backing field for CurrentModel read-only property,
//so that GameController can always read the CurrentModel through this property
private GameObject currentModel;
//GameController must have a reference to the FileLoader script in order to access this property
//i.e. public FileLoader in GameController (probably in TouchRotation script)
//then drag-n-drop the FileManager GameObject from the Hierarchy
//to the FileLoader field in the Inspector of GameController
public GameObject CurrentModel
{
get { return currentModel; }
}
// Use this for initialization
void Start()
{
ObjReader.use.autoCenterOnOrigin = true; //it seems this doesn't belong here... maybe moved to TouchRotation?
TouchRotation tRotation = this.gameObject.AddComponent<TouchRotation>(); //remove
tRotation.camera = this.camera; //remove
if (models.Count > 0)
{
//Set the highest index possible, which depends on the number of models available in the models List
highestIndex = models.Count - 1;
//Load the first model, so the user has something to see when the app starts
//without having to click on any buttons
currentModel = Instantiate(models[0]) as GameObject;
////If you decide to populate the models List with instantiated GameObjects in the scene (in a disabled state),
////rather than prefabs from Assets\Models, then replace the above line currentModel = Instantiate(models[0]) as GameObject;
////with the following:
//currentModel = models[0];
//currentModel.SetActive(true);
currentIndex = 0;
}
else
{
Debug.LogError("No Models found! Please add at least one model in the Models List.");
Debug.Break();
}
}
//Instantiates the GameObject at index position "index" in the models List
//and references it with the currentModel variable
public void loadModel(int index)
{
//make sure that when index is higher than highestIndex
//it will be reset to 0 (according to specification)
//
//Since Previous() is the only point where currentIndex is decremented
//but it will never make a loadModel() call unless currentIndex > 0,
//the following condition index < 0 will always be FALSE.
//So condition might appear redundant at this point, and may be removed.
//However, it was included as a safety measure, in case of future code changes
if (index > highestIndex || index < 0)
{
index = 0;
}
//Previous GameObject instance referenced by currentModel will be Garbage Collected
currentModel = Instantiate(models[index]) as GameObject;
////If you decide to populate the models List with instantiated GameObjects in the scene (in a disabled state),
////rather than prefabs from Assets\Models, then replace the above line currentModel = Instantiate(models[index]) as GameObject;
////with the following:
////
////Disable current model - no need to check for null, since initialized in Start()
//currentModel.SetActive(false);
//currentModel = models[index]; //reference new model
//currentModel.SetActive(true); //activate new model
currentIndex = index;
}
//No need for an unloadModel() method
//Just assign a new model from the model List to the currentModel field
//
//public void unloadModel()
//{
// Destroy(clones[currentIndex]);
//}
//Renamed "incrementCounter" to Next because it's more descriptive of what it does
//e.g. when the counter is at the highest number and then it goes to 0, it's actually decrementing the counter...
//
//Load the Next model
public void Next()
{
//Since loadModel(0) is called in Start() this will always be TRUE
//So the if{} statement might appear redundant at this point, and may be removed.
//However, it was included as a safety measure, in case of future code changes
if (currentIndex >= 0)
{
//after the model at the highestIndex has been previously loaded in currentModel
//the parameter for the following loadModel() call will equal to highestIndex + 1
//but it will be reset to 0 inside the loadModel() method,
//in which case the first model (at index 0) will be loaded.
//Therefore, when "Next" is called, it will call loadModel(),
//with its parameter ranging from 0 to highestIndex + 1
loadModel(currentIndex + 1);
}
}
//Renamed "decrementCounter" to Previous because it's more descriptive of what it does
public void Previous()
{
//if Next() has not be called at least once after the first element in the Models List is loaded
//and currentIndex increased to a value greater than 0, calling the Previous() method will not do anything
//so, loadModel() will never be called with a parameter that is smaller than 0.
//Therefore, when "Previous" is called, it will call loadModel(),
//with its parameter ranging from 0 to highestIndex - 1
if (currentIndex > 0)
{
loadModel(currentIndex - 1);
}
}
}