Retry failed player saves instead of draining them to SafeStorage

[HarleyGilpin]

Fixes #1250. Rebased onto main now that #1249 has merged — the diff is the three files below and nothing else.

The bug

SaveQueue's exception handler closed over pending rather than the batch that failed:

private val handler = CoroutineExceptionHandler { _, exception ->
    logger.error(exception) { "Error saving players!" }
    scope.fallback(pending.values.toList())
}

run() snapshots pending.values.toList() and hands that to storage.save. If the write throws, the handler re-reads pending, which by then also holds accounts queued after the snapshot, ones storage was never asked to write. All of them go to fallback, which writes them out and clears them.

The fallback is SafeStorage, and it's write-only: load returns null, exists returns false, names returns an empty map, and the files are named "$timestamp-$name.toml" so nothing would find them anyway. So a failed batch meant those accounts were removed from pending, never retried, and invisible to every later load. The next login served whatever the account file held before the session, with no signal to the player and one Error saving players! line as the only trace.

Blast radius wider than the failure: a disk momentarily full took out every account queued at that instant, not the batch that failed.

The fix

The handler goes away in favour of a try/catch inside the coroutine, so the failed batch is the one that gets handled.

On failure the accounts stay in pending and the next tick retries real storage. A transient error self-heals. storage.save.retryMinutes (default 5) bounds the retry per account; past it the account is written to the failed saves directory and dropped, so a permanently broken backend doesn't wedge the queue forever. Five minutes covers the #1212 shape, a host stall that pushed a tick to 25918ms, without being an indefinite hang.

While an account is pending it still can't log in (PlayerAccountLoader checks saveQueue.saving). That's the point: refusing the login is better than serving a stale file under a save that hasn't landed.

direct() passes retry = false. At shutdown there's no next tick, so a failure should dump immediately, and the dump now happens inside the job the caller joins, rather than in a sibling coroutine the process could exit before it runs. That second half was a real hole: AutoSave's worldDespawn joins the save job, and the old handler's scope.fallback(...) was not that job.

A CoroutineExceptionHandler stays on the scope purely as a last-resort log, so an Error escaping the catch (e: Exception) isn't silent.

Tests

Three in SaveQueueTest, each checked by reverting the corresponding production change and confirming the right test fails:

• Failure only touches the accounts that were attempted — blocks a save on a latch, queues a second account while the first is in flight, then fails the write. The second account must not be dumped and must stay pending. Fails when failed() is fed pending.values.toList().
• Failed save retries against real storage and doesn't kill the queue — replaces the old Failed save falls back and doesn't kill the queue, which asserted the drain behaviour this PR removes. A transient failure must be retried and must not reach the fallback. Fails when the retry window is dropped.
• Shutdown save writes to the fallback before the job completes — direct().join() must return with the account already on disk somewhere. Fails when direct() retries instead.

Full suite green.

Known limitation

run() submits all of pending as one batch, so an account whose data reliably breaks serialisation takes the whole batch with it for the retry window before everything gets dumped together. Isolating a poison account needs per-account writes, which is a bigger change than this one and would trade DatabaseStorage's batched transaction for N of them. Left alone deliberately.