Avoid out of bounds access when event is referencing non-existent unit

[crumblingstatue]

Fixes #122

Current approach is to just bail out if we detect that the unit doesn't exist.
We might want to do something more graceful, like at least show an error message, or maybe
even create a dummy unit.

But I'm not sure which approach to take.

[yuxshao]

Thanks for such a quick PR!

If it's ok, I might prefer if we check that the whole project is valid on load. If the project's invalid, either the editor could bail or it could sanitize whatever's invalid (e.g. here it could assign unit no 0). IIRC the pxtone source code has a place on load where it does some such checks already.

The reason is, there's probably implicit validity conditions that different parts of the editor rely on to work. Here, you've found a unit # may be too big, but elsewhere maybe something could be negative (maybe it's unsigned so not possible, I don't remember the source sadly), or a voice no could also be too big and trigger an OOB access as soon as the song's played.

To me, consolidating the checks in one place and then relying on the song to be valid seems more sustainable than adding extra logic for each check at each point the value is used, esp since I don't think these invalid project states are reachable (barring engine bugs) in normal editor workflows. But LMK if you disagree!

[crumblingstatue]

Yes, I agree.
Do you think that a sanity_check() function that runs right after the project has loaded would be appropriate?
If it fails, then we discard the project we have just loaded.

[yuxshao]

(Finally at my computer!) Ah, looking a bit thru the code, this is the place that loads a project. It's inherited from the pxtone engine source code and has a few checks in place already. Perhaps the best place is to add appropriate checks when loading events that the referenced unit exists then?