0
\$\begingroup\$

I'm trying to make a mini quiz game, but I'm stuck. I'm showing questions, randomly, but the first question never comes up. Furthermore, when I set the size of the questionsNumberChoosen array to 10 and run the game, it crashes.

public struct Question
{
    public string questionText;
    public string[] answers;
    public int correctAnswerIndex;

    // .. we also construct the Question, directly providing all values.
}

public Text questionText;
public Button[] answerButtons;

private Question currentQuestion;
private Question[] questions = new Question[10];
private int[] questionNumbersChoosen = new int[9];
private int questionsFinished;

void Start () 
{       
    questions [0] = new Question("1", new string[]{"A", "A", "A", "A", "A" }, 2);
    questions [1] = new Question("2", new string[]{"A", "A", "A", "A", "A" }, 0);
    questions [2] = new Question("3", new string[]{"A", "A", "A", "A", "A" }, 4);
    questions [3] = new Question("4", new string[]{"A", "A", "A", "A", "A" }, 0);
    questions [4] = new Question("5", new string[]{"A", "A", "A", "A", "A" }, 0);
    questions [5] = new Question("6", new string[]{"A", "A", "A", "A", "A" }, 2);
    questions [6] = new Question("7", new string[]{"A", "A", "A", "A", "A" }, 0);
    questions [7] = new Question("8", new string[]{"A", "A", "A", "A", "A" }, 4);
    questions [8] = new Question("9", new string[]{"A", "A", "A", "A", "A" }, 0);
    questions [9] = new Question("10", new string[]{"A", "A", "A", "A", "A" }, 0);

    setQuestionIndex ();
    assignQuestion (questionNumbersChoosen[0]);
}

void assignQuestion(int questionNum)
{
    currentQuestion = questions [questionNum];
    questionText.text = currentQuestion.questionText;
    for (int i=0; i < answerButtons.Length; i++)            
        answerButtons[i].GetComponentInChildren<Text>().text 
            = currentQuestion.answers [i];
}

public void checkAnswer(int buttonNum)
{
    if (buttonNum == currentQuestion.correctAnswerIndex) 
        print ("Correct!");
    else
        print ("Incorrect!");

    if (questionsFinished < (questionNumbersChoosen.Length - 1))        
        moveToNextQuestion ();
}

void setQuestionIndex()
{
    for (int i = 0; i < questionNumbersChoosen.Length; i++) 
    {
        int questionNum = Random.Range (0, questions.Length);

        if (!isContain (questionNumbersChoosen, questionNum))
            questionNumbersChoosen [i] = questionNum;
        else
            i--;
    }

}

bool isContain (int[] num, int numbers)
{
    for (int i=0; i < num.Length; i++) 
    {
        if (numbers == num [i])
            return true;            
    }

    return false;
}

public void moveToNextQuestion()
{
    assignQuestion (questionNumbersChoosen
        [questionNumbersChoosen.Length -1 - questionsFinished++]);
}
\$\endgroup\$
1
  • \$\begingroup\$ As people say in their answers, what you are doing in the for loop in not really safe. You would be better off with a while loop inside the for loop instead of doing that ugly i--;. That said, it is really easy for you to find what the problem is : Put a breakpoint in your loop and check what is happening \$\endgroup\$ – realUser404 Dec 22 '16 at 9:50
1
\$\begingroup\$

You could potentially be causing problems by editing the iterator in your for loop setQuestionIndex().

Try doing it without, like:

void setQuestionIndex()
{
    int questionIterator = 0;
    for (int i = 0; i < questionNumbersChoosen.Length; i++) {
         int questionNum = Random.Range (0, questions.Length);
         if (!isContain (questionNumbersChoosen, questionNum))
         {
              questionNumbersChoosen [questionIterator ] = questionNum;
              questionIterator ++;
         }
     }
}
\$\endgroup\$
4
  • \$\begingroup\$ This code will not do as expected unless you change the loop \$\endgroup\$ – realUser404 Dec 22 '16 at 9:46
  • \$\begingroup\$ Good point, a while loop would probably be better for what he's trying to do. \$\endgroup\$ – Raeles Dec 22 '16 at 9:58
  • 1
    \$\begingroup\$ Just change the for loop to a while (questionIterator < questionNumbersChoosen.Length) and it should be good imho \$\endgroup\$ – realUser404 Dec 22 '16 at 10:08
  • \$\begingroup\$ This seems to be the best answer, here. While @MikeWhite addresses the logical error that causes question[0] to fail, this addresses why the program crashes. By creating a generic for(int x = 0; x < length; x++) loop that involves x--, you are effectively running the same number over and over again. This should create an infinite loop. Infinite loops are bad, mmkay? \$\endgroup\$ – Gnemlock Jan 22 '17 at 12:37
1
\$\begingroup\$

You initialize the size of questionNumbersChoosen to length of 9 (or 10), and the values default to 0.

That means that anytime you randomly select question[0], isContain(questionNumbersChoosen, questionNum) is true, which is considered a failure.

When the length of questionNumbersChoosen is 10, and 9 questions have been assigned, the only question left to choose from is question[0], butquestion[0]keeps being rejected sincequestionNumbersChoosen[9]` is still the default value of 0.

The quickest fix is to initialize questionNumbersChoosen[] to a value outside of the range 0 to 9.

\$\endgroup\$
2
  • \$\begingroup\$ I rolled back my answer because @Gnemlock edited it after misreading it. It appears he confused the idea of the array length with my suggestion of initializing the values inside the array to a something outside of the range of 0 to 9, -1 or 10 would be an example. Changing the code to a while loop, will still create an infinite loop if the array contains the default value of 0. \$\endgroup\$ – Mike White Jan 22 '17 at 15:49
  • \$\begingroup\$ Ahh, I understand, now. Sorry for the confusion. I'm going to reapply the formatting, as it still makes it easier for others to read / interpret. \$\endgroup\$ – Gnemlock Jan 22 '17 at 15:54
-1
\$\begingroup\$

Though I do not see precisely what the specific problem is, I will list the few things that bother me in your code, and how you can easily find the bug yourself :

  1. The loop in setQuestionIndex() method is error-prone and I suggest you fix it, as changing the iterator inside the loop is a bad practice imho. An option would be to do as Raeles suggests, replacing the for loop by a while (see my comment below his answer). An other option would just be to do some random permutations in your question array, which should prove more efficient.
  2. I do not see where you initialize questionsFinished to 0. Maybe in Unity it is not mandatory to initialize variables, but still a good practice would be to explicitely write questionsFinished = 0 in your start method.
  3. You do not need to pass questionNumbersChoosen in the parameters of isContain as it is a class variable. That just makes the code more confusing.

Now how would I debug it if I were you? In most IDE you can easily debug your code by adding Breakpoints. Just check step by step what is happening, put one in your setQuestionIndex loop to see if you have an infinite loop there (as you said Unity is freezing), if not just keep going, see how the questions are ordered in your array, check the state of the variables, and you should understand the problem pretty quickly.

\$\endgroup\$
2
  • \$\begingroup\$ This appears to be commentary on the example, rather than a solution to the problem at hand. \$\endgroup\$ – Gnemlock Jan 22 '17 at 12:22
  • \$\begingroup\$ @Gnemlock if you read the comments on other answers you will see I addressed the problem there already and I did not think it was any relevant to add another answer saying the same thing. However I feel my answer is very interesting for the OP as it allows him to improve instead of feeding him the fish. Sorry if you do not feel the same, and thx for -1 \$\endgroup\$ – realUser404 Jan 23 '17 at 9:27

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.