Bug & fix with conversation thread that ends with a Player Speaking node

Unity Courses Show
,
#1

If the last node in a dialogue thread is a blue “Player Speaking” node (with no children), there’s an exception raised in PlayerConversant.Next() because there are no AI children nodes.

Unfortunately Random.Range(0, 0) returns 0, even though the function documentation says that it should never return the exclusive end of the range (perhaps it should throw an exception instead?). So using the length of the child array as the endpoint results in attempting to address index 0 of a 0-length array.

I fixed this by first checking for no children, and if there aren’t any, calling Quit() instead. This seems to work nicely - you get the last choice of dialogue, you pick one, and then the dialogue panel is hidden.

    if (childNodes.Length > 0)
    {
        currentNode = childNodes[Random.Range(0, childNodes.Length)];
    }
    else
    {
        Quit();
    }
1 Like
#2

Well done with this! What you’ve spotted is known as a corner case, a unique set of circumstances that can lead to an error.

It’s true, Random.Range(0,0) will return 0 (it has to return something). Interestingly enough, I looked through my personal project, and I must have arrived at the same issue (I don’t have any notes about it), or more likely anticipated it.

#3

it has to return something

Well, perhaps… isn’t that what exceptions are for? However I’d lean more towards this being a documentation bug since I can imagine useful scenarios where you want a random number from a range and would expect the one value in that range to be generated if it was length zero.

It does however result in the inconsistency that Random.Range(0, 0) is functionally equivalent to Random.Range(0, 1) but this is not true in the general case of Random.Range(x, x) equivalent to Random.Range(x, x + 1).

Thinking aloud… mathematically, does the interval [x, x) contain x? Wikipedia suggests that for integers the interval is actually always inclusive at both ends, and that [x, x) notation (exclusive end-point) is rare for such numbers, such that the endpoint is always included in the set. Anyway, I digress.

#4

You could consider putting in a bug fix request with Unity to return an exception, but in truth, using the result of Random.Range(0,empty collection.Length) will return an exception already.

As a rule, any time you’re using a Random element, a collection should be checked to be sure it’s not empty.

#5

I like the sound of this and added it to my PlayerConversant.cs in the next procedure.

I have a problem with the naming of the Quit… I re-named it to QuitDialogue, so as to not be confused with Quitting the game…

        public void Next()
        {
            int numPlayerResponses = currentDialogue.GetPlayerChildren(currentNode).Count();
            if(numPlayerResponses > 0)
            {
                isChoosing = true;
                TriggerExitAction();
                onConversationUpdated();
                return;
            }  

            DialogueNode[] children = currentDialogue.GetAIChildren(currentNode).ToArray();
            if(children.Length > 0)
            {
                TriggerExitAction();
                currentNode = children[UnityEngine.Random.Range(0, children.Count())];
                TriggerEnterAction();
                onConversationUpdated();
            }
            else
            {
                Quit();
            }
            //TODO
            //TriggerExitAction();
            //currentNode = children[randomIndex];
            //TODO
        }

Did I put it in the right spot seems to work…?

#6

Looks right!

#7

Just sharing another way:

public void SelectChoice(DialogueNode chosenNode)
        {
            _currentNode = chosenNode;
            TriggerEnterAction();
            _isChoosingDialogue = false;

            if (HasNextDialogue() == false)
                ExitDialogue(); // AKA Quit()
            else
                NextDialogue(); // AKA Next()
        }

Privacy & Terms