Clean up / refactor save file logic

[HunterZ]

Looking at save.c turned up a lot of notes:

• update to 64-bit timestamps for Y2K38?
  • NOTE: this will break save compatibility
  • explicitly casting to 32-bits for now
• resolve externs.h include order concern
  • see legacy code comments
  • hopefully Remove dead code and clean up formatting #2 / Remove multi-user system features #5 will clear the way
• move MS-DOS configurable wall/floor symbol feature out of core code and into the I/O layer, then clean up save file handling of it (the fact that it leaked this deep is a code smell!)
• (DONE) define constants for settings flags (variable 'l') to help keep the save and load code in sync
• (WIP) get rid of save.c file globals
  • it's better to just pass stuff around than to share global state
  • bundle them in a struct if it's convenient for passing around
  • file_ptr and xor_byte fixed. from_savefile and start_time need more thought
• get rid of static function prototypes
  • define them in the order used for now?
  • see Restructure the codebase #3 for longer-term thoughts
• fix casting of data
  • signed versus unsigned int's, etc.
  • change vars to unsigned where possible?
  • implement proper signed read/write functions?
• document save file format somewhere?
  • at least add more comments to the code if nothing else
  • it's probably not documented in a text file to discourage cheating (also XORs data and such for this reason)
• (DONE) remove DEBUG macro?
• (WIP) I don't like the while() loop in save_char().
  • Seems like it could get caught forever if _save_char() goes awry
  • Not as bad now that player prompts are better
• (DONE) I don't like the logic in save_char() either
  • Especially the delete via unlink() seems problematic
  • People can just tab out and delete/chmod a file now
  • Maybe just give the player an option to pick a different name or exit?

[HunterZ]

get_char() uses goto everywhere - maybe use an inner function that can return instead?

Edit: The landing point for the error goto is ugly. Need to somehow get rid of these for sure.

Could also try to tolerate save file oddities better, like "dropping" extra inventory items if a save contains more than the allowed limit.

[HunterZ]

There's a lot of save file loading code to work with files from older UMoria 5.x versions. Should probably consider just removing support for old save files, since this is a fork. Maybe support whatever the oldest version is that doesn't currently require version checks?

At the very least, duplicate code resulting from data being at different places in different versions should be collapsed via use of a shared function for loading that data from the current file position.

[HunterZ]

Save file loading code has a bunch of status messages that aren't even visible on modern computers because they go by too fast... Might be worth thinking about printing messages sequentially and then having the player hit a key before continuing on to the game proper.

Another option might be to put the loading messages in the previous messages log, but I think the log gets saved to and read from the save file, so some of that data would be lost/displaced.

[HunterZ]

Current implementation of "accepted"/"risky" notice for version-mismatched save files is pretty bogus.

A more meaningful implementation might be to say "risky" for anything where version-dependent code was applied in an attempt to get an older save working, and "accepted" for everything else.

[HunterZ]

This is all now fixed!

The wr_/rd_ functions should take a FILE* parameter instead of relying on a global fileptr. Same probably goes for xor_byte, which should either be passed in as a pointer, or passed and returned by value; I think pointer makes more sense.

Could possibly do multi-byte reads/writes, but I think that could open things up to byte-ordering issues that the current implementation avoids.

The current code is also kind of mind-bending with all the XOR dancing. For writing at least, I'm replacing a bunch of it with calls to wr_byte().

[HunterZ]

rd_string() (and maybe others) should probably take a count limit to protect against buffer overruns that could occur when reading a corrupt save file.

[HunterZ]

Duplicate character logic seems odd, and is currently only supported under *nix. Remove?