0

I need to combine the following scripts into one script containing two functions, one that instantiates the next prefab in the array and the other that instantiates the previous prefab in the array (it is a virtual tour app). Both should also destroy the current prefab. I could then call the functions via event triggers.

I have this code for the "next Prefab" script.

public GameObject[] Spheres;
    int currentIndex = 0;
    GameObject currentObject;
    public Camera MainCamera;

    void OnMouseDown()
    {
        if (Input.GetMouseButtonDown(0))
            if (gameObject.tag == "ArrowNEXT")
        {
            Vector3 rayOrigin = MainCamera.ViewportToWorldPoint(new Vector3(0.1f, 0.1f, 0));
            RaycastHit hit;

            if (Physics.Raycast(rayOrigin, MainCamera.transform.forward, out hit))
            {
                if (hit.collider != null)
                {
                    {
                            Destroy(currentObject);
                            currentIndex++;
                            if (currentIndex > Spheres.Length - 1) currentIndex = 0;
                            currentObject = Instantiate(Spheres[currentIndex]);
                    }
                }
            }


        }

    }

I need to combine it with the following:

using UnityEngine;


public class RayCastPrevFIX: MonoBehaviour
{
    public GameObject[] Spheres;
    int currentIndex = 0;
    GameObject currentObject;
    public Camera MainCamera;

    void OnMouseDown()
    {
        if (Input.GetMouseButtonDown(0))
            if (gameObject.tag == "ArrowPREV")
            {
            Vector3 rayOrigin = MainCamera.ViewportToWorldPoint(new Vector3(0.1f, 0.1f, 0));
            RaycastHit hit;

            if (Physics.Raycast(rayOrigin, MainCamera.transform.forward, out hit))
            {
                if (hit.collider != null)
                {
                    {
                            Destroy(currentObject);
                            currentIndex--;
                            if (currentIndex < 0) currentIndex = Spheres.Length - 1;
                            currentObject = Instantiate(Spheres[currentIndex]);
                    }
                }
            }


        }

    }
}

How would I go about this? Any help is greatly appreciated.I have this so far:

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class SphereSwap : MonoBehaviour
{ 
   public GameObject[] Spheres;
   int currentIndex = 0;
   GameObject currentObject;
   public Camera MainCamera;

   void Next()
   {
       Destroy(currentObject);
       currentIndex++;
       if (currentIndex > Spheres.Length - 1) currentIndex = 0;
       currentObject = Instantiate(Spheres[currentIndex]);
   }

   void Previous()
   {
       Destroy(currentObject);
       currentIndex--;
       if (currentIndex < 0) currentIndex = Spheres.Length - 1;
       currentObject = Instantiate(Spheres[currentIndex]);
   }
}
JWolf
  • 3
  • 3

1 Answers1

0

You're on the right path, but you could shorten the code as well as use one function for both cases to avoid boilerplate:

using UnityEngine;
public class SphereSwap : MonoBehaviour
{
    public GameObject[] Spheres;
    int currentIndex = 0;
    GameObject currentObject;
    public Camera MainCamera;

    const string ArrowPrevTag = "ArrowPREV";
    const string ArrowNextTag = "ArrowNEXT";


    private void HandleClick(bool next)
    {
        if(Spheres == null || Spheres.Length == 0)
        {
            Debug.Log($"Spheres list is empty, nothing to swap.");
            return;
        }

        Vector3 rayOrigin = MainCamera.ViewportToWorldPoint(new Vector3(0.1f, 0.1f, 0));
        RaycastHit hit;
        if (Physics.Raycast(rayOrigin, MainCamera.transform.forward, out hit))
        {
            if (hit.collider != null)
            {
                // destroy current sphere.
                Destroy(currentObject);
                // go next or previous.
                currentIndex += next ? 1 : -1;

                // circular clamp if overflow
                if (currentIndex < 0)
                    currentIndex = Spheres.Length - 1;
                else if (currentIndex >= Spheres.Length)
                    currentIndex = 0;
                // finally do instantiate.
                currentObject = Instantiate(Spheres[currentIndex]);
            }
        }

    }
    private void OnMouseDown()
    {
        if (Input.GetMouseButtonDown(0))
        {
            // 'CompareTag' is more efficient than gameobject.tag == "sometag"
            if (gameObject.CompareTag(ArrowNextTag))
                HandleClick(true);
            else if (gameObject.CompareTag(ArrowPrevTag))
                HandleClick(false);
        }
    }
}
ChoopTwisk
  • 1,296
  • 7
  • 13
  • Oh wow, I hadn't thought of that. I'm a game artist, so I am learning as I go. Two questions: 1.) I am getting an "index outside of the array" error highlighting the currentObject = Instantiate(Spheres[currentIndex]); line 32. 2.) Is the comparetag more efficient as my way works similar to a find (where it has to track down the tag?) It's nice you avoided using the event trigger, which is good. – JWolf Oct 14 '19 at 19:19
  • Ahh the constant value is more efficient due to the fact it is an unchanged string, so it doesn't have to be "checked" in memory management, right? – JWolf Oct 14 '19 at 19:27
  • @JWolf I added an extra check, that checks if your 'Spheres' list is empty before swapping, thus preventing the error 'index outside of the array', do you still get it ? About constants, 2 strings don't have that much impact on memory in today's computing, its just a coding convention, preventing what we call magic strings, so if you were to use the tag somewhere else, you don't need to care about its spelling, just use the variable, and when you need to edit it, you can edit it only once where it was assigned. – ChoopTwisk Oct 14 '19 at 19:37
  • Yeah I just caught that ;) As usual when an artist is learning programming, the bug generator is between the chair and the keyboard. Thank You. If I attach this script to both a forward and reverse arrow object in the scene how would it know which object to destroy (between the two scripts running)? Interesting about the tags, it's almost like creating a reference that refers to the original code so you don't have to write it over and over. – JWolf Oct 14 '19 at 19:57
  • Using two of these scripts attached to different objects (forward and reverse arrows) is causing some issues. Clicking the reverse arrow object after the forward arrow object does not destroy the last sphere instantiated by the forward arrow. The reverse arrow instantiates a new sphere and now there are two spheres in the scene. Is there a way to keep track of these spheres by array index, allowing the delete function for each arrow script to delete all spheres in the scene when instantiating a new one? – JWolf Oct 15 '19 at 17:22