Programming Project Apollo - NASSP

One of the goals for Project Apollo was that a flight could be saved at any time and reloaded and you could continue the scenario without major problems. That puts some limitations on how you write your code.

Firstly, wherever possible it should be state-based rather than flag-based. The launch vehicle code has a stage variable and a stage state variable, and that controls how it functions during the launch.

If you do add a new class variable, set it to a sane default in the constructor. Even if it's not really needed, getting into the habit of doing so will help to eliminate random bugs due to uninitialised variables.

Another thing you need to do is figure out whether the variable really needs to be saved or can just be reset to a sane default next time. For example, if you need to do something once a second but it doesn't matter _when_ during that second, you can just reset the timer you use to track the last timestep you ran the program to a sane default and it will continue running afterwards. Equally, if a variable is only needed in the context of one time-step in the program, make it a local variable on the stack, not a 'global' variable in the class.

Try not to run your program on every timestep: it's better to run at a fixed rate, say ten times a second. Firstly, you're probably wasting CPU time running 100 times a second if you run every timestep, secondly it helps to ensure that your program will work fine with low frame-rates or time acceleration. If you run happily ten times a second that means you can run at 10fps with 1x time acceleration or 100fps at 10x time acceleration with no problems. There are variables in the code already allowing you to keep track of the next time you need to run, just re-use them.

In the main code for the Saturn 1b and Saturn V there are a lot of flags, and most of them are saved in a few state variables. Basically they're big bitfield structures and the flags are packed into the structure and then read out as 32-bit integers. You should still ensure that flags are set to true or false in the initialisation code before being overridden by scenario values, since VC6 seems to use 8-bit variables for 'bool' and get confused at times if the value in there is anything other than 1 or 0 for true and false.

There's now no need to check that sounds are valid before playing them: if you call sound.play() and haven't previously loaded the sound file it will just ignore the call. Some code still calls sound.isValid() first, but that's just due to a quick translation of old code to the new interface and can be deleted. The only real reason to do that is for debugging to check that the sound is loaded, or for dynamic sound loading: since OrbiterSound loads the sound on the timestep after you request it, you can't load and immediately play... the simple solution is to check that the sound is valid, if not then load it and continue, if it is valid then play it. That way, on the next timestep it will be valid and will be played.

With hindsight, that doesn't appear to work anymore: I tried doing that in the LEVA code, but the sound wouldn't play the timestep after it was loaded. I ended up having to load it earlier to get it to play.

It's perfectly safe to load a sound multiple times, since the soundlib code will check whether a sound of the same filename is loaded and re-use it. However, you need to remember to call sound.done() when you're finished so that it will be unloaded if no-one else is using it. You can safely call done() when the sound is still playing, it will continue to the end then be unloaded (at least it will if the soundlib code is working properly since the rewrite!).

If you can do something sensible if the handle doesn't exist, check that handles are non-zero before using them: Orbiter seems to die if passed a zero handle. In some cases (e.g. you create a new vessel and get a zero handle back because the creation failed), there's not much you can do other than crash.

Wherever possible, explicit checks of switch states should only be in the panel and systems code: the main code should call functions to check whether systems are working. That way we don't need to revise lots of code if the systems simulation becomes more sophisticated, just the low-level functions that tell us what's working or enabled. Similarly, arcane variables like ENGIND[7] should really be replaced by functions with names that humans understand that set and get their values, or by more understandable variable names... I made a start on that with engine indicators, but didn't fix all of them as I'm not sure what some of them do.

Minimise use of pointers: the less pointers we have, the less NULL pointers we can try to use and cause a CTD. In some cases that can't be avoided, because we have to call Orbiter functions that give us pointers, but in most cases it can. Even when you have to pass a 'pointer' to a function, try to make it a reference instead so the lower-level code can't accidentally trash the pointer. As with the AGC and DSKY, even if you might need to pass a pointer to a different class, you can sometimes get around it by declaring a reference in the other class and initialising it in the constructor (it can't be changed after the constructor is called).

Use strncpy() and snprintf() rather than strcpy() and sprintf() to avoid buffer overflows. It's unlikely that any hackers would use an Orbiter scenario to try to take over your PC by overwriting the stack, but a badly-edited scenario could still make Orbiter crash if it overflows by accident!

Use Doxygen to document code as you change it. A fair portion of the code is documented that way already, but much still needs work.