Test bad event code for crashes

[mateofio]

For every possible event command, we need to test it for bad data. Things like #1719

For each event command, try to find a way to have it use invalid data. Player should either ignore it if its benign, or report an error using Output::Error().

This will be more important when people are using Player to develop games, as bad event code should report errors instead of crashing.

[Ghabry]

When I added sanity checks to all the data coming from LCF files I ignored event script on purpose because it was too much work to handle this but should happen someday.

Don't use Output::Error for this. Errors are extremely annoying for the user because the game terminates without saving. When you look through the code you will see that any recoverable Error is a Warning.
You will also notice that some Warnings must be later changed to a Debug because RPG_RT accepts some bad values silently without giving an error.

[mateofio]

Don't use Output::Error for this. Errors are extremely annoying for the user because the game terminates without saving. When you look through the code you will see that any recoverable Error is a Warning.

I'm not sure about this. Take #1719 for example. Here RPG_RT will exit with a dialog popup. This is because its a bug in the game's event code. Do we really want to just silently skip the CallChangeName command, log a warning (which the user will never check), and continue on?

In cases like this, we could end up in a state of bad event code where instead of the game crashing without allowing the user to save, the user saves and their save is corrupted.

It's also not our fault. The error indicates a fatal bug in the game, not Player.

I think it's definitely a case by case basis. There are some other situations where you could just warn and ignore without consequences.

[Ghabry]

Check missing files, invalid event calls, invalid event page calls, invalid skills, invalid items, invalid actor references: These are all cases where RPG_RT terminates but we handle all of them with a warning.
What makes this "invalid actor ID" warning different to all the others?

[mateofio]

Lets imagine some scenarios:

1. A simple and optional action event with a bug. This event is provides an optional feature and is not critical for game progression (such as change hero name). The player goes and talks to the event, the buggy command is skipped and the event just doesn't work. Then the player saves their game and continues on. This is maybe ok.

2. A main story event with a bug. The player talks to a key NPC, chooses some options (or happens to have some preconditions) which triggers the bug. The bug is ignored and we finish the event without setting all the proper conditions to continue the game, but we do manage to enable a switch to disable talking to this event again. The player saves, and then find out they are stuck and cannot continue the game. They are forced to start over from beginning.

3. A very complex game with lots of carefully orchestrated parallel and autostart events. A bug goes ignored and everything gets in a bad state. It's like a C++ program corrupting memory and instead of crashing at the site of the bug you crash later in some unrelated place. Or we get the same result as (2).

One more thing to keep in mind. Plugging these and removing player crashes will likely result in a big reduction of android crash reports.

[mateofio]

Leaving a note to remember #1707 Test cases 13 for this one.