HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Simple Q&A game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
gamesimplestackoverflow

Problem

I just wanted to get some criticism on my programming to make me better. I made a game in Unity, just a simple question and answer game.

Demo

```
using UnityEngine;
using UnityEngine.UI;
using System.Collections.Generic;
using System.Collections;
using System.Linq;
using UnityEditor.VersionControl;
using System.Threading;
using System;

[System.Serializable]
class Question
{
public string question;

public bool answer;

public Question(string question, bool answer)
{
this.answer = answer;
this.question = question;
}

public string getQuestion()
{
return question;
}
public void setQuestion(string msg)
{
question = msg;
}

public bool getAnswer()
{
return answer;
}

}

class NoMoreQuestionsException : Exception
{
public NoMoreQuestionsException()
{
}
}

public class GameManager : MonoBehaviour {

Question currentQuestion;
[SerializeField]
float buttonMoveSpeed;

int points = 0;

bool isPlayerCorrect;

bool changedAfterAnswer = false;

bool moveComplete = false;

int answer = 0;

int currentQuestionNumber;

[SerializeField]
float waitBetweenQuestions;

//List of questions
[SerializeField]
Question[] questions;

List unansweredQuestions = new List();

//Variables
[SerializeField]
GameObject questionText;

[SerializeField]
GameObject falseButton;

[SerializeField]
GameObject trueButton;

[SerializeField]
Text trueAnswerText;

[SerializeField]
Text falseAnswerText;

[SerializeField]
Text pointsText;

public void Answered (bool playerAnswer)
{
if (playerAnswer)
{
answer = 1;
} else
{
answer = -1;
}
}

void NewRound()
{
if (unansweredQuestions.Count ();
txt.text = currentQuestion.question;
//Reset players result
answer = 0;
//Set the

Solution

You should avoid duplication of code.

e.g.

  1. You can extract



if (falseButton.transform.position.x > 1330)
    {
        moveComplete = true;
        return;
    }

    Vector3 pos = falseButton.transform.position;

    pos.x += buttonMoveSpeed;

    falseButton.transform.position = pos;
    moveComplete = false;


to a method passing 1330 or -25 as a parameter and reuse it instead of having nearly exactly the same code twice.

  1. You can extract



if (!changedAfterAnswer)
        {
            if (isCorrect(true))
            {
                setAnswerText(true, "Correct!");
            }
            else
            {
                setAnswerText(true, "Incorrect");
            }
            changedAfterAnswer = true;
        }
        //Move the false button
        moveFalse();
        if (!startedTimer)
        {
            Debug.Log("STARTED WA8T");
            startedTimer = true;
            StartCoroutine(StartWait());
        }


to a method passing true or false as a parameter and reuse it instead of having nearly exactly the same code twice.

Code Snippets

if (falseButton.transform.position.x > 1330)
    {
        moveComplete = true;
        return;
    }

    Vector3 pos = falseButton.transform.position;

    pos.x += buttonMoveSpeed;

    falseButton.transform.position = pos;
    moveComplete = false;
if (!changedAfterAnswer)
        {
            if (isCorrect(true))
            {
                setAnswerText(true, "Correct!");
            }
            else
            {
                setAnswerText(true, "Incorrect");
            }
            changedAfterAnswer = true;
        }
        //Move the false button
        moveFalse();
        if (!startedTimer)
        {
            Debug.Log("STARTED WA8T");
            startedTimer = true;
            StartCoroutine(StartWait());
        }

Context

StackExchange Code Review Q#126825, answer score: 5

Revisions (0)

No revisions yet.