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).
All 747 imported PNGs were imported losslessly (compress/mode=0) without
mipmaps. Lossless textures are decompressed to raw RGBA8 in GPU memory,
so the big gameplay assets were extremely expensive: the 4100x2448
bush_curtain alone used ~40 MB of VRAM, and the 6400x8001 minigame
spritesheets ~205 MB each. Summed over the affected files, the
worst-case footprint was ~2.4 GB of texture memory for a game that
targets low-end 1-2 GB tablets with the gl_compatibility renderer. The
absence of mipmaps also caused shimmering and wasted bandwidth whenever
these textures were drawn below their native size, which is almost
always the case with the 2560x1800 canvas_items stretch.
- Switch the 119 textures with a dimension >= 512 px to VRAM
compression (compress/mode=2) with generated mipmaps. The project
already enables etc2_astc import, so mobile gets ETC2/ASTC and
desktop gets S3TC/BPTC. The total footprint for these textures drops
from ~2.4 GB raw RGBA8 to ~770 MB including mipmaps (~3x smaller
resident set, plus faster texture uploads).
- Keep small UI textures (< 512 px, 626 files) lossless: their VRAM
cost is negligible and block-compression artifacts would be most
visible on small crisp UI art.
- Set the default 2D texture filter to Linear Mipmap so the generated
mipmaps are actually sampled by canvas items (without this, mipmap
generation would only add memory).
- Remove viewport/hdr_2d=true: HDR 2D allocates RGBA16F framebuffers,
doubling framebuffer memory and bandwidth at 2560x1800 on exactly the
GPUs that can least afford it. No scene uses glow or HDR values (no
WorldEnvironment anywhere), so this was pure cost. As false is the
default value, Godot drops the line entirely.
Texture quality should be eyeballed in the editor on the largest
illustrations; any texture where compression artifacts are noticeable
can be switched back to lossless individually.
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).
vincent.p.mercier
Adrien Ufferte