fix: address issues #93 (false-positive eyebrow defect) and #94 (game crash when both open) - #95
Conversation
Issue #93: the eyebrow-701 ("no eyebrows") mutation was always flagged as a birth defect because a 700-706 range heuristic was applied before GPAK data had a chance to correct it. Trust GPAK's `tag birth_defect` when present; keep the range heuristic only as a fallback for missing or legacy GPAK entries. Also log every detected defect at INFO with its slot/ID/source so users uploading logs can pinpoint mis-detections. Issue #94 (game crashes when both it and the manager are open): snapshot the live .sav (and any -wal/-shm/-journal sidecars) to a temp directory before opening it, for both the full SaveLoadWorker and the QuickRoomRefreshWorker. Eliminates any co-access between the running game and our SQLite readers. Log the snapshot size and copy duration plus parse timing so uploaded logs expose the full read path.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94b4d86808
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| elif not is_sentinel_missing and 700 <= mutation_id <= 706: | ||
| # No GPAK data at all — use the range heuristic as a best guess. | ||
| is_defect = True | ||
| defect_source = "range_fallback_no_gpak" | ||
| raw_name = str(raw_name).strip() |
There was a problem hiding this comment.
Move GPAK name/detail mapping out of no-GPAK fallback
The GPAK-to-display mapping (raw_name/stat_desc → display_name/detail) now only runs inside the elif not is_sentinel_missing and 700 <= mutation_id <= 706 fallback branch, so normal entries that do have gpak_info skip that mapping entirely. In practice this makes most non-defect visual mutations fall back to synthetic labels ("<slot> <id>") with empty details, which is a regression from prior behavior where GPAK names were shown.
Useful? React with 👍 / 👎.
| raw_name = str(raw_name).strip() | ||
| detail = str(stat_desc).strip() |
There was a problem hiding this comment.
Initialize GPAK fields before fallback defect formatting
This fallback branch references raw_name and stat_desc, but those variables are only assigned in the earlier if gpak_info: block. If a 700–706 mutation is present without GPAK metadata (but with a catalog fallback), this path raises UnboundLocalError during parse and the save load fails instead of continuing.
Useful? React with 👍 / 👎.
Summary
tag birth_defectmarker is now authoritative when present; the range heuristic is only a fallback for legacy/missing entries. Every detected defect is also logged at INFO with its slot/ID/source so users uploading logs let us pinpoint mis-detections.SaveLoadWorkerfull parse andQuickRoomRefreshWorkerfast path) now copy the.sav(plus any-wal/-shm/-journalsidecars) to a temp directory first and read the copy, so there's no co-access with the running game. Added a sharedsave_snapshothelper and INFO-level timing/size logs around snapshot and parse so uploaded logs contain a complete trace of every read.If #94 keeps reproducing after this, the logs will capture:
save load start path=… size=… mtime=…save snapshot path=… bytes=… copy_s=…save parse ok parse_s=…save load summary cats=… errors=…defect detected slot=… mutation_id=… defect_source=… name=…lineCloses #93. Addresses #94 (keep open pending user confirmation).
Test plan
%APPDATA%/MewgenicsBreedingManager/logs/mewgenics.logafter a load and confirm the new snapshot/parse/defect lines are present.pytest tests/test_game_update_crash_fixes.pystill passes (monkey-patched sqlite / parse_save shims still exercise the post-snapshot path).