Querying non-existent characters yields `denied` where I would expect `unknown`

[phylll]

Using this code in our test game, where only two of the character sheets exist under these exact names, yields surprising results:

!mmm script
!mmm   for charName in m3mgdPlayerCharacters
!mmm     debug do charName, charName.token_id, isunknown(charName.token_id), isdenied(charName.token_id), getreason(charName.token_id)
!mmm   end for
!mmm end script

m3mgdPlayerCharacters is a game-global list of our six plain text character names.

The result tells us that characters whose sheets exist in the game but who are currently not represented by a token on the board are reported as unknown (which is what I'd expect) while character names that do not match a character sheet at all are reported as NOT unknown and denied. I would expect non-existent characters to be equally unknown.

[michael-buschbeck]

It's by design that querying an attribute of an object that doesn't exist gives you the same response as querying an attribute you don't have access to.

Otherwise there'd be an information leak:

• unknown means "thing doesn't exist"
• denied means "thing exists (but you shouldn't be able to see it)"

Arguably, it wouldn't hurt if someone with GM privileges who has blanket permission to access everything could distinguish "doesn't exist" from "can't access" – then again, since a GM won't ever hit "can't access", denied can only ever mean "doesn't exist" for them.

I didn't think to special-case GMs in that regard. I'm torn as to whether it's more or less confusing to have attribute queries always return unknown instead of denied if done by a GM even though they'll always return denied and never unknown for non-GMs – but you're a better judge for that, so I'd be interested in your opinion on that in light of the discussion above.

[phylll]

Hmm. I hadn't understood these subtleties so far, since I probably thought too intuitively about it rather than explicitly in terms of the underlying data/security model.

Now, denied as "you can't see this, whether or not it exists" makes sense for the case of querying an attribute of an object that does not exist. And the reason I don't get unknown, as well, is that something can only ever have one value, so you can't assign both? While I got a little lost trying to think this through, my original surprise was not with getting denied but with NOT getting unknown since it would be even more intuitive to expect a non-existent thing to be unknown :)

[michael-buschbeck]

I could certainly make isunknown() return true for denied values as well – but I think that would muddy the distinction between isunknown() and isdenied().

Arguably though anything that's denied is also unknown, so… not sure. (Strictly speaking it'd be a compatibility-breaking change though; not to say we can't do it.)

[phylll]

Before we consider a compatibility-breaking change that won't make everything super-easy, anyway, maybe you can help me improve how I'm handling my standard use case. What happens in every little piece of code is that I'm trying to query some token or character attribute, and there is some risk of the token id or attribute name to be wrong or inaccessible to the current user, so I need to cover for that.

!mmm     set targetID = weaponWielderID.(m3mgdEarthBondPCAttrTargetID)

Assuming that weaponWielderID refers to an accessible token or character sheet, how do I best/most efficiently make sure that

1. weaponWielderID.(m3mgdEarthBondPCAttrTargetID) is the actual content of that attribute (i.e. the attribute is present and accessible, even if that content may be "") rather than some kind of error object or undefined value that will look ugly ([object] object) when displayed & may kill the sandbox when given to setattr(), and
2. it (and thus, targetID) refers to an accessible token or character sheet (with some other attribute I need also being accessible to me)?

I'm pretty sure that this is not all necessary, but various troubles and weird bugs led me to a place where my check has gotten pretty bloated and unspecific:

!mmm     if isunknown(targetID) or isdenied(targetID) or isdenied(targetID.token_id) or isunknown(targetID.token_id) or isdenied(targetID.(m3mgdEarthBondStatusMarker))
!mmm       do whisperback("Target token '" & targetID & "' not a token ID, or does not allow access to attributes:" & getreason(targetID))
!mmm       return false
!mmm     end if

[michael-buschbeck]

I see. Makes sense that you'd like to fail fast rather than have the script soldier on, bushwhacking its way through nonsense.

Several thoughts based on your example:

• If you pass denied or unknown on either side of the . lookup operator, they propagate – unknown.foo will get you the original unknown including diagnostics you passed in. So isunknown(foo) or isunknown(foo.bar) can be safely abbreviated as isunknown(foo.bar).

• Maybe obvious, but: You can (now) write your own utility function to cover isunknown(foo) or isdenied(foo). I'll have a go at a custom isvalue(foo) function that also covers undef below.

• You could, similarly, write a utility function that passes though its argument when it's all fine and stops the script when not, and wrap any expression you can't abide to return a half-hearted unknown or denied in a call to that function.

• Would you be interested in having a function you can call to raise a runtime exception (with custom message)?

• Sandbox crashing is a bug and it's disgraceful that it still persists through several feature releases. I'll have a harsh word with the cat on that.

• [object Object] appearing anywhere user-facing is also a bug. I'm aware of that happening in debug outputs but not anywhere else yet – is this something you can reproduce?

Here's my attempt at an isvalue() function that returns false for unknown, denied, and undef, but true for anything else (including "" and false and 0):

!mmm function isvalue(value)
!mmm     for item in value
!mmm         if not isunknown(item) and not isdenied(item)
!mmm             return true
!mmm         end if
!mmm     end for
!mmm     return false
!mmm end function

This takes advantage of the fact that undef is regarded as an empty list so the for loop will never be entered if value is undef. It will also return false if a list of several unknown etc. is given – seems fine by me, but if it's not, one could add a counter and only return false if there's at most one item.

(And, yes, if it's useful I can easily make this into a built-in function, of course.)

[phylll]

Thank you! This is very helpful. I'll think through the variations of my use cases and as you say, with my improved understanding of what's happening under the hood, should be able to write some simple utility functions to serve my purpose. The passthrough validator function you mention is something I've built for a very specific use case already, and have been thinking about expanding.

What would a runtime exception do? whisperback(errormsg) + exit script?

Re: [object Object], to be honest, I don't remember. I'll keep an eye out and I'll file a bug report if it comes up. Maybe I'm mixing it up with another [something something] representation of an unexpected value, like those we get when translate tries to access a value that chat [label] hasn't provided.

[michael-buschbeck]

What would a runtime exception do? whisperback(errormsg) + exit script?

Essentially, yes, and now that you're putting it that way it doesn't seem all that useful after all.

Maybe in connection with a try … catch … end try block?

[michael-buschbeck]

What would a runtime exception do? whisperback(errormsg) + exit script?

Essentially, yes, and now that you're putting it that way it doesn't seem all that useful after all.

Well, one important difference: While exit script probably isn't a parse-time error right now (due to an oversight of mine), it won't work from inside a function – it'll work like exit function.

Throwing an exception would exit the script regardless of where the exception is thrown (unless it's caught).

[phylll]

While exit script probably isn't a parse-time error right now

Indeed, it isn't, just checked. Simply leaves the function. Just FYI.

Otherwise, I'm still mulling if try...catch and throw would be worth the effort. I've never worked with exceptions in the old days and so I'm not sure of what it would mean to use that feature properly. I guess I could get rid of constantly checking every relevant return value of any function in which something might have gone wrong, and rather just write a sequence of how things are supposed to happen if each of my functions did internally catch its errors and kill the entire script in case something major went wrong?

[michael-buschbeck]

Otherwise, I'm still mulling if try...catch and throw would be worth the effort. I've never worked with exceptions in the old days and so I'm not sure of what it would mean to use that feature properly. I guess I could get rid of constantly checking every relevant return value of any function in which something might have gone wrong, and rather just write a sequence of how things are supposed to happen if each of my functions did internally catch its errors and kill the entire script in case something major went wrong?

Essentially, yes – though it's up to you whether anything going wrong kills the entire script or just does something else (e.g. skip the troublesome item; output an apology; try a different approach). As you say, it mostly relieves you from having to interleave your "happy path" code with your "something may have gone wrong" code.

MMM doesn't throw exceptions in and of itself and I'm not planning to change that. There's currently very little that can cause a script to crash at runtime. (Attempting to call undefined functions is one of those few things.)

However, I'd add a throw function or command that allows scripts to throw any kind of value they like to the nearest catch block up the call stack. If e.g. your happy path doesn't want to deal with unknown or denied or undef values, you could write your own required(val) function that you'd use like set foo = required(sender.foo) and that simply throws an error if its argument isn't valid per your definition of "valid".

Something else I forgot to mention: Support for a try block would typically also come with support for a finally section – code in that section is guaranteed to be run after the try block regardless of how the try block is exited – after normal operation, or through an exit command, or because an error or an exception was thrown. Very useful for cleanup code; I'm not sure if our scripts have any need for it though.