andrei-drexler / ironwail

High-performance QuakeSpasm fork
GNU General Public License v2.0
530 stars 49 forks source link

Change when to show/hide loading plaque #132

Closed daftmugi closed 2 years ago

daftmugi commented 2 years ago

I found that error messages cleared on load error and sounds temporarily stopped.

This patch postpones showing the loading plaque until after common error messages. The loading plaque is shown a little bit after loading technically starts, but that seems fine since it's also a UX technique to postpone loading indication a bit to make loading seem faster. This has three main benefits:

  1. Error messages are shown in-game and not cleared on error.
  2. The loading plaque is not temporarily shown when there is a common error, such as file not found or version mismatch.
  3. Sounds do not temporarily stop (due to a line in SCR_BeginLoadingPlaque()).

At the console, this patch keeps the console from "jumping" (closing and opening again) during load, restart, etc. and the loading plaque is not shown.

andrei-drexler commented 2 years ago

Hey, sorry about the delay! I think it's best to show the loading plaque before doing any kind of actual loading (including reading the savefile), but the changes in SCR_BeginLoadingPlaque look good - committted in 4f7c0b168b3558500f5ff15107fb7b80f48eb5ab. Thanks!

daftmugi commented 2 years ago

Thanks, I appreciate it!

Is there another way we can address the three issues (or some of the issues) above that my patch fixed?

To test, install and launch a new game/mod without a quicksave file. Try to quickload. It'll blink the loading plaque and glitch the sounds without displaying an error message to the player. The player must know about the console in order to investigate the issue.

For clarity, the three issues are:

  1. On error, no message is shown to the player, because it is cleared before the player has a chance to see it.
  2. On error, the loading plaque is temporarily shown, such as file not found or version mismatch. I find this to be unexpected behavior. If the file is not found, why did it say "loading" when there's nothing to load?
  3. On error, sounds temporarily stop and restart.

Thank you for working on this!

andrei-drexler commented 2 years ago

Ideally I think the loading plaque would be shown in-between opening the file and loading its contents, but that would mean splitting up COM_LoadMallocFile_TextMode_OSPath, which I'd rather not do. The next best thing IMO is to check that the file exists before showing the plaque - added in 663de01b4c39cdae406342dbd9722c87919a9cc4.

daftmugi commented 2 years ago

Works great, thank you!