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. The AGC code has a program variable and a program stage variable, and again that controls most of the functions of the program. Those variables are saved and loaded for you automatically, so if you stick to using them rather than your own special variables there's nothing extra you need to do.
Equally, if you really do need some flags, there are ProgFlag01 to ProgFlag04 and a couple of other flags which are saved and loaded automatically. There are also a bunch of 'real' AGC flag words which you can use if you're simulating the appropriate programs: the intention was that you could use the memory editing functions in the AGC to change the operation of programs the same way that the astronauts sometimes had to for the real AGC.
Since variables are only valid in the context of one program, if someone else already has a variable similar to one you want to add, just re-use theirs. There's no point having, say, five different PitchRate variables when only one is required! If it's a variable that may be generally useful, add it to apolloguidance.h and set it to zero in apolloguidance.cpp, otherwise add it to the CSM or LEM code specifically. It's also probably a good idea to zero it after you finish the program to save cluttering up the scenario file with variables that aren't being used.
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.
If you do add another variable that has to be saved, you need to add it to the 'erasable memory' functions to get it saved and loaded in the scenario file. At the moment shared variables from apolloguidance.cpp are in locations 00 (octal) to 67 (octal), while variables specific to the CSM or LEM are in 70 (octal) upwards. Remember that they are octal addresses, so you need to put a 0 in front of them and 0 follows 7, not 9! The original idea was that we could use the same variables and locations as the real AGC, so that data could be shared between yaagc and the C++ AGC, but that didn't work out. There is still a benefit in being able to debug your programs by viewing the 'erasable memory' version of your variables from the DSKY, and, at some point in the future, being able to change them... plus the yaagc memory can just be saved to the scenario file in the EMEM lines once it's hooked in without needing much extra work.
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.
You should only need to update the DSKY registers directly from a program on a few rare occasions: most information displays should be handled by VERB 6 or 16 and an appropriate NOUN. If need be, create a new noun to display the data you need, either find a suitable one in the Delco manual and implement it or pick an unused number and create your own. If it's usable in both CSM and LEM then add it to the 'generic' functions in apolloguidance.cpp rather than make it CSM-specific or LEM-specific.
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 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.