Refactor FlushFirmwareTask

- Instead of throwing an exception, it returns an invalid task
- Invalid tasks are now ignored by retro::task::push
- Change some names and text for clarity
- FlushFirmware now accepts two paths; one for the firmware, one for the WFC settings
- If the FlushFirmwareTask failed to be created, warn the user that firmware changes won't be saved

src/libretro/config.hpp

@@ -258,7 +258,6 @@ namespace melonds {
             [[nodiscard]] std::string_view DsiFirmwarePath() noexcept;
             [[nodiscard]] std::string_view DsiNandPath() noexcept;
             [[nodiscard]] std::string_view GeneratedFirmwareSettingsPath() noexcept;
-            [[nodiscard]] std::string_view EffectiveFirmwarePath() noexcept;
             [[nodiscard]] inline std::string_view FirmwarePath(enum ConsoleType type) noexcept {
                 return type == ConsoleType::DSi ? DsiFirmwarePath() : FirmwarePath();
             }

src/libretro/config/config.cpp

@@ -325,11 +325,6 @@ namespace melonds::config {
         string_view DsiNandPath() noexcept { return _dsiNandPath; }
 
         string_view GeneratedFirmwareSettingsPath() noexcept { return "melonDS DS/wfcsettings.bin"; }
-
-        string_view EffectiveFirmwarePath() noexcept {
-            string_view firmware = FirmwarePath(ConsoleType());
-            return (firmware == config::values::BUILT_IN) ? GeneratedFirmwareSettingsPath() : firmware;
-        }
     }
 
     namespace video {

src/libretro/libretro.cpp

@@ -64,6 +64,8 @@
 using std::make_optional;
 using std::optional;
 using std::nullopt;
+wfcSettingsPathusing std::string;
+using std::string_view;
 using retro::task::TaskSpec;
 
 namespace melonds {
@@ -619,7 +621,13 @@ static void melonds::load_games(
         retro::task::push(file::FlushTask());
     }
 
-    retro::task::push(sram::FlushFirmwareTask(config::system::EffectiveFirmwarePath()));
+    string_view firmwareName = config::system::FirmwarePath(config::system::ConsoleType());
+    if (TaskSpec flushTask = sram::FlushFirmwareTask(firmwareName)) {
+        retro::task::push(std::move(flushTask));
+    }
+    else {
+        retro::set_error_message("System path not found, changes to firmware settings won't be saved.");
+    }
 
     if (_loaded_gba_cart && (NDS::IsLoadedARM9BIOSBuiltIn() || NDS::IsLoadedARM7BIOSBuiltIn() || SPI_Firmware::IsLoadedFirmwareBuiltIn())) {
         // If we're using FreeBIOS and are trying to load a GBA cart...

src/libretro/retro/task_queue.cpp

@@ -40,8 +40,10 @@ void retro::task::init(bool threaded, retro_task_queue_msg_t msg_push) noexcept
 
 void retro::task::push(TaskSpec&& task) noexcept {
     ZoneScopedN("retro::task::push");
-    task_queue_push(task._task);
-    task._task = nullptr;
+    if (task.Valid()) {
+        task_queue_push(task._task);
+        task._task = nullptr;
+    }
 }
 
 void retro::task::wait() noexcept {

src/libretro/retro/task_queue.hpp

@@ -33,6 +33,7 @@ namespace retro::task {
 
     void init(bool threaded, retro_task_queue_msg_t msg_push) noexcept;
 
+    /// Ignores invalid tasks.
     void push(TaskSpec&& task) noexcept;
     void check() noexcept;
     void reset() noexcept;
@@ -41,6 +42,8 @@ namespace retro::task {
 
     class TaskSpec {
     public:
+        // Exists to create a trivial task that does nothing.
+        TaskSpec() noexcept = default;
         TaskSpec(const TaskHandler& handler, const TaskCallback& callback = nullptr, const TaskHandler& cleanup = nullptr, retro_time_t when = ASAP, const std::string& title = "");
         ~TaskSpec() noexcept;
         TaskSpec(TaskSpec&& other) noexcept;
@@ -49,8 +52,10 @@ namespace retro::task {
         TaskSpec& operator=(const TaskSpec& other) = delete;
         [[nodiscard]] bool Valid() const noexcept { return _task != nullptr; }
 
-        [[nodiscard]] retro_time_t When() const noexcept { return _task->when; }
-        void When(retro_time_t when) noexcept { _task->when = when; }
+        [[nodiscard]] retro_time_t When() const noexcept { return _task ? _task->when : 0; }
+        void When(retro_time_t when) noexcept { if (_task) _task->when = when; }
+
+        operator bool() const noexcept { return _task != nullptr; }
     private:
         void FreeTask() noexcept;
         static void TaskHandlerWrapper(retro_task_t* task) noexcept;
@@ -60,6 +65,11 @@ namespace retro::task {
         retro_task_t* _task;
     };
 
+    static bool operator==(const TaskSpec& lhs, std::nullptr_t) noexcept { return !lhs; }
+    static bool operator==(std::nullptr_t, const TaskSpec& rhs) noexcept { return !rhs; }
+    static bool operator!=(const TaskSpec& lhs, std::nullptr_t) noexcept { return (bool)lhs; }
+    static bool operator!=(std::nullptr_t, const TaskSpec& rhs) noexcept { return (bool)rhs; }
+
     class TaskHandle {
     public:
         // Default destructor

src/libretro/sram.cpp

@@ -122,21 +122,23 @@ static void FlushGbaSram(retro::task::TaskHandle &task, const retro_game_info& g
     }
 }
 
-static void FlushFirmware(const string& firmwarePath) noexcept {
+static void FlushFirmware(const string& firmwarePath, const string& wfcSettingsPath) noexcept {
     ZoneScopedN("melonds::sram::FlushFirmware");
     using SPI_Firmware::Firmware;
     using namespace melonds;
 
     retro_assert(!firmwarePath.empty());
     retro_assert(path_is_absolute(firmwarePath.c_str()));
+    retro_assert(!wfcSettingsPath.empty());
+    retro_assert(path_is_absolute(wfcSettingsPath.c_str()));
 
     const Firmware* firmware = SPI_Firmware::GetFirmware();
 
     retro_assert(firmware != nullptr);
     retro_assert(firmware->Buffer() != nullptr);
 
     if (firmware->Header().Identifier != SPI_Firmware::GENERATED_FIRMWARE_IDENTIFIER) {
-        // If this is not the default built-in firmware...
+        // If this is a native firmware blob...
         Firmware firmwareCopy(*firmware);
         // TODO: Apply the original values of the settings that were overridden
         if (filestream_write_file(firmwarePath.c_str(), firmware->Buffer(), firmware->Length())) {
@@ -148,19 +150,17 @@ static void FlushFirmware(const string& firmwarePath) noexcept {
     } else {
         u32 eapstart = firmware->ExtendedAccessPointOffset();
         u32 eapend = eapstart + sizeof(firmware->ExtendedAccessPoints());
-
         u32 apstart = firmware->WifiAccessPointOffset();
-        u32 apend = apstart + sizeof(firmware->AccessPoints());
 
         // assert that the extended access points come just before the regular ones
         assert(eapend == apstart);
 
         const u8* buffer = firmware->ExtendedAccessPointPosition();
         u32 length = sizeof(firmware->ExtendedAccessPoints()) + sizeof(firmware->AccessPoints());
-        if (filestream_write_file(firmwarePath.c_str(), buffer, length)) {
-            retro::debug("Flushed %u-byte WFC settings to \"%s\"", length, firmwarePath.c_str());
+        if (filestream_write_file(wfcSettingsPath.c_str(), buffer, length)) {
+            retro::debug("Flushed %u-byte WFC settings to \"%s\"", length, wfcSettingsPath.c_str());
         } else {
-            retro::error("Failed to write %u-byte WFC settings to \"%s\"", length, firmwarePath.c_str());
+            retro::error("Failed to write %u-byte WFC settings to \"%s\"", length, wfcSettingsPath.c_str());
         }
     }
 }
@@ -189,26 +189,35 @@ retro::task::TaskSpec melonds::sram::FlushGbaSramTask(const retro_game_info& gba
     return task;
 }
 
-retro::task::TaskSpec melonds::sram::FlushFirmwareTask(string_view path) {
-    optional<string> firmwarePath = retro::get_system_path(path);
+retro::task::TaskSpec melonds::sram::FlushFirmwareTask(string_view firmwareName) noexcept {
+    optional<string> firmwarePath = retro::get_system_path(firmwareName);
     if (!firmwarePath) {
-        throw environment_exception("Failed to get system path for firmware named " + string(path));
+        retro::error("Failed to get system path for firmware named \"%s\", firmware changes won't be saved.", firmwareName.data());
+        return retro::task::TaskSpec();
+    }
+
+    string_view wfcSettingsName = config::system::GeneratedFirmwareSettingsPath();
+    optional<string> wfcSettingsPath = retro::get_system_path(wfcSettingsName);
+    if (!wfcSettingsPath) {
+        retro::error("Failed to get system path for WFC settings at \"%s\", firmware changes won't be saved.", wfcSettingsName.data());
+        return retro::task::TaskSpec();
     }
+
     return retro::task::TaskSpec(
-        [path=*firmwarePath](retro::task::TaskHandle &) noexcept {
+        [firmwarePath=*firmwarePath, wfcSettingsPath=*wfcSettingsPath](retro::task::TaskHandle &) noexcept {
             ZoneScopedN("melonds::sram::FlushFirmwareTask");
 
             if (TimeToFirmwareFlush != nullopt && (*TimeToFirmwareFlush)-- <= 0) {
                 // If it's time to flush the firmware...
                 retro::debug("Firmware flush timer expired, flushing data now");
-                FlushFirmware(path);
+                FlushFirmware(firmwarePath, wfcSettingsPath);
                 TimeToFirmwareFlush = nullopt; // Reset the timer
             }
         },
         nullptr,
-        [path=*firmwarePath](retro::task::TaskHandle&) noexcept {
+        [path=*firmwarePath, wfcSettingsPath=*wfcSettingsPath](retro::task::TaskHandle&) noexcept {
             ZoneScopedN("melonds::sram::FlushFirmwareTask::Cleanup");
-            FlushFirmware(path);
+            FlushFirmware(path, wfcSettingsPath);
             TimeToFirmwareFlush = nullopt;
         }
     );

src/libretro/sram.hpp

@@ -38,7 +38,7 @@ namespace melonds::sram {
     void InitGbaSram(GbaCart& gba_cart, const struct retro_game_info& gba_save_info);
 
     retro::task::TaskSpec FlushGbaSramTask(const retro_game_info& gba_save_info) noexcept;
-    retro::task::TaskSpec FlushFirmwareTask(std::string_view path);
+    retro::task::TaskSpec FlushFirmwareTask(std::string_view firmwareName) noexcept;
 
     /// An intermediate save buffer used as a staging ground between retro_get_memory and NDSCart::LoadSave.
     class [[deprecated("Expose the buffers to libretro directly")]] SaveManager {