Conversation
… English, Portuguese, and Russian
…raction to clarify default behavior
…me download payload types
… GameFilesManager
…maticallyDeleteArchiveFiles parameter
…wnloadSettingsModal and RepacksModal, and update related user preferences in SettingsContextDownloads
…tails component for enhanced download handling
…roved user experience
…into feat/LBX-650
chubbygrannychaser
left a comment
chubbygrannychaser
left a comment
There was a problem hiding this comment.
Found two regressions that should be fixed before merge; see inline comments.
| await DownloadManager.startDownload(download).then(() => { | ||
| return downloadsSublevel.put(gameKey, download); | ||
| }); | ||
| await downloadsSublevel.put(gameKey, download); |
There was a problem hiding this comment.
Persisting the download before DownloadManager.startDownload() regresses the failure path. If startDownload() throws while resolving the URL / validating credentials, we now leave a queued+active record in LevelDB even though nothing actually started. The previous order only wrote after a successful start, so please either restore that ordering or clean up the inserted record in the catch block.
| setSelectedDownloader(getDefaultDownloader(availableDownloaders)); | ||
| }, [getDefaultDownloader, userPreferences?.downloadsPath, downloadOptions]); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
This modal rehydrates deleteArchiveFilesAfterExtraction on open, but automaticExtractionEnabled is still only initialized once from props. If the user toggles extraction, closes the modal, and reopens it, this checkbox can keep stale local state instead of reflecting userPreferences?.extractFilesByDefault ?? true. Please reset that state on open too, the same way you do for the new delete-archive toggle.
| userPreferences?.deleteArchiveFilesAfterExtractionByDefault ?? | ||
| false; | ||
|
|
||
| if (shouldDelete) { |
There was a problem hiding this comment.
The automatic-delete path bypasses the existing deleteArchive flow and calls fs.unlink() directly. That loses the extra bookkeeping in src/main/events/library/delete-archive.ts (notably clearing installerSizeInBytes, plus centralized logging/error handling), so auto-deletion can leave stale installer metadata behind. Could this reuse the same deletion path instead of unlinking files inline?
…d of unlinking files directly
|
3 participants
When submitting this pull request, I confirm the following (please check the boxes):
Fill in the PR content: