All failure paths of the download/extract/swap flow displayed the same
generic ERROR_DOWNLOADING popup, which made user reports impossible to
map to an actual cause (the recent .DS_Store swap failure surfaced as a
plain "download error" even though the download and extraction had both
succeeded).
- Replace the magic error indices with a DownloadError enum, so call
sites read _show_error(DownloadError.REPLACE_FAILED) instead of
_show_error(2) with a comment.
- Add three specific messages with fr/es/pt_BR/it translations:
- ERROR_EXTRACTING_PACKAGE: archive missing or extraction failed
- ERROR_INVALID_PACKAGE: extracted pack failed the validity check
(kept distinct from INVALID_LANGUAGE_DIRECTORY, which describes the
local pack found while offline)
- ERROR_REPLACING_PACKAGE: previous pack could not be removed or the
new one could not be renamed into place; tells the user to restart
and retry rather than suggesting a download problem
- ERROR_DOWNLOADING is now used only for actual download/server
failures.
Localization completeness check, naming lint and GUT suite (65/65) all
pass.
The new extract/verify/swap flow surfaced a long-standing bug in
Utils.clean_dir(). On macOS, browsing a folder in the Finder drops a
hidden .DS_Store file in it, and clean_dir() had two flaws around that:
- DirAccess listings skip hidden files by default, so .DS_Store was
never listed nor removed and the directory was never actually empty.
- The return value of dir.remove(subfolder) was ignored, so the failed
removal of a subdirectory (non-empty because of its own hidden file)
was silently swallowed and clean_dir() still reported success.
As a result, delete_directory_recursive() emptied the old language pack
but could not delete its directory, the rename of the new pack onto
that still-existing directory failed, and the downloader showed the
download-error popup. Since the version on disk was never updated, the
app re-downloaded and failed again in a loop on every launch. The old
pre-swap flow had the same deletion failure but masked it: it extracted
directly into the existing directory and ignored rename errors.
- clean_dir() now lists hidden files (include_hidden) and checks every
removal, trying to remove everything (best effort, so callers like
clear_all_local_data still wipe as much as possible) and reporting
the first error encountered instead of a false OK.
- The downloader verifies that the previous pack directory is really
gone before renaming the new one into place; if not, it aborts the
swap, keeps the temporary directory and shows the error instead of
attempting a rename that cannot succeed.
- Add regression tests covering hidden-file deletion, full content
cleanup and the missing-directory error path.
Verified with the GUT test suite (65/65 passing).
Follow-up to the previous download fix: the old pack was no longer
deleted before the download, but it was still deleted at the start of
the extraction step. If the app crashed mid-extraction or the archive
was truncated, the device was left without a valid pack; worse, a
partially extracted directory containing language.db would pass
is_language_directory_valid() on the next offline launch and be
accepted as a working pack.
The extraction thread now follows an extract / verify / swap sequence:
1. Extract the archive into <lang>_tmp instead of the final location,
leaving the current pack untouched and avoiding any risk of merging
old and new files.
2. Validate the extracted pack (non-empty, language.db present) before
touching the current one; on failure, discard the temp directory and
the archive, show the invalid-directory error, and keep playing with
the current pack.
3. Only then delete the previous pack and rename the new one into
place. The window without a valid pack shrinks from the whole
extraction to two filesystem operations.
If the final rename fails, the temp directory is kept (the data is
intact on disk) and the error popup is shown; the next launch detects
the missing pack and downloads it again. A leftover temp directory from
a crash is removed at the start of the next extraction.
Verified with the GUT test suite (62/62 passing).
The package downloader deleted the installed language pack as soon as a
version difference with the server was detected, before the download had
even started. If the download then failed (connection drop, server
error), the device was left with an empty language directory: on the
next offline launch, the directory check failed and the app was unusable
until internet access was restored.
The early cleanup was also redundant: _copy_data() already removes the
existing directory right before extracting the new archive, i.e. after
the download has succeeded (HTTP 200).
Changes in package_downloader.gd:
- Remove the pre-download cleanup of the current language directory; the
previous pack now stays usable for offline play until the new one has
been fully downloaded.
- Check the return value of HTTPRequest.request() and show the download
error popup when the request cannot be started, instead of leaving the
screen waiting forever on a request that was never sent.
- Surface extraction failures (missing archive, unzip error) through the
error popup instead of silently returning from the extraction thread
and leaving the user stuck on a frozen progress screen.
Changes in login.gd:
- Handle the result of UserDataManager.login_student(), which was
previously logged and ignored. The synchronization performed right
before the login can delete or move the student locally (e.g. the
server requested a local deletion), in which case the child was sent
to the gardens scene with no student session loaded, ending up on a
dead screen. On failure, play the wrong-password feedback and reset
the code keyboard instead of changing scene.
Verified with the GUT test suite (58/58 passing).
The UserDatabaseSynchronizer outlives the scenes that use it, which made
the periodic background synchronization fragile in two ways:
1. A freed loading popup permanently broke the sync.
The teacher settings screen injects its LoadingPopup into the
synchronizer but never cleared the reference on scene exit. Since a
freed Object is not equal to null in Godot 4, the next timer-driven
synchronization called into the freed node, crashed the coroutine
after `synchronizing` had been set to true, and every subsequent
synchronize() call was then silently cancelled by the re-entrancy
guard until the app restarted.
- All popup guards now use is_instance_valid() (plus is_inside_tree()
where get_tree() is needed).
- The teacher settings screen clears the synchronizer's popup
reference in _exit_tree().
2. Server payloads were applied without shape validation.
_apply_server_response() indexed JSON fields (lesson unlock data,
remediation score tuples, confusion matrix entries) without checking
their types, as flagged by the five "TODO ADD SECURITY" comments. A
single malformed or legacy record raised a script error mid-sync,
which also wedged the `synchronizing` flag as described above.
- Lesson unlock entries, remediation score pairs and confusion matrix
entries are now validated before use; malformed records are logged
and skipped individually so one bad record cannot abort the whole
synchronization or corrupt local student data.
- The three copy-pasted remediation parsing blocks are factored into
a single _parse_score_remediation() helper.
Verified with the GUT test suite (58/58 passing).
Reorders the left sidebar so risky actions sit at the bottom with a gap:
Dashboard, Synchronize, Logout | Change Language (red), Delete Account.
The new Change Language button opens a warning popup with a language
dropdown. On confirm, if the selected locale differs from the current
one, the client calls POST /reset_language. On success it wipes the
local teacher folder (progression, remediation, confusion matrix…),
applies the new language to device and teacher settings, logs out and
reloads from the splash screen. On error the user sees an error popup
and nothing local changes.
When credentials are edited after a successful password reset, the error
state is cleared via _hide_login_error. Re-enable the button there so it
is always in a clean state before being shown again on the next login
failure, preventing it from being permanently disabled within a session.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Disable reset button while awaiting server response to prevent duplicate requests
- Show CHECK_YOUR_EMAIL message on success (was previously only logged, never shown)
- Show RESET_PASSWORD_FAILED message on failure instead of incorrectly showing CHECK_YOUR_EMAIL
- Re-enable the reset button only on failure so the user can try again
- Add RESET_PASSWORD_FAILED localization key in all supported languages (fr, es, pt_BR, it)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adrien Ufferte