0
\$\begingroup\$

I want to construct a 7 segment display (as shown below). When you input a number, the script will read the input number and light up the respective segment (change the spriterenderer color to red) to display the number.

But I think the code has become too long, since there is 0-9 digit to display, I need 10 if statements, 8 scripts and the function of each statement is to light up the respective segment so it displays the number correctly.

Is there any better way to make the code shorter?

using UnityEngine;

public class ClockController :  MonoBehaviour
{
    Rect1 rect1;
    Rect2 rect2;
    Rect3 rect3;
    public GameObject othergameobjectrect2;
    public GameObject othergameobjectrect3;

    void Start()
    {
        int x = 1;
        if (x == 1)
        {
            rect1 = GetComponent<Rect1>();
            rect1.Rectangle1();
           
            rect2 = othergameobjectrect2.GetComponent<Rect2>();
            rect2.Rectangle2();

            rect3 = othergameobjectrect3.GetComponent<Rect3>();
            rect3.Rectangle3();

        }
    }
}
using UnityEngine;

public class Rect3 : MonoBehaviour
{
    public void Rectangle3()
    {
        GetComponent<SpriteRenderer>().color = Color.red;
        
    }
}
using UnityEngine;

public class Rect2 : MonoBehaviour
{
    public void Rectangle2()
    {
         GetComponent<SpriteRenderer>().color = Color.red; 
    }
}
using UnityEngine;

public class Rect1 : MonoBehaviour
{   
    public void Rectangle1()
    {    
        GetComponent<SpriteRenderer>().color = Color.red;    
    }
}

enter image description here

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Hi, I'm not sure if GDSE is the place to ask "How do I make my code shorter". That said, start by getting to know prefabs. You don't want 1 script per rectangle, you want 1 Rectangle class (without any number in the name) and make 8 different instances of that prefab in your scene, at different positions and with different indexes. To contol them, you could make an array of those rectangle prefabs, which is keyed by 0-7, so instead of doing if(x == 1) { rectangle1... } you don't have to check anything, you just say give me array[1] or array[2] and that returns you the correct one. \$\endgroup\$ – D.Kallan Jan 13 at 4:30
4
\$\begingroup\$

I'd attack it something like this...

Have one script that represents the digit as a whole, and assign to that script in the Inspector an array of SpriteRenderers representing the seven segments. They don't need their own custom scripts, since the only functionality we need from them is setting the colour, which the built-in SpriteRenderer component has already.

Then store a lookup table that encodes which segments should be on for each digit value.

When we set the clock digit to a new number, we can loop over all our segments, and set them to the corresponding state from the lookup table for that value.

The one tricky part here is that you need to assign your segment references to the correct array slots so they get the right value from the lookup table, but that should be easy enough to do with a little care.

public class ClockDigit : MonoBehaviour {

    //  Assuming you number your segments as follows:
    //   -- 0 --
    //  |       |
    //  5       1
    //  |       |
    //   -- 6 --
    //  |       |
    //  4       2
    //  |       |
    //   -- 3 --

    // Store a lookup table for which segments 
    // should be active when displaying each digit.
    static readonly bool[,] SEGMENT_IS_ACTIVE = new bool[,] {

        {true,  true,  true,  true,  true,  true,  false}, // 0
        {false, true,  true,  false, false, false, false}, // 1
        {true,  true,  false, true,  true,  false, true }, // 2
        {true,  true,  true,  true,  false, false, true }, // 3
        {false, true,  true,  false, false, true,  true }, // 4
        {true,  false, true,  true,  false, true,  true }, // 5
        {true,  false, true,  true,  true,  true,  true }, // 6
        {true,  true,  true,  false, false, false, false}, // 7
        {true,  true,  true,  true,  true,  true,  true }, // 8
        {true,  true,  true,  true,  false, true,  true }  // 9
    };    

    public Color32 activeColour = Color.red;
    public Color32 inactiveColour = Color.black;

    public SpriteRenderer[] segments = new SpriteRenderer[7];

    public void Display(int number) {
        var digit = number % 10;
        if (digit < 0) digit *= -1;

        for (int i = 0; i < 7; i++) {
            if (SEGMENT_IS_ACTIVE[digit, i]) {
                segments[i].color = activeColour;
            } else {
                segments[i].color = inactiveColour;
            }
        }
    }   
}
\$\endgroup\$
7
  • \$\begingroup\$ +1, this is what I used years ago for a digital clock screensaver \$\endgroup\$ – htmlcoderexe Jan 13 at 9:12
  • \$\begingroup\$ Why we need the command line if digit less than0,digit=digit+10? Under what condition, digit will be less than 0? Another command line if(SEGMENT....),below the for loop, this is to check the Boolean expression true or false in the specific arrary? As I following video tutorial haven't came across such way of writing. \$\endgroup\$ – chuackt Jan 13 at 11:31
  • \$\begingroup\$ If you pass a negative number as the argument to the function, the modulo operator will return a negative. To be robust against this type of misuse, I transform it into the corresponding positive (although I think I chose the wrong fix for that in this case. Usually with modulo we want a saw wave everywhere, but for this case we actually do want a direction change at zero. Let me change that to an absolute value...) \$\endgroup\$ – DMGregory Jan 13 at 12:46
  • \$\begingroup\$ I see, I was thinking to further modify the code to read datetimenow to get minutes to implement on the 7 segments, didn't figure that command is use to handle negative number. \$\endgroup\$ – chuackt Jan 13 at 14:14
  • \$\begingroup\$ I'd recommend keeping this code mostly as-is. Following the single responsibility principle, this code's one job is to set the display to a number. Make a new Clock script to decide what that number should be. It will have variables for four ClockDigit instances, compute the time each frame, and call Display(value) on each digit with the tens of hours, hours, tens of minutes, and minutes respectively. \$\endgroup\$ – DMGregory Jan 13 at 14:29
2
\$\begingroup\$

As mentioned by @D.Kallan stack exchanges are not normally a good place to ask about making things run more efficiently so much as about how to make them run (for example "How to I make my 7-segment display code run faster?" vs. "How do I make a 7-segment display?"). That being said your problem is pretty simple and easy to clean up.

public class SevenSegmentNumber : MonoBehaviour
{
    // Segment layout
    //  0   --
    // 1 2 |  |
    //  3   --
    // 4 5 |  |
    //  6   -- 
    public List<SpriteRenderer> segments = new List<SpriteRenderer>();
    public Color onColor = Color.red;
    public COlor offColor = Color.black;

    public void SetDisplayValue(int digit)
    {
        int value = digit % 10;

        switch (value)
        {
           case 0:
               segments[0].color = onColor;
               segments[1].color = onColor;
               segments[2].color = onColor;
               segments[3].color = offColor;
               segments[4].color = onColor;
               segments[5].color = onColor;
               segments[6].color = onColor;
               break;
            case 1:
               segments[0].color = offColor;
               segments[1].color = offColor;
               segments[2].color = onColor;
               segments[3].color = offColor;
               segments[4].color = offColor;
               segments[5].color = onColor;
               segments[6].color = offColor;
               break;
            // Fill in the rest here
        }
    }
}

public class SevenSegmentDisplay : MonoBehaviour
{
    public List<SevenSegmentNumber> digitDisplays = new List<SevenSegmentNumber>();
    
    private void Awake()
    {
        SetDisplayValue(0);
    }

    public void SetDisplayValue(int value)
    {
        int remainder = value;

        for (int digitId = 0; digitId < digitDisplays.Count; digitId++)
        {
            digitDisplays[digitId].SetDisplayValue(remainder % 10);
            remainder = remainder / 10;
        }
    }
}

Basically your problem is coming from the fact that you are writing unique scripts for every single game object rather than just one for each type (ex. "NpcBob", "NpcJim", "NpcJoe" scripts vs a single "Npc" script). I would suggest you check out some doing tutorials to learn a bit more about how to make generic scripts that can help you prevent this kind of scripting issue from coming up again. This will be especially helpful if you need to change your seven-segment display to show letters/strings, binary numbers, hex numbers, or punctuation.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.