Problem with switching between functions

so in this code

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

public class Lumos : MonoBehaviour
{
    public GameObject lumosLight;
    public GameObject Incendio;

    void Start()
    {
        Debug.Log ("literally nothing is active");
        nothing ();
    }
   
        void nothing()
        {
            Debug.Log ("NOTHING is active");
            if (Input.GetButtonDown ("scroll1"))
            {
                lumosSpell ();
            }

            if (Input.GetButtonDown("scroll2"))
            {
                incendioSpell ();
            }

        }


        void incendioSpell()
        {
            Debug.Log ("INCENDIO is active");
            if (Input.GetButtonDown ("off"))
            {
                Incendio.SetActive (false);
            }

            if (Input.GetButtonDown ("on"))
            {
                Incendio.SetActive (true);
            }

            if (Input.GetButtonDown ("scroll1"))
            {
                nothing ();
            }

            if (Input.GetButtonDown ("scroll2"))
            {
                lumosSpell ();
            }

        }

        void lumosSpell()
        {
            Debug.Log ("LUMOS is active");
            if (Input.GetButtonDown ("off"))
            {
                lumosLight.SetActive (false);
            }

            if (Input.GetButtonDown ("on"))
            {
                lumosLight.SetActive (true);
            }

            if (Input.GetButtonDown ("scroll1"))
            {
                incendioSpell ();
            }

            if (Input.GetButtonDown ("scroll2"))
            {
                nothing ();
            }
    }
}

switching between spells using scroll1 and scroll 2 (bound to e and q, yes they are bound correctly) doesnt work, what I want is like a simple system to scroll trough spells, and you start at spell ‘‘nothing’’
What did I screw up this time?

Thanks in advance,
Frisout

Input has to be tied to Update or some sort of Update type system where it’s checking every frame.

Your input calls don’t run because they never get checked. I can’t say that your stuff will work correctly as other parts still look odd, but for the input part, you will generally need to do something like.

void Update()
{
   if(Input.GetButtonDown("off")
      lumosLight.SetActive(false);
}

Just as a basic idea. Input is one of the things you do want in Update.

it looks like there is some a issue with the logic. This script looks like you have to push two buttons at the same time,
input scroll1 and then on or off

using System.Collections;
using System.Collections.Generic;
using UnityEngine;
 
public class Lumos : MonoBehaviour
{
    public GameObject lumosLight;
    public GameObject Incendio;
 
    void Start()
    {
        Debug.Log ("literally nothing is active");
        //nothing ();
    }

        void Update()
       {
            if (Input.GetButtonDown ("scroll1")) //this is the first button that is being held down
            {
                lumosSpell ();
            }
 
            if (Input.GetButtonDown("scroll2"))  //this is the first button that is being held down
            {
                incendioSpell ();
            }
        }
   
        void nothing() // not needed
        {
            Debug.Log ("NOTHING is active");
            if (Input.GetButtonDown ("scroll1"))
            {
                lumosSpell ();
            }
 
            if (Input.GetButtonDown("scroll2"))
            {
                incendioSpell ();
            }
 
        }
 
 
        void incendioSpell()
        {
            Debug.Log ("INCENDIO is active");
            if (Input.GetButtonDown ("off")) //this is the second button that is being held down
            {
                Incendio.SetActive (false);
            }
 
            if (Input.GetButtonDown ("on")) //this is the second button that is being held down
            {
                Incendio.SetActive (true);
            }
 
            if (Input.GetButtonDown ("scroll1")) // you are still holding this button down, so it will do it I would remove these
            {
                //nothing ();
            }
 
            if (Input.GetButtonDown ("scroll2")) // you are still holding this button down, so it will do it I would remove these
            {
                //lumosSpell ();
            }
 
        }
 
        void lumosSpell()
        {
            Debug.Log ("LUMOS is active");
            if (Input.GetButtonDown ("off")) //this is the second button that is being held down
            {
                lumosLight.SetActive (false);
            }
 
            if (Input.GetButtonDown ("on")) //this is the second button that is being held down
            {
                lumosLight.SetActive (true);
            }
 
            if (Input.GetButtonDown ("scroll1")) // you are still holding this button down, so it will do it. I would remove these
            {
                //incendioSpell ();
            }
 
            if (Input.GetButtonDown ("scroll2")) // you are still holding this button down, so it will do it I would remove these
            {
                //nothing ();
            }
    }
}

Yes, nothing is needed since I want to start with no spell active.
the off / on are the spells themselves, don’t bother about em.

I made an UI that says ‘‘Current active spell’’ and it never gets further than ‘‘None’’ so that means q and e (scroll1 and scroll2) ain’t working at all, possibly due to what’s said here. So it’s nothing with having to press 2 buttons.

I already have a script that makes 1 spell, Lumos work but that spell is always active without an option to switch, I thought I’d test something out and do this but apparently it’s not getting updates, how to fix that? can I put a void Update after a void Nothing for example?

if i’m understanding what you’re trying to do. this is one way to do it.

using System.Collections;
using System.Collections.Generic;
using UnityEngine;
 
public class Lumos : MonoBehaviour
{
    public GameObject lumosLight;
    public GameObject Incendio;
    private bool lumosActive = false;
    private bool IncendioActive = false;
 
    void Start()
    {
        Debug.Log ("literally nothing is active");
    }
 
        void Update()
       {
            if (Input.GetButtonDown ("scroll1"))
            {
                disableUi();
                lumosActive = true;
            }
 
            if (Input.GetButtonDown("scroll2"))  
            {
                disableUi();
                IncendioActive = true;
            }

            if(IncendioActive == true)
            {
                incendioSpell();
            }else if(lumosActive == true)
            {
                lumosSpell();
            }
        }
 
 
        void incendioSpell()
        {
            Debug.Log ("INCENDIO is active");
            if (Input.GetButtonDown ("off"))
            {
                Incendio.SetActive (false);
            }
 
            if (Input.GetButtonDown ("on"))
            {
                Incendio.SetActive (true);
            }
 
        }
 
        void lumosSpell()
        {
            Debug.Log ("LUMOS is active");

            if (Input.GetButtonDown ("off"))
            {
                lumosLight.SetActive (false);
            }
 
            if (Input.GetButtonDown ("on"))
            {
                lumosLight.SetActive (true);
            }
 
        }

        void disableUi()
        {
            lumosLight.SetActive (false);
            Incendio.SetActive (false);
            lumosActive = false;
            IncendioActive = false;
        }
}

what we got now is q activating 1 spell, and e activating another spell. this way I’ll never be able to add more spells unless I assign more buttons. What I want is a scrolling system with a UI I work out later, but pressing q scrolls/cycles to the left, and pressing e scrolls/cycles to the right trough the list of spells.

I am not asking you to write a whole script for me, I won’t learn from that, just a few hints are fine

so you have an array or a list of all the spells that get cycled through? I didn’t see anything about cycling through a bunch of spells in the first post. all that script did was enable/disable a gameObject.
I guess what i’m saying is, that I don’t have full picture of what you’re trying to do. So I was trying to make sense of the script I was given.

You’ll have to have a list of spells and keep track of which is active through that. And then create the scrolling effect of simply you hit e and it adds +1 to the index and moves to the next spell. If you hit the top of your list, then you have to reset that index to 0 so you start at the beginning of your list again. And same if you hit 0 and hit q to -1 from your index.

The index just simply keeps track of where you are in the list to determine what spell to turn on or off (you always turn off a spell, modify the index, then turn on the spell).

@johne5 I think that is what they are asking. The current system is setup to do a single spell per button and they want a scrolling list so they don’t have to add a button per spell.

You don’t need to care about the object enabling / disabling, that’s all on my side and is fine.

Just the ‘‘scroll1’’ ‘‘scroll2’’ and the differn’t void’s I made. when I press q or e I want to switch to a differnt void which is containing what you don’t need to care about.
And you didn’t see a list since I am doing it all manualy, but in such a way that it does cycle. I know it’s not the most efficient way but it’s something I understand as beginning scripter.

Here is a version of the script with only what you need to care about

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


public class Lumos : MonoBehaviour
{
   public GameObject lumosLight;
   public GameObject Incendio;


    void Start()
    {
        Debug.Log ("literally nothing is active");
        nothing ();
    }
   
        void nothing() //spell 1
        {
            Debug.Log ("NOTHING is active");
            if (Input.GetButtonDown ("scroll1"))
            {
                lumosSpell (); //goes to lumos, the spell to the left
            }


            if (Input.GetButtonDown("scroll2"))
            {
                incendioSpell (); //goes to incendio, the spell on the right
            }


        }


        void incendioSpell() //spell 3
        {
            Debug.Log ("INCENDIO is active");


            if (Input.GetButtonDown ("scroll1")) //goes to ''no active spell'' the spell on the left
            {
                nothing ();
            }


            if (Input.GetButtonDown ("scroll2")) //goes to lumos, the spell on the right
            {
                lumosSpell ();
            }


        }


        void lumosSpell() //spell 2
        {
            Debug.Log ("LUMOS is active");


            if (Input.GetButtonDown ("scroll1")) //goes to incendio, the spell on the left
            {
                incendioSpell ();
            }


            if (Input.GetButtonDown ("scroll2")) //goes to nothing, the spell on the left
            {
                nothing ();
            }
    }
}

how about this idea

using System.Collections;
using System.Collections.Generic;
using UnityEngine;
 
public class Lumos : MonoBehaviour
{
    public GameObject[] spellsList;
    private int activeSpell = 0;

 
    void Start()
    {
        Debug.Log ("literally nothing is active");
        switchSpell(spellsList[activeSpell]); // activate spell 0
    }
 
        void Update()
       {
            if (Input.GetButtonDown ("scroll1"))
            {
                activeSpell++;
                switchSpell(spellsList[activeSpell]);
            }
 
            if (Input.GetButtonDown("scroll2")) 
            {
                activeSpell--;
                switchSpell(spellsList[activeSpell]);
            }
 

        }
 
        void switchSpell(GameObject spell)
        {
            if (Input.GetButtonDown ("off"))
            {
                disableUi();
                spell.SetActive (false);
            }
 
            if (Input.GetButtonDown ("on"))
            {
                disableUi();
                spell.SetActive (true);
            }
        }

 
        }
 
        void disableUi()
        {
            lumosLight.SetActive (false);
            Incendio.SetActive (false);
        }
}

The array index will be out of range when you go too far. but you can fix that by setting it to 0

Yah, he didn’t say that

Honestly, while I understand what you think you want, you’ll probably not need a single method for each spell. You simply need a list of spells and a way to cycle up and down through them. Unless you are asking something completely different. And it appears @johne5 posted something similar to what I was thinking you were wanting.

Yeah, and maybe I’m confused as to what they want also.

hhm, okay but how will my spell list be a game object? an object with a script containing it all or…?

I was replacing
public GameObject lumosLight; //spell 1
public GameObject Incendio; //spell 2

with
public GameObject[ ] spellsList; // you can put multiple spells in this, via unity inspector

Alright, I give it a go after I had some lunch.
Thanks for the help so far guys :smile:

‘‘Array index is out of range’’
I added the game object for Lumos (a point light) and the object for incendio (particle system) to the list which now says Size: 2, but whenever pressing q or e it says that error :confused:

    using System.Collections;
    using System.Collections.Generic;
    using UnityEngine;
   
    public class Lumos : MonoBehaviour
    {
        public GameObject[] spellsList;
        private int activeSpell = 0;
   
   
        void Start()
        {
            Debug.Log ("literally nothing is active");
            switchSpell(spellsList[activeSpell]); // activate spell 0
        }
   
            void Update()
           {
                if (Input.GetButtonDown ("scroll1"))
                {
                    disableUi();
                    if(activeSpell == spellsList.Length -1)
                       activeSpell = 0;
                    else
                       activeSpell++;
                    switchSpell(spellsList[activeSpell]);
                }
   
                if (Input.GetButtonDown("scroll2"))
                {
                    disableUi();
                    if(activeSpell == 0)
                       activeSpell = spellsList.Length -1;
                    else
                       activeSpell--;
                    switchSpell(spellsList[activeSpell]);
                }
   
   
            }
   
            void switchSpell(GameObject spell)
            {
                if (Input.GetButtonDown ("off"))
                {
                    spell.SetActive (false);
                }
   
                if (Input.GetButtonDown ("on"))
                {
                    spell.SetActive (true);
                }
            }
   
   
            }
   
            void disableUi()
            {
                spellsList(activeSpell).SetActive(false);
            }
    }

Modified @johne5 code a bit.

I will add that the index out of range error is pretty common and an easy to understand fix. So might be good to brush up on tutorials covering collections (List and arrays are some of the more common ones) so you understand them.

@Brathnann Thanks for the code update, I know that the null ref would be an issue. I wasn’t even sure if this was what he was after, so I didn’t put in the effort to prevent them.