I'd be tempted to make mudlet::playSound(...) return a boolean indicating whether that was successful; however given that we do not test for the sound file's actual existence (actually IIRC it could be a remote resource on the Internet so that is a whole different kettle of worms!) I'd fall back to issuing an unconditional:

lua_pushboolean( L, true );
return 1;

instead of nothing with the following:

return 0;

because returning less arguments then the user has coded for will give them a nil for the missing value(s). If the end user has got into the habit of checking for a nil + error message for runtime value errors this means they will get a nil + nil pair of values on success (as far as this function believes) instead of true + nil. On the other hand I think rain is wet so who am I to say...

I'm not sure if QMediaPlayer::Error would have populated yet since the return-to-lua would occur the next line immediately following the playSound request. It would likely still be in the process of finding/opening the sound file/ codecs.

So just change the return on the command to a 'true' instead of nothing?

Yep. In most cases the user/scripter won't pay any attention, but if you are using the command from the "command line" it is nice (IMHO) to get confirmation that the command went through Okay. It also work better (also IMHO) if you ARE checking for errors by checking for NOT a nil + error message because the absence of any wanted return value from a Llua command is a nil or at least that is what the variable waiting for a value gets. e.g. :

function someRandomFunction()
-- stuff
local x = 1
local y = "two"
-- stuff that doesn't change x or y
return x,y
end

then if we use this:

local x,y,z = someRandomFunction()

then x will be 1, y will be "two" and z will be nil as I understand Lua {which is not as good as some others 😉} and getting both a nil when things work and a nil when they don't is a little user unfriendly 😈.

As for a failure to find/play the sound file given; that might be a suitable case for a new "sysSoundEvent" TEvent type which might include other "callback" type features such as:

• notification of reaching the end (or some %age point in the sound playback) of a media file
• a time-out related change in the number of QMediaPlayers if we decide to cull them {as this series of commits only ever increases them with no ability to get rid of them if not needed}
• notifications of QMediaPlayer errors that arise after the request was made to play a file

These are just outline thoughts and not something for immediate action, more likely the "let's think about this at some idle moment" pile...

This should go into the errors view.

🐛 Also, a leading new-line will break the code that detects the type of message that was posted and applies the colour coding to it...

This error should actually never be displayed, it's really only there for a just in case thing. I'd had to reverse the if statement to test it, at which point it printed out at the end of the line that triggered the error. That's why I added the leading newline, though I will remove it. This also wouldn't be an issue though it was going to the correct window (errors, not pH). I'll take a look, but could someone post a snippet demonstrating use of the the errors window?

Take a look though the map related classs e.g. TMap, TRoomDB, TArea, TRoom {which is an area where I have been particularly active 😊 } besides which, as you have poked around in the cTelnet::postMessage(...) you might see that the code looks for the "[ something ] - " tag on the first line. FTR once the code detects this type of string the code aligns all following lines to the same indentation as the position of the first character after the ending "... - " of the prefix so I try to ensure that all messages have the same width in the portion "[ something ] - " so that they all align on screen in the console; you may notice the in source code formatting of such messages try to replicate this alignment. I also try to avoid individual message lines including indentation to be over 80 characters so that they don't trip the line wrapping that the main console text display stuff does. This doesn't always work - especially when you have %1...%10 type variable replacements by strings...

Also applies/ed to #359.

Might I suggest this replacement for the above that better fits our current "standards" for Lua commands with better error reporting?

int TLuaInterpreter::playSoundFile( lua_State * L )
{
    Host * pHost = TLuaInterpreter::luaInterpreterMap[L];
    if( ! pHost ) {
        lua_pushnil( L );
        lua_pushstring( L, tr( "playSoundFile: NULL Host pointer - something is wrong!" )
                        .toUtf8().constData() );
        return 2;
    }

    QString sound;
    if( ! lua_isstring( L, 1 ) ) {
        lua_pushstring( L, tr( "playSoundFile: bad argument #1 type (sound pathAndFilename as string expected,\n"
                               "got %1!)" )
                        .arg( luaL_typename( L, 1) )
                        .toUtf8().constData() );
        lua_error( L );
        return 1;
    }
    else {
        // Windows platforms, at least, can have non-ASCII characters in path/filenames:
        sound = QString::fromUtf8( lua_tostring( L, 1 );
 
        if( sound.isEmpty() {
            lua_pushnil( L );
            lua_pushstring( L, tr( "playSoundFile: bad argument #1 value (sound pathAndFilename cannot be empty.)" )
                            .toUtf8().constData() );
            return 2;
        }

#ifdef Q_OS_WIN32
        sound.replace( QLatin1Char( '\\' ), QLatin1Char( '/' ) );
#endif
    }

    qint64 soundVolume = 100;
    if( lua_gettop( L ) > 1 ) {
        if( ! lua_isnumber( L, 2 ) {
            lua_pushstring( L, tr( "playSoundFile: bad argument #2 type (sound volume percentage as number is\n"
                                   "optional {default = 100 percent if ommitted}, got %1!)"
                            .arg( luaL_typename( L, 2 ) )
                            .toUtf8().constData() );
            lua_error( L );
            return 1;
        }

        soundVolume = lua_tointeger( L, 2 );
        if( soundVolume < 0 || soundVolume > 100 ) {
            lua_pushnil( L );
            lua_pushstring( L, tr( "setAreaName: bad argument #2 value (sound volume %1 is not in range"
                                   "{0 = silent; 100 = full, default if omitted}.)" )
                            .arg( soundVolume )
                            .toUtf8().constData() );
            return 2;
        }
    }

    mudlet::self()->playSound( sound, soundVolume );
    lua_pushboolean( L, 1 );
    return 1;
}