#6727 KYRA (LOL): Engine assertion at various dialogues if no items in inventory

If you have no items in your inventory when you speak to King Richard, as he attempts to give you his private library key, the game will crash. (Note attached screenshot)

Steps to reproduce:
Start the game with any character (I chose Conrad).
Go and see Victor and sell the 3 items you start with.
See the king and halfway through the dialog the game will crash.

ScummVM Versions tried: 1.70 & 1.80git707-g556414e
Language: English
Game Version: CD (1.02)
Platform: Win32

[Willem Jan Palenstijn]

For convenience: the screenshot shows:

Assertion failed: pos > 0 && pos < 48, file engines/kyra/items_lol.cpp, line 268

Additional information : This problem will also occur with Geron Arbroath (Royal Chamberlain) when he attempts to give you the writ before you leave the castle.

Only known workaround is to have at least 1 item in your inventory before initiating either of the above conversations.
I was able to take my equipped weapon out for this task.

[digitall]

Looking at the code, this looks to be an issue in the inventory position slot insertion logic in bool LoLEngine::addItemToInventory(Item itemIndex) which is where the failed assertion is:
https://github.com/scummvm/scummvm/blob/master/engines/kyra/items_lol.cpp#L268

Based on looking at the code in this function, this can only occur if the value of _inventoryCurItem is negative or far outside the normal 0 to 48 range at the top end. In this case, the likely value of _inventory[pos] will read outside the buffer, get zero and then continue with a bad value of pos to hit the assertion:
https://github.com/scummvm/scummvm/blob/master/engines/kyra/items_lol.cpp#L250

The code earlier in that function should be hardened and assertions added at the start... then someone needs to trace back and find out when and why the bad _inventoryCurItem gets set.

[digitall]

Replicated here on Linux x86_64, by dropping all three items at start of game and then going to Throne Room.

Will debug.

[digitall]

Hmm.. Added debug output to this function:
LoLEngine::addItemToInventory(Item itemIndex:11)
_inventoryCurItem:0
pos:0
scummvm: engines/kyra/items_lol.cpp:273: bool Kyra::LoLEngine::addItemToInventory(Kyra::Item): Assertion `pos > 0 && pos < 48' failed.

[digitall]

AH. Have worked out the error. The assertion is wrong. It should limit to the range of inventory positions which are 0 to 47, but it is limiting to 1 to 47!!

Will amend and test.

[digitall]

Fixed by 261d15d007d83e3087699068e3f46a8f250dd757 and tested.

This is now in master, so the bug submitter should test with the next nightly build.

Closing as fixed.