3
\$\begingroup\$

Below is some part of a slot machine game that I have made using unity. I always work alone and I know I poorly comment. I also feel insecure putting this on github and show it to potential employers due to poor commenting and inefficient code.

For the rewards I generate a random number.

float randomVariable = Random.value/payoutRatio;

Then I use many many else if statements to check if this number below N1 give this prize, else if it is below N2 give that prize. I am not happy with that approach.

Would appreciate any help on commenting and also better approaches.

using UnityEngine;
using UnityEngine.UI;
using UnityEngine.SceneManagement;
using System.Collections;
using System.Linq;
using System.Collections.Generic;

public class randomController : MonoBehaviour {
    public AudioSource sound;
    public AudioSource winSound;
    public AudioSource bigWin;
    public Button spinButton;
    public Text payout1;
    public Text credit;
    public Text multiplierLabel;
    public Sprite dimmedButton;
    public Sprite litButton;
    public static bool[] updateCredit = {false, false, false};
    public static bool slotsStopped = false;
    public static bool winAnimPlayed = false;
    public static int creditCounter;
    public static bool spinButtonPressed = false;
    public static int multiplier = 1;
    private int winningNumber;
    private int donemlikAtilan;
    private int donemlikVerilen;
    private float profit;
    public static int spinWinning = 0;
    private int[] slotPositions = { 1, 2, 1 };
    private bool tripleSlot = false;
    private bool oneOrTwoCherries = false;
    private bool animationPlaying = false;
    public static bool wofIsSpinning = false;
    private string winAnimatorName;
    private string winAnimationName;
    private float payoutRatio;
    private float[] payoutRanges = { 0.792069f, 0.150602f, 0.031325f, 0.012048f, 0.004819f, 0.0070871f, 0.0012048f, 0.00084337f };
    private postRequest postman = new postRequest ();


    void Start() {
        payoutRatio = PlayerPrefs.GetFloat ("payoutRatio", 0.5f);
        donemlikAtilan = PlayerPrefs.GetInt("donemlikAtilan", 0);
        donemlikVerilen = PlayerPrefs.GetInt("donemlikVerilen", 0);
        creditCounter = PlayerPrefs.GetInt ("currentCredit", 0);
        credit.text = creditCounter.ToString ();
        Button btn = spinButton.GetComponent<Button> ();
        btn.onClick.AddListener(TaskOnClick);
    }
    void FixedUpdate() {
        creditCounter = PlayerPrefs.GetInt ("currentCredit", 0);
        credit.text =  creditCounter.ToString ();
        donemlikAtilan = PlayerPrefs.GetInt("donemlikAtilan", 0);
        donemlikVerilen = PlayerPrefs.GetInt("donemlikVerilen", 0);
        if (spinButtonPressed) {
            TaskOnClick ();
        }
        if (slotsStopped && wofIsSpinning && !animationPlaying)
        {
            StartCoroutine(wofAnimationRun());
        }
        if (slotsStopped && !wofIsSpinning && spinWinning > 0 && !winAnimPlayed) {
            StartCoroutine (startWinAnim ());
            if (PlayerPrefs.GetInt ("instantReward", 0) == 1) {
                serialListener.stream.Open ();
                serialListener.stream.Write ("000");
                serialListener.stream.Write ((spinWinning).ToString () + "G");
            }
        }
        if (spinButton.IsActive() && !serialListener.stream.IsOpen && !animationPlaying) {
            serialListener.stream.Open ();

            creditCounter = PlayerPrefs.GetInt ("currentCredit", 0);
        }
        if (spinButton.IsActive() && serialListener.stream.IsOpen) {
            credit.text =  creditCounter.ToString ();
            creditCounter = PlayerPrefs.GetInt ("currentCredit", 0);
        }
        multiplierLabel.text = multiplier.ToString();
    }
    void TaskOnClick() {

        if (creditCounter < multiplier) {
            return;
        }
        payoutRatio = PlayerPrefs.GetFloat ("payoutRatio", 0.5f);
        profit = (donemlikAtilan - donemlikVerilen) - donemlikVerilen * (1f - payoutRatio)/3 + 50;
        animationPlaying = false;
        spinWinning = 0;
        PlayerPrefs.SetInt ("toplamAtilan",PlayerPrefs.GetInt("toplamAtilan",0)+multiplier);
        PlayerPrefs.SetInt ("donemlikAtilan",PlayerPrefs.GetInt("donemlikAtilan",0)+multiplier);
        PlayerPrefs.SetInt ("grandTotal",PlayerPrefs.GetInt("grandTotal",0)+multiplier);
        winAnimPlayed = false;
        spinButtonPressed = false;
        serialListener.stream.Close ();
        slotsStopped = false;
        toggleOffWin ();
        sound.Play ();
        updateCredit [0] = false;
        updateCredit [1] = false;
        updateCredit [2] = false;
        creditCounter -= 1*multiplier;
        postman.postCoins (1, creditCounter);
        PlayerPrefs.SetInt ("currentCredit", creditCounter);    
        credit.text = creditCounter.ToString ();
        float randomVariable = Random.value/payoutRatio; //this number is in google sheets            Debug.Log(randomVariable);
        periodicBCIncluded3.noWin = false;
        if (randomVariable < (payoutRanges[1]) && (profit) > 2 * multiplier) {
            winningNumber = 1;
            oneOrTwoCherries = true;
            spinWinning = 2 * multiplier;
            creditCounter += spinWinning;

            PlayerPrefs.SetInt ("currentCredit", creditCounter);
            int cherrySlot = Random.Range (0, 3);
            Debug.Assert (cherrySlot != 3);
            slotPositions [cherrySlot] = 1;
            List<int> availableElements = new List<int> ();
            for (int i = 1; i <= 7; i++) {
                if (i != 1) {
                    availableElements.Add (i);
                }
            }

            Debug.Assert (availableElements.Count == 6);
            for (int i = 0; i < 3; i++) {
                if (i != cherrySlot) {
                    slotPositions [i] = availableElements [Random.Range (0, availableElements.Count - 1)];
                }
            }
            bool noExtraCherry = false;
            while (!noExtraCherry) {
                int cherryCount = 0;
                for (int i = 0; i < 3; i++) {
                    if (slotPositions[i] == 1) {
                        cherryCount++;
                    }
                }
                if (cherryCount == 1) {
                    noExtraCherry = true;
                }
            }
            //Two cherries x5
        } else if (randomVariable < (payoutRanges[1]+payoutRanges[2]) && profit > 5 * multiplier) {
            oneOrTwoCherries = true;
            spinWinning = 5 * multiplier;
            creditCounter += spinWinning;

            PlayerPrefs.SetInt ("currentCredit", creditCounter);
            List<int> availableElements = new List<int> ();
            for (int i = 1; i <= 7; i++) {
                if (i != 1) {
                    availableElements.Add (i);
                }
            }

            int nonCherrySlot = Random.Range (0, 3);
            for (int i = 0; i < 3; i++) {
                if (i != nonCherrySlot) {
                    slotPositions [i] = 1;
                }
            }
            slotPositions [nonCherrySlot] = availableElements [Random.Range (0, availableElements.Count)];
            //3 Cherries x10
        } else if (randomVariable < (payoutRanges[1]+payoutRanges[2]+payoutRanges[3]) && profit > 10 * multiplier ) {
            spinWinning = 10 * multiplier;
            creditCounter += spinWinning;

            PlayerPrefs.SetInt ("currentCredit", creditCounter);
            slotPositions [0] = 1;
            slotPositions [1] = 1;
            slotPositions [2] = 1;
            tripleSlot = true;
        } else if (randomVariable < (payoutRanges[1]+payoutRanges[2]+payoutRanges[3]+payoutRanges[4]) && profit > 20 * multiplier) {
            spinWinning = 20 * multiplier;
            creditCounter += spinWinning;

            PlayerPrefs.SetInt ("currentCredit", creditCounter);
            slotPositions [0] = 2;
            slotPositions [1] = 2;
            slotPositions [2] = 2;
            tripleSlot = true;
//          carkifelek
        } else if ((randomVariable < (payoutRanges[1]+payoutRanges[2]+payoutRanges[3]+payoutRanges[4]+payoutRanges[5])) )  {
            Debug.Log ("asdasdqwqqqqqq");
            float[] wofPayRanges = { 0.5f, 0.35f, 0.1f, 0.05f};
            float wofRandom = 600f;
            bool spinSuccess = false;
            if (profit > 100) {
                wofRandom = Random.value;

            } else if (profit > 50) {
                wofRandom = Random.value * (wofPayRanges [0] + wofPayRanges [1] + wofPayRanges [2]);
            } else if (profit > 30) {
                wofRandom = Random.value * (wofPayRanges [0] + wofPayRanges [1]);
            } else if (profit > 10) {
                wofRandom = Random.value * (wofPayRanges [0]);
            }

            if (wofRandom > (wofPayRanges [0] + wofPayRanges [1] + wofPayRanges [2]) && profit > 100 * multiplier) {
                spinWinning = 100 * multiplier;
                creditCounter += spinWinning;
                spinSuccess = true;
            } else if (wofRandom > (wofPayRanges [0] + wofPayRanges [1]) && profit > 50 * multiplier) {
                spinWinning = 50 * multiplier;
                creditCounter += spinWinning;
                spinSuccess = true;
            } else if (wofRandom > (wofPayRanges [0]) && profit > 30 * multiplier) {
                spinWinning = 30 * multiplier;
                creditCounter += spinWinning;
                spinSuccess = true;
            } else if (wofRandom > 0 && profit > 10 * multiplier) {
                spinWinning = 10 * multiplier;
                creditCounter += spinWinning;
                spinSuccess = true;
            }
            if (spinSuccess) {
                wofIsSpinning = true;
                PlayerPrefs.SetInt ("currentCredit", creditCounter);
                slotPositions [0] = 7;
                slotPositions [1] = 7;
                slotPositions [2] = 7;
                tripleSlot = true;
            }
        } else if (randomVariable < (payoutRanges[1]+payoutRanges[2]+payoutRanges[3]+payoutRanges[4]+payoutRanges[5]+payoutRanges[6]) && (donemlikAtilan - donemlikVerilen) > 50 * multiplier) {
            spinWinning = 50 * multiplier;
            creditCounter += spinWinning;

            PlayerPrefs.SetInt ("currentCredit", creditCounter);
            slotPositions [0] = 4;
            slotPositions [1] = 4;
            slotPositions [2] = 4;
            tripleSlot = true;
        }   else if (randomVariable < (payoutRanges[1]+payoutRanges[2]+payoutRanges[3]+payoutRanges[4]+payoutRanges[5]+payoutRanges[6]+payoutRanges[7]) && (donemlikAtilan - donemlikVerilen) > 100 * multiplier ) {
            spinWinning = 100 * multiplier;
            creditCounter += spinWinning;

            PlayerPrefs.SetInt ("currentCredit", creditCounter);
            slotPositions [0] = 5;
            slotPositions [1] = 5;
            slotPositions [2] = 5;
            tripleSlot = true;
        }
        if (spinWinning == 0)
        {

            periodicBCIncluded3.noWin = true;
            bool noMoneyBool = false;
            while (!noMoneyBool)
            {
                for (int i = 0; i < slotPositions.Length; i++)
                {
                    slotPositions[i] = Random.Range(1, 8);
                    Debug.Assert(slotPositions[i] < 10);
                }
                var val = slotPositions[0];
                bool allElementsEqual = slotPositions.All(x => x == val);
                bool noCherry = slotPositions.All(x => x != 1);
                if (!allElementsEqual && noCherry)
                {
                    noMoneyBool = true;
                }
            }
        }
        periodicBCIncluded.stoppingElement = (slotPositions[0]).ToString();
        periodicBCIncluded2.stoppingElement = (slotPositions[1]).ToString();
        periodicBCIncluded3.stoppingElement = (slotPositions[2]).ToString();
        if (spinWinning > 0) {
            PlayerPrefs.SetInt ("toplamVerilen", PlayerPrefs.GetInt ("toplamVerilen", 0) + spinWinning);
            PlayerPrefs.SetInt ("donemlikVerilen", PlayerPrefs.GetInt ("donemlikVerilen", 0) + spinWinning);
            PlayerPrefs.SetInt ("grandGiven",PlayerPrefs.GetInt("grandGiven",0)+spinWinning);
        }
        toggleButton();
    }

    IEnumerator startWinAnim() {
        if (spinWinning > 0) {
            animationPlaying = true;
            //serialListener.stream.Close ();
            Animator winAnimator = GameObject.Find ("winAnimation").GetComponent<Animator> ();
            Animation winAnimation = GameObject.Find ("winAnimation").GetComponent<Animation> ();
            winAnimator.SetTrigger ("notIdle");
            StartCoroutine (WaitForAnimation (winAnimation));
            winAnimPlayed = true;
            toggleButton ();
            animationPlaying = false;
            yield return new WaitForSeconds (0);
        }
    }

    IEnumerator wofAnimationRun()
    {
        animationPlaying = true;
        serialListener.stream.Close();
        Animator winAnimator = GameObject.Find("wof").GetComponent<Animator>();
        winAnimator.SetTrigger((spinWinning/multiplier).ToString());
        Debug.Log ((spinWinning / multiplier).ToString ());
        yield return new WaitForSeconds(0);
    }
    private IEnumerator WaitForAnimation ( Animation animation )
    {
        do
        {
            yield return null;
        } while ( animation.isPlaying );
    }
    void toggleOffWin() {
        GameObject.Find ("win1_1").GetComponent<Renderer> ().enabled = false;
        GameObject.Find ("win1_2").GetComponent<Renderer> ().enabled = false;
        GameObject.Find ("win1_3").GetComponent<Renderer> ().enabled = false;
        GameObject.Find ("win2_1").GetComponent<Renderer> ().enabled = false;
        GameObject.Find ("win2_2").GetComponent<Renderer> ().enabled = false;
        GameObject.Find ("win2_3").GetComponent<Renderer> ().enabled = false;
        GameObject.Find ("win3_1").GetComponent<Renderer> ().enabled = false;
        GameObject.Find ("win3_2").GetComponent<Renderer> ().enabled = false;
        GameObject.Find ("win3_3").GetComponent<Renderer> ().enabled = false;
    }

    void toggleButton(){
        if (spinButton.enabled) {
            spinButton.enabled = false;
            spinButton.GetComponent<Image> ().sprite = dimmedButton;
        } else {
            spinButton.enabled = true;
            spinButton.GetComponent<Image> ().sprite = litButton;
        }
    }

}
\$\endgroup\$
  • 4
    \$\begingroup\$ Holy cow! These a really a lot of ifs and fields. It's called a God object. \$\endgroup\$ – t3chb0t Oct 27 '17 at 9:20
  • 2
    \$\begingroup\$ The first thing that I've noted is that you are using different standards for your opening brace: either stick to the Java standard, or choose another like the ISO standard, but don't do both. \$\endgroup\$ – Shaun Bebbers Oct 27 '17 at 10:34
  • 1
    \$\begingroup\$ @ShaunBebbers Thank you I'll correct myself. \$\endgroup\$ – DocWork Oct 27 '17 at 11:15
  • \$\begingroup\$ @t3chb0t I realize I have to do some extended reading/studying to get rid of making god objects. Thanks a lot! \$\endgroup\$ – DocWork Oct 27 '17 at 11:16
  • 3
    \$\begingroup\$ @DocWork - no problems about the Turkish - it's more consistency that is important. I would find it difficult to write C# or in Turkish or anything other than English (and I struggle with that sometimes) \$\endgroup\$ – Shaun Bebbers Oct 30 '17 at 9:49
5
\$\begingroup\$

Your code is a mess on all levels.

  • Sometimes you write Method () with a space, sometimes you write Method() without the space.
  • All method and class names should start with an uppercase letter. It should be RandomController instead of randomController.
  • In the i != 1 loop, why don't you just start counting at 2?
  • Never use static fields. They are only for things that exist exactly once in the whole world. If I am to open a casino and put 10 of your slot machines into it, each one of them must have its own creditCounter. Currently they all share one, making your slot machine a multiplayer game.
  • I have no idea what donemlikAtilan means. Don't mix languages in the variable names.
  • What does a serialListener have to do with the code for the calculation? Nothing. Therefore they should not be in the same class.

Your code should be split into two parts:

  • One part is the actual slot machine that only calculates things without caring about the user interface.
  • The other part is the user interface. It gives commands to the slot machine and receives feedback.

Splitting the code has the benefit that if you find a bug in the calculations, you only have to search for it in half of the code, since it is clearly separated from the displaying code.

The class for the slot machine should be written so that you can easily test it. For that, it needs to simulate time. Because in the tests you want to have reproducible behavior, but in production it must be random and unpredictable.

public class SlotMachine {

    private readonly Random rnd;
    private readonly TimeProvider timeProvider;

    public SlotMachine(Random rnd, TimeProvider timeProvider) {
        this.rnd = rnd;
        this.timeProvider = timeProvider;
    }

    public void StartWheels() { ... }
    public void StopWheel() { ... }
}

Then, you can test it reliably:

[TestClass]
public class SlotMachineTest {

    [TestMethod]
    public void TestSimpleGame() {
        var time = new SimulatedTimeProvider();
        var rnd = new Random(0);
        var machine = new SlotMachine(rnd, time);

        machine.StartWheels();
        time.Pass(5000);
        machine.StopWheel();
        time.Pass(1000);
        machine.StopWheel();
        time.Pass(2000);
        machine.StopWheel();

        Assert.AreEqual("777", machine.Display);
    }
}

Something like this.

Oh, and please do us a favor. Let your IDE format your code consistently. Just press Ctrl+K Ctrl+D in an open file.

\$\endgroup\$
  • \$\begingroup\$ Pretty good points. Thank you very much for your time and effort! It helps a lot! \$\endgroup\$ – DocWork Oct 30 '17 at 19:30
4
\$\begingroup\$

You said you comment code poorly, which isn't exactly true, you don't comment. I can't be bothered to read all that code and try and actually understand what it is doing, I just scanned it looking for the comments.

You code isn't very easy on the eye, mainly the lack of comments, but also the lack of blank lines between functions and the consistency (as mentioned before the braces) also the fact you are relying on implicit privacy specifications in some places and others you are saying it explicitly.

I would certainly look at sub-classing some of the class data and possibly functionality. I haven't looked at it in depth, but I think some of it could be another class.

Also your use of arrays is inconsistent, i.e.:

periodicBCIncluded3.stoppingElement = (slotPositions[2]).ToString();

Why is periodicBCIncluded not an array too?

I am sorry but to be honest I wouldn't employ you at this time. Devs spend more time maintaining code than writing it, more often than not other peoples code. If I gave this to a different dev they would just re-write it, because they couldn't understand what you have done. However if you sorted out your 'bad habits' (mainly comments) then you would be considered. Basically, don't put it in Github yet, spend an hour tidying it up.

Please don't be disheartened by what I have said, you have potential and most importantly you know what you are doing wrong. All you need to do now is sort it out. :)

\$\endgroup\$
  • 1
    \$\begingroup\$ You definitely, hundred percent, cannot divide by my disheartenedness. Thank you very much! I have never worked with other programmers I am self taught and only done this job for people who wouldn't see the code so it didn't give me much chance to learn how to work in a team who will actually see your code. \$\endgroup\$ – DocWork Oct 30 '17 at 17:59

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.