Need guidance simplifying Code with lots of repetitive if-statements

Unity Engine
, ,
1

Hi, I’m still new to programming and although I have code that works now, it’s very repetitive. I’m certain there are ways to simplify this, but I don’t know how. It would be a great learning opportunity if you could give me some tips on how to make this code simpler on the eyes and my limited hardware.

The main idea is to have static images that get swapped out as the player clicks on a collider. Basically movement of the player through a series of static images. The colliders connect the screens to one another and they all have their own tag.

Thanks everyone in advance!

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

public class GameManager : MonoBehaviour
{
    public GameObject mainScreen;
    public GameObject centerScreen;
    public GameObject doorScreen;
    public GameObject bedScreen;
    public GameObject bedTopScreen;
    public GameObject bedBottomScreen;
    public GameObject tableScreen;
    public GameObject tableTopScreen;
    public GameObject tableBottomScreen;
    public GameObject lampScreen;
    public GameObject leftScreen;
    public GameObject topShelfScreen;
    public GameObject bottomShelfScreen;
    public GameObject drawerMainScreen;
    public GameObject safeMainScreen;

    // Start is called before the first frame update
    void Start()
    {
        mainScreen.SetActive(true);
        centerScreen.SetActive(false);
        doorScreen.SetActive(false);
        bedScreen.SetActive(false);
        bedTopScreen.SetActive(false);
        bedBottomScreen.SetActive(false);
        tableScreen.SetActive(false);
        tableTopScreen.SetActive(false);
        tableBottomScreen.SetActive(false);
        lampScreen.SetActive(false);
        leftScreen.SetActive(false);
        topShelfScreen.SetActive(false);
        bottomShelfScreen.SetActive(false);
        drawerMainScreen.SetActive(false);
        safeMainScreen.SetActive(false);
    }

    // Update is called once per frame
    void Update()
    {
        if (Input.GetMouseButtonDown(0))
        {
            // print("Click detected");
            Vector2 mousePos = Camera.main.ScreenToWorldPoint(Input.mousePosition);
            RaycastHit2D hit = Physics2D.Raycast(mousePos, Vector2.zero);

            if (hit.collider != null)
            {
                Debug.Log("Collider was hit (" + hit.collider.gameObject.tag + ")");
                if (hit.collider.gameObject.tag == "ToCenter")
                {
                    mainScreen.SetActive(false);
                    centerScreen.SetActive(true);
                    doorScreen.SetActive(false);
                    bedScreen.SetActive(false);
                    tableScreen.SetActive(false);
                    lampScreen.SetActive(false);
                    leftScreen.SetActive(false);
                }
                if (hit.collider.gameObject.tag == "BacktoMain")
                {
                    mainScreen.SetActive(true);
                    centerScreen.SetActive(false);
                    lampScreen.SetActive(false);
                    leftScreen.SetActive(false);
                    safeMainScreen.SetActive(false);
                }
                if (hit.collider.gameObject.tag == "ToDoor")
                {
                    mainScreen.SetActive(false);
                    centerScreen.SetActive(false);
                    doorScreen.SetActive(true);
                    bedScreen.SetActive(false);
                    tableScreen.SetActive(false);
                }
                if (hit.collider.gameObject.tag == "ToBed")
                {
                    mainScreen.SetActive(false);
                    centerScreen.SetActive(false);
                    doorScreen.SetActive(false);
                    bedScreen.SetActive(true);
                    bedTopScreen.SetActive(false);
                    bedBottomScreen.SetActive(false);
                    safeMainScreen.SetActive(false);
                }
                if (hit.collider.gameObject.tag == "ToTopBed")
                {
                    bedScreen.SetActive(false);
                    bedTopScreen.SetActive(true);
                    bedBottomScreen.SetActive(false);
                }
                if (hit.collider.gameObject.tag == "ToBottomBed")
                {
                    bedScreen.SetActive(false);
                    bedTopScreen.SetActive(false);
                    bedBottomScreen.SetActive(true);
                }
                if (hit.collider.gameObject.tag == "ToTable")
                {
                    mainScreen.SetActive(false);
                    centerScreen.SetActive(false);
                    doorScreen.SetActive(false);
                    tableScreen.SetActive(true);
                    tableTopScreen.SetActive(false);
                    tableBottomScreen.SetActive(false);
                }
                if (hit.collider.gameObject.tag == "ToTopTable")
                {
                    tableScreen.SetActive(false);
                    tableTopScreen.SetActive(true);
                    tableBottomScreen.SetActive(false);
                }
                if (hit.collider.gameObject.tag == "ToBottomTable")
                {
                    tableScreen.SetActive(false);
                    tableTopScreen.SetActive(false);
                    tableBottomScreen.SetActive(true);
                }
                if (hit.collider.gameObject.tag == "ToLamp")
                {
                    mainScreen.SetActive(false);
                    centerScreen.SetActive(false);
                    lampScreen.SetActive(true);
                }
                if (hit.collider.gameObject.tag == "ToLeft")
                {
                    mainScreen.SetActive(false);
                    centerScreen.SetActive(false);
                    leftScreen.SetActive(true);
                    topShelfScreen.SetActive(false);
                    bottomShelfScreen.SetActive(false);
                    drawerMainScreen.SetActive(false);
                    //window
                }
                if(hit.collider.gameObject.tag == "ToTopShelf")
                {
                    leftScreen.SetActive(false);
                    topShelfScreen.SetActive(true);
                    bottomShelfScreen.SetActive(false);
                }
                if (hit.collider.gameObject.tag == "ToBottomShelf")
                {
                    leftScreen.SetActive(false);
                    topShelfScreen.SetActive(false);
                    bottomShelfScreen.SetActive(true);
                    drawerMainScreen.SetActive(false);
                }
                if (hit.collider.gameObject.tag == "ToDrawer")
                {
                    leftScreen.SetActive(false);
                    bottomShelfScreen.SetActive(false);
                    drawerMainScreen.SetActive(true);
                }
                if (hit.collider.gameObject.tag == "ToSafe")
                {
                    mainScreen.SetActive(false);
                    safeMainScreen.SetActive(true);
                }
            }
            else
            {
                Debug.Log("no collider detected");
            }
        }
    }
}
2

since something can only have 1 tag at a time you could use a switch statement, other thing you could do, since most of your cases are just disabling all but 1 object, you could have a function that sets the active state of everything to false, then in each case just enable the one object you care about.

2 Likes
3

Yes, I was thinking of something along those lines, but I’m not sure how to do that exactly… It would be awesome if you could point me in the right direction.

4

Assuming you add an else before every if in the chain except for the 1st one, switch doesn’t really have much benefit in this use case, nor do I think it looks cleaner.

5

Well, I think that what you actually need is a UI system. Consider creating a class say Window that will have the functionality of every window/screen in a game. And then with inheritance create a class for your windows.

Super simple example:

public class Window : MonoBehaviour
{
    public void Show()
    {
        gameObject.SetActive(true);
    }

    public void Hide()
    {
        gameObject.SetActive(false));
    }
}

With that, create classes for your screens/windows:

public class MainWindow : Window
{
    
}

Now you can create a UI Manager class that will call Show on a Window that you want to open, and Hide on every other window that you have.

1 Like
6

This looks interesting and very helpful! I think that might just be what I’m looking for.

I’m only a bit unsure about this part:

I’m not sure if this works the same way with hiding/showing the windows when the corresponding collider was detected. I mean, it probably would, but I’m not sure how exactly… But thank you so far, I’m gonna fiddle around with this

7

To expand, take all of those SetActive(false)'s in Start, and move them into a new function named hideEverything. Call it in Start, and also call it before that pile of IF’s:

void Start() {
  hideEverything();
  ...
}

void Update() {
  ...
  if (hit.collider != null) {
    hideEverything();
    if (hit.collider.gameObject.tag == "ToCenter")
      centerScreen.SetActive(true);
    else ...
  ...
}

void hideEverything() {
  mainScreen.SetActive(false);
  ...
}

Maybe a nicer way would be to put everything in an array and remember mainScreen has index 0, and so on, but if you don’t know arrays well it would be messy.

2 Likes
8

Now this I like a lot. That’s, I feel, how it should have been. Now I don’t know how to mess with arrays, really, but this is the perfect starting off point, I think. Thank you!
The other replies also suggested something along those lines, but it’s good to actually see it.