adv3Lite Library bug (?)

This is weird. I’ve created an Achievement object called findEggAch. The following code, while unnecessarily convoluted, works as one would expect:

initSpecialDesc = "Among the branches of the hedge you notice a bird's nest.<<foo()>> " foo() { findEggAch.awardPointsOnce(); }
When the initSpecialDesc is printed, the points are awarded. However, the more straightforward version fails:

initSpecialDesc = "Among the branches of the hedge you notice a bird's nest.<<findEggAch.awardPointsOnce()>> "

This produces a runtime error with the message “String value required.” The error is in output.t, in the quoteFilter object, on the line

quoteRes = rexSearch(quotePat, txt);

At the point when quoteFilter is invoked, how can it possibly make any difference whether it’s processing foo() or findEggAch.awardPointsOnce()? It’s not the dot operator that’s tripping up the output filter; if I put foo() in some other object and invoke it via <<someOtherObject.foo()>>, there’s no runtime error.

I’m using adv3Lite 1.4 and TADS 3.1.3, the most current versions.

Probably because findEggAch.awardPointsOnce() returns true, which is erroneously feed into the rexSearch as opposite to foo() returning nothing. But you should write Eric about that, it would be better he fixes the library as he knows the whole architecture, I rather won’t speculate which place is the right to fix this.

Thanks, Tomas. You’re correct. When I modify Achievement by getting rid of the word ‘return’ in awardPointsOnce(), the error disappears. Ideally, quoteFilter should be able to handle this situation, but I wouldn’t attempt to modify it. If Eric doesn’t notice this thread for a few days, I’ll email him.

I’ve seen the thread so there’s no need to email me. It may take a while till I get to looking at the problem, though.

I haven’t had a chance to look at this yet, but thinking about it I’m not sure it’s exactly a library bug. The statement:

initSpecialDesc = "Among the branches of the hedge you notice a bird's nest.<<findEggAch.awardPointsOnce()>> "

Is roughly equivalent to:

initSpecialDesc()
{
    say('Among the branches of the hedge you notice a bird's nest.' + findEggAch.awardPointsOnce()
}

My guess is that the txt parameter is being passed to the relevant routing in quoteFilter as ‘Among the branches of the hedge you notice a bird’s nest.’ + findEggAch.awardPointsOnce(), and that the routine fails where it does because this is the first time this string is evaluated. Since findEggAch.awardPointsOnce() evaluates to true, the string value is then effectively:

  'Among the branches of the hedge you notice a bird's nest.' + true

Which, I suspect, is a type mismatch error. You should be able to test this by seeing whether a statement of the form:

  foo = 'bar ' + true;

Throws the same run-time error. If it does, this is a language feature, not a library bug (there’s nothing I could do in quoteFilter to change it).

It’s another matter, which I’ll have to look into later, whether awardPointsOnce() needs to return true, but that doesn’t alter the underlying issue that you may just have to be a bit careful using embedded expressions in strings where the embedded expression evaluates to something that can’t be concatenated to a string.

I should point out that I haven’t actually tested this; I just thought I’d point it out now in case you wanted to test it yourself before I get round to looking at it.

Hm, when you do 'bar ’ + true, then the true is converted into text, so it reads ‘bar true’. Problem is that awardPointsOnce returns only the bool value true, so the conversion is not triggered before it passes to output filter. (And say('bar ’ + nil) says nothing at all!)

Actually I was quite sure that this is a bug and that it is desirable to convert true returned by embeded expression into empty string and not display anything, because it occured to me that embedding some true/nil returning expressions can be quite common and really doesn’t make sense converting it to text. But now I’ve checked that adv3 behaves the same (although the runtime error is in other part of the library). But hey, wouldn’t it to be a nice feature? I’ve been quite surprised that I didn’t yet tried to award points in an embeded expression myself, I was under impression it is perfectly legal.

The function awardPointsOnce probably needs to return true/nil because it is signaling whatever the points was awarded or it was called subsequently and no points was awarded. The solution would need to be on more general level, then changing awardPoints once nor changing the quotes output filter itself. There are places, such as switch in OutputStream::writeToStream, where is a logic involving how to convert different types of variables into string. Something similar would need to be on a place where embeded expressions are called and their return value placed into the string. But I don’t know where it is.

edit: On a second thought, embedded expressions are probably really in the core language and not in the library :-/

Converting a true value to a string wouldn’t be desirable in this particular case. One wants the return value to be thrown away. In many other cases, of course, the return value will be a text string that one specifically doesn’t want thrown away.

One hacky solution would be a flag in gameMain – something like returnTrueToOutput = nil. The user could, if necessary, set this flag to true before some particular usage and then set it back to nil immediately afterward. AwardPointsOnce() would either return true or not, depending on the flag. This might be tricky, however, as the compiler would probably complain (over and over) that one branch of a method returns no value. We don’t want that as a side effect. A better option would be to add an awardPointsOnceNoReturn() method. Extra typing, but it would eliminate the problem without side effects.

How many embedded functions might need this second version? And is there a way that you could write just ONE noReturn function and then <<noReturn(&someObj.awardPointsOnce())>>? I’m not enough of a programmer to code that, but I’ll bet it could be done.

We can always fill a bug to Mike, rather than inventing hacks. I still thing it is reasonable to expect bool return values (as opposed to string return values) from an embedded expression to be thrown away, especially when the behaviour is not symmetrical, i.e. returning nil produces empty string. TADS is full of little tweaks to make programming IF a joy and this would be in the line of making programming easier and more expectable.

Bool returns are typically statuses whatever something was successful or not directed to the programmer and not content directed to the player. I don’t know for how many functions in the library it is relevant but user can make his own functions with bool returns and use them in embedded expressions.

Just to be more precise the filter chain is called per partes, ie. first time on text before expression, then on the output of the expression, then on the rest of the string. It’s not concatenated together before filtering.

A bit of experimenting suggests there may be a way to fix this in the adv3Lite library. In the output.t file change the definition of the say() function to:

say(val)
{
    /* 
     *   Use the dmsg() function to pass a string value through the message
     *   substitution parameter filter, otherwise output the value directly.
     */
    if(dataType(val) == TypeSString)   
    {       
        dmsg(val);           
    }
    else if(dataType(val) == TypeTrue)
        ; // do nothing
    else
        oSay(val);

}

This will make attempts to say(true) do nothing.

That seems to work perfectly – thanks! I edited output.t directly, since I’m not sure how to modify a function.

I’ve now uploaded a fixed version to GitHub. In the end I decided on a slightly different fix, putting it in the OutputStream object rather than the say() function, since this is more consistent with the way the library handles a nil argument. When I get the chance I’ll also carry out an improved version of the PUT ALL IN XX fix and push that up to GitHub too.

Just like a class with replace keyword: http://www.tads.org/t3doc/doc/sysman/proccode.htm#funcModRep