Event function not working as expected

Question

First I will explain whats happening, then what I am expecting to happen, and finally the code behind it

So whats happening is when I press enter the color of the text is green

What I expect to happen is the color turn red

This is based on if i type "Bad" into the field

//Please note I have edited uni9mportant code out

//Event Listener
inputField.onEndEdit.AddListener (delegate {
            VerifyWords();
});

//Clss that handles the dictionary
public abstract class WordDictionary: MonoBehaviour{
    public static Dictionary<string,bool> _wordDictionary = new Dictionary<string,bool> ();

    private void Start(){
        _wordDictionary.Add ("Bad",true);
    }
}

//Function that handles the word verification
private void VerifyWords(){
        if (openChat == false) { //If we done have open chat
            bool hasBadWords = false; //Reset boolean
            string[] stringSplit = inputField.text.Split (' '); //Split text string

            for (int i = 0; i < stringSplit.Length; i++) { // Go through each word in the string array
                if (WordDictionary._wordDictionary.ContainsKey (stringSplit[i])) { //If the word is in the dictionary
                    hasBadWords = true; //Then there is a bad word
                }
            }

            if (hasBadWords == true) { //If a bad word was found
                inputField.textComponent.color = Color.red; //Then the text should be red
            } else {
                inputField.textComponent.color = Color.green; //The text should be green
            }
        }
    }

EDIT I edited the code with comments to kinda put my thinking into perspective

Answer 1

The problem is that the class is marked as abstract. An abstract class cannot be instantiated, and therefore Unity can't call Start on a class that it cannot instantiate. The easiest fix is to simply remove the abstract from the class definition:

public class WordDictionary: MonoBehaviour{
    public static Dictionary<string,bool> _wordDictionary = new Dictionary<string,bool> ();

    private void Start(){
        _wordDictionary.Add ("Bad",true);
    }
}

However you now have a new problem. WordDictionary has a static member that is initialized by a non-static method. This means that each time you create a new WordDictionary, Start() will be called and it will add all the words into the dictionary again (or at least it will attempt to, you will get a duplicate key exception in this case, to avoid that you can also write _wordDictionary["Bad"] = true which replaces an existing key if it exists).

The better option here is to use a static constructor. This will make sure that the dictionary is only initialized once:

public class WordDictionary: MonoBehaviour{
    public static Dictionary<string,bool> _wordDictionary = new Dictionary<string,bool> ();

    static WordDictionary() {
        _wordDictionary.Add ("Bad",true);
    }

    private void Start(){
    }
}

Now you can use WordDictionary without worrying about the dictionary growing each time the class is instantiated. But at this point there really is no use in making WordDictionary a MonoBehavior because really it is just a holder for a bunch of words. So your class now just becomes:

public class WordDictionary: {
    private static Dictionary<string,bool> _wordDictionary = new Dictionary<string,bool> ();

    public static Dictionary<string, bool> Words {
        get { return _wordDictionary; }
    }

    static WordDictionary() {
        _wordDictionary.Add ("Bad",true);
    }
}

I added a property here because really you should be using properties, and having the underscore names (in my code world) means that it is a private field. You can extend your dictionary to do other things instead:

public class WordDictionary: {
    private static List<string> _wordList = new List<string> ();

    static WordDictionary() {
        _wordList.Add ("Bad");
    }

    public static Contains(string word) {
        return _wordList.Contains(word);
    }

    public static ContainsAny(IEnumerable<string> words) {
        return words.Any(w => Contains(w));
    }
}

I don't see any reason to use a Dictionary here, if it contains the word then it is "bad", if it doesn't contain the word then it would be "good". So changing to a list makes things simpler. If you hide how the "Dictionary" works in the background and just expose the "contains" and "contains any" methods, you get two advantages, use becomes simpler, and you can change the underlying "engine" without changing the interface and downstream code.

And now your colorization function becomes much simpler:

private void VerifyWords() {
    if (openChat)
        return;

    var stringSplit = inputField.text.Split(' ');

    if (WordDictionary.ContainsAny(stringSplit))                
        inputField.textComponent.color = Color.red;
    else
        inputField.textComponent.color = Color.green;
}

Answer 2

First I would recomend that you dont use MonoBehaviour for your WordDictionary. I see no reason to do so. Insted use ScriptableObject.

Second it doesn't need to be abstract and I dont think you need a dictionary at all a list would do just fine assuming you only store the 'bad' words.

e.g. (and I used dicitonary here, could be list assuming your requirements would work with only the bad words being stored)

[CreateAssetMenu(menuName = "Custom Object/Word Dictionary")]
public class WordDictionary : ScriptableObject
{
    public Dictionary<string, bool> Values = new Dictionary<string, bool>();
}

Scriptable objects are created in your assets and can be referenced by GameObjects in your scenes. So in gameObjects that need to use the dicitonary just add a

public WordDictionary gameDictionary;

And set it to the dictionary you created in your assets. In general singletons/static classes and similar approches of making a monoBehaviour act like a static object leads to problems.

Scriptable Objects are a great workaround. They are available at start of game without initalization like a singleton, they can contain data or funcitons like any other class but unlike MonoBehaviour they do not have Start, Update, etc. ... doesn't mean you can call a AtStart() you write in the Scriptable object from some specific initalization behaviour in your game however.

1 important note, in editor the data updated in a scriptable object persists, at runtime it does not. e.g. when testing your game in editor the 'bad' words will be retained between sessions ... in build it will not but I assume you are preparing your dictionary on initalization of the game anyway so this should be a non-issue.