Skip to content

Commit ea146b0

Browse files
dsarnoclaude
andauthored
Gate in-editor self-update off on Godot < 4.4 (#475) (#482)
* Gate in-editor self-update off on Godot < 4.4 (#475) On Godot < 4.4 the self-update takes the `_install_zip_inline` extract-then-restart path, and that engine's stricter `GDScript::reload()` (`!p_keep_state && has_instances` -> `ERR_ALREADY_IN_USE`) turns the extract-over-live-scripts into a reload error flood plus a SIGSEGV in `EditorDockManager::remove_dock` / `SceneTree::finalize` on the restart/quit. Approaches that tried to make the in-place flow safe on 4.3 were disproven (see #475). This gates the in-editor update off on < 4.4 instead: `start_install()` opens the GitHub releases page for a manual download and shows "in-editor update needs Godot 4.4+ — replace addons/godot_ai/ manually", never entering the extract pipeline that crashes those engines. The update banner's button is relabeled "Open download page" up-front at check time so the action is honest before the click. 4.4+ is unchanged (real in-place update via update_reload_runner). Version logic is split into a pure static `_version_can_self_update( major, minor)` so it's unit-testable without faking the engine; guards `major` so a future 5.x (minor 0) isn't misclassified. Verified e2e on real engines (disposable fixtures at v2.5.6 vs the v2.5.7 release): - Godot 4.3: clicking the gated button opens the release page + manual message; NO extract, NO ERR_ALREADY_IN_USE, NO crash; a subsequent Cmd-Q exits cleanly with no new crash report. Only the two benign `extends Logger` parse errors remain (cosmetic, < 4.5). - Godot 4.6: gate does NOT fire; real in-place update runs end-to-end (2.5.6 -> 2.5.7 via update_reload_runner), 0 errors, 0 crash. - GDScript unit tests pin the predicate (4.3/4.0/3.x false; 4.4/4.6/5.0 true); full update_manager suite green (14/14). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Gated update banner: show guidance up-front + wrap the label Two UX fixes on the < 4.4 self-update gate, from dogfooding the 4.3 banner: - The manual-update explanation only appeared AFTER clicking. Move it to check time: _on_update_check_completed now emits the full guidance label (with the available version) alongside the "Open download page" button, so the user understands what the button does before pressing it. Extracted into a single _manual_update_label(version) helper so the check-time and click-time text cannot drift. - The long label was a single non-wrapping line that stretched the dock super wide. _update_label now uses AUTOWRAP_WORD_SMART + SIZE_FILL so it wraps to the dock width instead. Adds GDScript tests for _manual_update_label (version present / absent). update_manager suite 16/16 green on 4.6; compiles clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Align install-dispatch version check with the gate predicate (F2) The _can_self_update() gate is major-aware (major>4 or (major==4 and minor>=4)), but _install_zip / _install_zip_inline still keyed off `minor >= 4` only. On a future Godot 5.0 (major=5, minor=0) the gate would let the user in expecting the seamless runner reload, but the minor-only check would route them to the pre-4.4 inline extract + "Restart editor" path — the crash-prone flow the gate exists to avoid. Route both install-dispatch checks through the same _version_can_self_update(major, minor) predicate (already unit-tested for 4.3/4.4/4.6/5.0) so the gate and the dispatch can't disagree. No behavior change on 4.4-4.6; fixes the latent 5.x inconsistency. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address Copilot review on the gate (#482): shell_open guard + preserve label Two Copilot findings on the < 4.4 gated start_install path: - OS.shell_open()'s return was ignored: the button switched to the disabled "Opened download page" state even if the browser never opened (no handler / headless), stranding the user. Now only claim success + lock the button when shell_open returns OK; on failure show "Couldn't open browser — retry" and keep the button enabled. - The click-path emit re-set label_text to _manual_update_label(""), which contradicted its own "keep the up-front guidance label" comment and dropped the version. Removed the label_text key so the version-bearing guidance label from check time persists through the click (matches the comment, keeps "Update vX.Y.Z available"). Verified: 4.6 import clean, update_manager suite 15/16 (1 dev-checkout skip), ruff green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 81e0b36 commit ea146b0

3 files changed

Lines changed: 118 additions & 6 deletions

File tree

plugin/addons/godot_ai/mcp_dock.gd

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,12 @@ func _build_ui() -> void:
487487
_update_label = Label.new()
488488
_update_label.add_theme_font_size_override("font_size", 15)
489489
_update_label.add_theme_color_override("font_color", Color(1.0, 0.85, 0.3))
490+
## Wrap long banner text (e.g. the < 4.4 manual-update guidance) instead
491+
## of letting a single line stretch the whole dock wide. The dock is a
492+
## fixed-width side panel, so constrain horizontally and wrap.
493+
_update_label.autowrap_mode = TextServer.AUTOWRAP_WORD_SMART
494+
_update_label.size_flags_horizontal = Control.SIZE_FILL
495+
_update_label.custom_minimum_size = Vector2(0, 0)
490496
_update_banner.add_child(_update_label)
491497

492498
var update_btn_row := HBoxContainer.new()

plugin/addons/godot_ai/utils/update_manager.gd

Lines changed: 72 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,65 @@ func clear_pending_download() -> void:
103103
_latest_download_url = ""
104104

105105

106-
## Driven by the dock's Update button. With no resolved download URL —
107-
## either the check never completed, or the release didn't ship a
108-
## matching asset — falls back to opening the release page. Otherwise
109-
## kicks off the download → extract → reload pipeline.
106+
## True when the running Godot can self-update in place. Godot < 4.4 takes
107+
## the `_install_zip_inline` extract-then-restart path, and that engine's
108+
## stricter `GDScript::reload()` (`!p_keep_state && has_instances` ->
109+
## `ERR_ALREADY_IN_USE`) turns the extract-over-live-scripts into a reload
110+
## error flood plus a SIGSEGV in `EditorDockManager::remove_dock` /
111+
## `SceneTree::finalize` on the restart/quit (#475). So on < 4.4 we don't
112+
## run the in-editor pipeline at all — the user updates manually.
113+
## Guards `major` too so a future Godot 5.x (minor 0) isn't misclassified.
114+
func _can_self_update() -> bool:
115+
var v := Engine.get_version_info()
116+
return _version_can_self_update(int(v.get("major", 0)), int(v.get("minor", 0)))
117+
118+
119+
## Pure version predicate, split out so it's testable without faking the
120+
## running engine. In-editor self-update needs Godot >= 4.4.
121+
static func _version_can_self_update(major: int, minor: int) -> bool:
122+
return major > 4 or (major == 4 and minor >= 4)
123+
124+
125+
## Banner guidance for the gated (< 4.4) path. Shown up-front at check time
126+
## (with the available version) and again on click, so the user understands
127+
## the manual-update flow before they press anything. Single source of truth
128+
## so check-time and click-time text never drift.
129+
static func _manual_update_label(version: String) -> String:
130+
var prefix := "Update available"
131+
if not version.is_empty():
132+
prefix = "Update v%s available" % version
133+
return (
134+
prefix
135+
+ " — in-editor update needs Godot 4.4+. Open the download page, then "
136+
+ "replace addons/godot_ai/ manually and relaunch."
137+
)
138+
139+
140+
## Driven by the dock's Update button. On Godot < 4.4 (see `_can_self_update`)
141+
## the in-editor install is disabled — we open the release page for a manual
142+
## download instead, never entering the extract pipeline that crashes those
143+
## engines. With no resolved download URL — either the check never completed,
144+
## or the release didn't ship a matching asset — also falls back to opening
145+
## the release page. Otherwise kicks off the download → extract → reload
146+
## pipeline.
110147
func start_install() -> void:
148+
if not _can_self_update():
149+
## Only claim success + lock the button if the browser actually opened.
150+
## On failure (no handler, headless) keep the button enabled so the
151+
## user can retry. Either way, leave the version-bearing guidance label
152+
## from check time in place — don't re-emit label_text.
153+
if OS.shell_open(RELEASES_PAGE) == OK:
154+
install_state_changed.emit({
155+
"button_text": "Opened download page",
156+
"button_disabled": true,
157+
})
158+
else:
159+
install_state_changed.emit({
160+
"button_text": "Couldn't open browser — retry",
161+
"button_disabled": false,
162+
})
163+
return
164+
111165
if _latest_download_url.is_empty():
112166
OS.shell_open(RELEASES_PAGE)
113167
return
@@ -233,6 +287,14 @@ func _on_update_check_completed(
233287
return
234288
_latest_download_url = String(parsed.get("download_url", ""))
235289
update_check_completed.emit(parsed)
290+
## On engines that can't self-update (Godot < 4.4, #475), surface the
291+
## full manual-update guidance AND relabel the button up-front — before
292+
## any click — so the user knows what the button does and why.
293+
if not _can_self_update():
294+
install_state_changed.emit({
295+
"button_text": "Open download page",
296+
"label_text": _manual_update_label(String(parsed.get("version", ""))),
297+
})
236298

237299

238300
func _on_download_completed(
@@ -284,7 +346,11 @@ func _install_zip() -> void:
284346
_plugin != null
285347
and _plugin.has_method("install_downloaded_update")
286348
)
287-
if int(version.get("minor", 0)) >= 4 and has_runner:
349+
## Same major-aware predicate as the _can_self_update() gate, so a future
350+
## Godot 5.x (minor 0) takes the runner path the gate promised — not the
351+
## pre-4.4 inline extract. A bare `minor >= 4` here would route 5.0 to the
352+
## crash-prone inline path even though the gate let it in.
353+
if _version_can_self_update(int(version.get("major", 0)), int(version.get("minor", 0))) and has_runner:
288354
install_state_changed.emit({"button_text": "Reloading..."})
289355
## Runner takes over: plugin tears down, runner extracts + scans +
290356
## re-enables. `install_downloaded_update` calls
@@ -337,7 +403,7 @@ func _install_zip_inline(version: Dictionary) -> void:
337403
if _plugin != null and _plugin.has_method("prepare_for_update_reload"):
338404
_plugin.prepare_for_update_reload()
339405

340-
if int(version.get("minor", 0)) >= 4:
406+
if _version_can_self_update(int(version.get("major", 0)), int(version.get("minor", 0))):
341407
install_state_changed.emit({"button_text": "Scanning..."})
342408
## Filesystem scan must complete before plugin reload — otherwise
343409
## plugin.gd re-parses against a ClassDB that hasn't seen the new

test_project/tests/test_update_manager.gd

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,46 @@ func suite_name() -> String:
2323
return "update_manager"
2424

2525

26+
# ---- _version_can_self_update (pure / static, #475 gate) ---------------
27+
28+
func test_version_can_self_update_false_below_4_4() -> void:
29+
## Godot < 4.4 takes the extract-then-restart path that crashes (#475),
30+
## so the in-editor self-update is gated off on those engines.
31+
assert_false(McpUpdateManagerScript._version_can_self_update(4, 3),
32+
"4.3 must be gated (in-editor self-update disabled)")
33+
assert_false(McpUpdateManagerScript._version_can_self_update(4, 0),
34+
"4.0 must be gated")
35+
assert_false(McpUpdateManagerScript._version_can_self_update(3, 9),
36+
"a hypothetical 3.x must be gated")
37+
38+
39+
func test_version_can_self_update_true_at_and_above_4_4() -> void:
40+
assert_true(McpUpdateManagerScript._version_can_self_update(4, 4),
41+
"4.4 is the first engine that can self-update in place")
42+
assert_true(McpUpdateManagerScript._version_can_self_update(4, 6),
43+
"4.6 can self-update")
44+
assert_true(McpUpdateManagerScript._version_can_self_update(5, 0),
45+
"a future 5.0 (minor 0) must not be misclassified by the minor check")
46+
47+
48+
func test_manual_update_label_includes_version_and_guidance() -> void:
49+
## Shown up-front on < 4.4 (before any click) so the user understands the
50+
## manual-update flow. Must name the version and the 4.4+ requirement.
51+
var with_v := McpUpdateManagerScript._manual_update_label("2.5.7")
52+
assert_contains(with_v, "2.5.7", "label must name the available version")
53+
assert_contains(with_v, "Godot 4.4+", "label must state the engine requirement")
54+
assert_contains(with_v, "addons/godot_ai/", "label must point at the manual-swap path")
55+
56+
57+
func test_manual_update_label_omits_version_when_unknown() -> void:
58+
## On the click path the version isn't re-threaded; the label degrades to
59+
## a generic "Update available" without a stray "v" token.
60+
var no_v := McpUpdateManagerScript._manual_update_label("")
61+
assert_contains(no_v, "Update available", "generic label must still lead with 'Update available'")
62+
assert_false(no_v.contains(" v"), "no version token when version is empty")
63+
assert_contains(no_v, "Godot 4.4+", "label must still state the engine requirement")
64+
65+
2666
# ---- parse_releases_response (pure / static) ---------------------------
2767

2868
func _make_body(json_str: String) -> PackedByteArray:

0 commit comments

Comments
 (0)