Deleting user vars by setting their value to None is intended behavior (otherwise there's no way to delete a single variable, you'd have to use clear_uservars()). What's the specific problem you're running into?

I tried to solve the bug #35 here. When the user variable value is None, should it stays there for the call i.e. last_match return None rather than undefined? Maybe I am not clear about your use case deleting user var. Could you explain further?

It's a little complicated because I want a user var to return the string "undefined" when called in RiveScript code, e.g. <get name> should return "undefined" if that variable isn't set for the user. But, from your Python code, functions like get_uservar(), there should be a way to differentiate whether the user is missing the variable entirely, or the variable was literally set to the word "undefined". (Note that get_uservar() currently does return "undefined" when the variable doesn't exist, but that should probably be fixed as well).

The problem started when I moved the session management code into its own module, where the same logic was being called both for <get> tags as well as the API functions. The session manager is what returns the string "undefined" when a variable doesn't exist, and the API functions can't tell whether the variable really doesn't exist or not.

Probably the best place to fix it would be in the session manager around here. The function could accept a new default parameter (to set your own "default value" when the variable doesn't exist), and do something like:

    def get(self, username, key, default="undefined"):
        if not username in self._users:
            return None
        return self._users[username].get(key, default)

This will preserve the existing behavior when a <get> tag is called, because the code won't provide a default parameter and therefore use the word "undefined" by default. But then the functions like get_uservar() and last_match() can be updated to set default=None.

Changing the rivescript.rivescript.get_uservar() function to do this should fix both things at the same time:

- return self._session.get(user, name)
+ return self._session.get(user, name, None)

The last_match() function does return self.get_uservar(user, "__lastmatch__") which would automatically benefit from the code change to get_uservar().

On an unrelated side note on deleting user vars: some people have a need to delete temporary user variables once they're done with them. Calling rs.set_uservar(user, "name", None) makes it delete that variable from memory altogether rather than just set its value to "undefined".

It's supposed to also be the case that you can do <set name=undefined> in RiveScript code and it should delete the variable too, but I don't think that currently happens. You end up with the variable name containing the literal string "undefined", which works for all intents and purposes (i.e. comparisons checking if <get name> == undefined work as expected), but under the hood the variable wasn't technically deleted.

Thanks for your detailed explanation of the design. Your idea to create a default parameter seems promising to solve this. But I need to check how to use this new set up in the existing code.

In my understanding, except last_match in #35 has only 2 possible types of value: either None or a string value of the last trigger; for all other user vars, they have 3 possible types of value:

1. "undefined" means they are not yet initialised (i.e. name undefined).

2. None has two meanings, either the key does not exist, or there is no value to show (i.e. history = None meaning that there is no history of the discourse yet).

3. A "usual" value of the variable (i.e. age == 5 or name == Alice)

If so, according to your explanation, there are two additional meaning of these terms.

1. "undefined" can also mean a "usual" value (overlapping with the 3rd use above) when variable was literally set to the word "undefined". As you said, API functions can't tell whether the variable really doesn't exist or not. Do we aim to solve this or accept this as a limitation of the API?

2. When a usual-valued variable is set to None, it means a deletion of the variable or subset of variables. Otherwise, we'd have to delete by clear_uservars(). Is this case the basis of your use of .pop(key,None) in sessions.py? After deletion, which is expected when the function get_uservar() calls that variable, None or "undefined"?

Your proposal of changing the get_uservars at the rivescript code would replace all "undefined" to None. That breaks our design above, and the test_set_uservars fails because it expects "undefined" at some of the tests i.e. get an unitialized name.

Changing the rivescript.rivescript.get_uservar() function to do this should fix both things at the same time:
- return self._session.get(user, name)
+ return self._session.get(user, name, None)

After writing a very long discussion, I realise that unit test suite will be much clearer to specify your design requirement of the final session manager. If I interpret the design, write the tests and fix the code at the same time, the tests may be self-biased. Could you please help to write a test suite for session manager?

Good points.

The root problem is the last_match() function, so we could probably just focus on that for now. It should return None when there was no match; if it returns "undefined" that could mean they matched a trigger named + undefined and you wouldn't be able to tell for sure. That's admittedly an edge case (who's going to have a + undefined in their bot?) but should still be taken into account.

I still like having the default parameter in the session manager, but then the implementation of last_match could be updated to be like this:

def last_match(self, username):
    return self._session.get(username, "__lastmatch__", None)

So calling rs.get_uservar() would have the same behavior wrt undefined variables by returning the string "undefined" when the variable doesn't exist, like it does in <get name> in RiveScript. This is probably the least surprising behavior for the end user.

In case the developer wants to know whether a variable actually is set, a new function could be added like

def uservar_exists(self, username, name):
    return self._session.get(username, name, None) != None

You don't have to worry about that for this PR, though.

tl;dr. suggested changes:

• In rivescript.sessions implement the default parameter for get() as mentioned above.
• In rivescript.rivescript.last_match(), have it go directly to the session manager instead of get_uservar and send the default=None param.

#63 fixed this instead.