Need help in assessing how good I am currently in writing scripts

Archived Forums Questions & Answers
,
1

I made these scripts by myself for my game. One is for the NPC behavior while the other is used for quests that might be part of an NPC. I want to check at what level of expertise do I lie for scripting.
For the NPC:

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class NPC : MonoBehaviour
{
    [SerializeField] bool questNpc=false;
    bool dialogueTriggered= false;
    [SerializeField] Animator npc_anim;
    [SerializeField] Animator npc_Box; // An exclamation mark that fades In or out as per the situation.
    DialogueTrigger trigger;
    Quest quest;
    bool popUp;
    bool questDialogueEnded = false;
    bool openKeyPressed=false;
    public bool PopUp
    {
        get { return popUp; }
        set
        {
            popUp = value;
            npc_Box.SetBool("Pop", popUp);
        }
    }
    // Start is called before the first frame update
    void Start()
    {
        trigger=GetComponent<DialogueTrigger>();
        quest=GetComponent<Quest>();
        if (!(quest == null))
        {
            questNpc = true;
        }
    }
    void Update()
    {
        if (Input.GetKeyDown(KeyCode.E)) // "E" is used to start a dialogue
            openKeyPressed = true;
        else if(Input.GetKeyUp(KeyCode.E))
            openKeyPressed = false;
    }
    private void OnTriggerStay2D(Collider2D collision)
    {
        if (collision.CompareTag("Player"))
        {
            // pop up the dialogue box key.
            PopUp = true;
            if (openKeyPressed&& !dialogueTriggered)
            {
                if (!questNpc)
                {
                    npc_anim.SetBool("isIdle", true);
                    trigger.TriggerDialogue();
                    dialogueTriggered = true;
                }
                else
                {
                    if (!quest.questFinished)
                    {
                        npc_anim.SetBool("isIdle", true);
                        trigger.TriggerDialogue();
                        dialogueTriggered = true;
                    }
                }
            }
        }
    }
    private void OnTriggerExit2D(Collider2D collision)
    {
        if (collision.CompareTag("Player"))
        {
            // pop out the dialogue box key.
            PopUp = false;
            if (questNpc == true && !quest.questFinished && dialogueTriggered)
            {
                quest.PlayQuestCinemtaic();
            }
        }
    }
    private void OnTriggerEnter2D(Collider2D collision)
    {
        if(quest.questFinished&& collision.CompareTag("Player") && !questDialogueEnded)
        {
            quest.EndQuest();
            questDialogueEnded = true;
        }
    }
    public void FinishQuest()
    {
        quest.questFinished=true;
    }
}

For the Quest:

using System.Collections;
using UnityEngine;
using UnityEngine.Playables;

public class Quest : MonoBehaviour
{
    public bool questFinished=false;
    [SerializeField] bool movingNPC=false;
    [SerializeField] GameObject questObject;
    [SerializeField] Dialogue dialogue;
    [SerializeField] DialogueManager manager;
    [SerializeField] PlayableDirector director;
    [SerializeField] string animPara;
    Animator animator;
    public void PlayQuestCinemtaic()
    {
        director.Play();
    }
    public void EndQuest()
    {
        questFinished = true;
        manager.StartDialogue(dialogue);
        if (movingNPC)
        {
            StartCoroutine(MoveNPC());
        }
    }
    // Start is called before the first frame update
    void Start()
    {
        manager=FindObjectOfType<DialogueManager>();
        animator=GetComponent<Animator>();
    }
    IEnumerator MoveNPC()
    {
        yield return new WaitForSecondsRealtime(20f);
        animator.SetBool(animPara, false);
    }
}

2 Answers

2
2

I don’t know what answer you’re look for, but here are my opinions:

[SerializeField] bool questNpc=false;

Don’t assign values to public or [SerializeField] in your code. Unity assigns these values automatically. Either just make them private, or don’t assign the value in the code.

 if (!(quest == null))

You should just use if (quest) , and avoid == operator for Components and GameObjects.

3

This part is a bit confusing you repeat the same lines of code why not use ||
also when coding try to always separate distinct blocs of code, in your case you only have few lines but it’s always better to organize things right from the beginning:

private void OnTriggerStay2D(Collider2D collision) {
   if (collision.CompareTag("Player") == true) {
      PopUp = true;
      this.CheckDialogueOpening();
   }
}

private void CheckDialogueOpening() {
   if (openKeyPressed == true && dialogueTriggered == false) {

      if (questNpc == false || quest.questFinished == false) {
         this.LaunchDialogue();
      }

   }
}

private void LaunchDialogue() {
   npc_anim.SetBool("isIdle", true);
   trigger.TriggerDialogue();
   dialogueTriggered = true;
}

Other than that, try to add more space in your code and avoid using ! it’s always cleaner and easier to read == false