HunterZ / hzmoria

My cleanup fork of UMoria 5.6
GNU General Public License v3.0
1 stars 0 forks source link

Clean up / refactor save file logic #6

Open HunterZ opened 2 years ago

HunterZ commented 2 years ago

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

HunterZ commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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