Update notification text when import is paused due to gameplay #30402
Conversation
peppy
added
the
area:import
|
more of a point regarding design, but the spinning circle doesn't change, which might, at a glance, imply that the import isn't paused. something like a yellow pause icon might be more appropriate? actually, for that matter, there is already the yellow spinning circle design, might as well just reuse that |
|
|
||
| try | ||
| { | ||
| pauseIfNecessary(parameters, notification.CancellationToken); |
There was a problem hiding this comment.
It's a little bit scary that this is the first entrypoint to the async context (there's no await prior to this line). For example if Import is ever called from the update/bdl thread, then this is going to potentially deadlock the game.
I suggest either await Task.Yield() before the pause, or wrapping the entire thing in a Task.Run().
Or.... if it can, use await Task.Delay() instead of Thread.Sleep(). Doesn't look like it can though.
There was a problem hiding this comment.
It's a fair point. I started by duplicating the method and making an async version, but then ended up refactoring things a bit to not require this.
Of note, the Parallel.ForEachAsync change was required because without it, the previous Select loop would seem to cause all imports to get past the pause check and run up to the Import part which would mean they never actually pause.
The whole import async logic is a bit of a mess but as far as I can tell it's working as expected.
Addresses #30388.
osu.2024-10-23.at.08.50.25.mp4