fix: critical bugs from code review

- Split get_boxart() into _enrich_metadata + _fetch_banner_textures (under 50 lines)
- Fix race condition: add _pending_gif_requests to prevent concurrent downloads
- Fix race condition: add is_instance_valid checks in _on_focus_changed
- Fix stale reference: check is_instance_valid on _focused_animated_texture
- Fix temp file collisions: use URL hash for unique temp file names
- Add timeout to HTTPRequest in _download_url_bytes (HTTP_TIMEOUT_MS was unused)
- Add bounds checks in _parse_gif_extension and _skip_gif_header for truncated GIFs

core/boxart_itch.gd

@@ -29,6 +29,7 @@ var _meta_cache_dir := "itch_art"
 var _enriched_meta: Dictionary = {}
 var settings_manager := load("res://core/global/settings_manager.tres") as SettingsManager
 var _animated_texture_cache: Dictionary = {}
+var _pending_gif_requests: Dictionary = {}
 var _last_animated_setting: bool = false
 var _focused_animated_texture: AnimatedTexture = null
 
@@ -69,7 +70,8 @@ func _is_animated_gifs_enabled() -> bool:
 func _on_focus_changed(control: Control) -> void:
 	# Pause the previously focused animation.
 	if _focused_animated_texture != null:
-		_focused_animated_texture.pause = true
+		if is_instance_valid(_focused_animated_texture):
+			_focused_animated_texture.pause = true
 		_focused_animated_texture = null
 
 	if control == null:
@@ -78,6 +80,8 @@ func _on_focus_changed(control: Control) -> void:
 	# Walk up the tree to find the GameCard parent.
 	var card: Control = control
 	while card != null:
+		if not is_instance_valid(card):
+			return
 		if card is GameCard:
 			break
 		card = card.get_parent()
@@ -132,9 +136,37 @@ func get_boxart(item: LibraryItem, kind: LAYOUT) -> Texture2D:
 		logger.warn("Game title is empty for id %d" % game_id)
 		return null
 
-	# Enrich on first access: scrape the itch.io page for screenshots.
+	var meta := await _enrich_metadata(game, game_id, title)
-	# Cached failures (empty screenshots) are stored to avoid repeated
+	var screenshots: Array = meta.get("screenshots", []).map(_sanitize_url)
-	# requests against Cloudflare-protected pages.
+	var cover_url := _sanitize_url(_best_portrait_url(game, screenshots))
+	if cover_url.is_empty():
+		logger.warn("No cover URL for: %s (id=%d)" % [title, game_id])
+		return null
+	logger.info("cover_url=%s" % cover_url.left(LOG_URL_MAX_LEN))
+
+	var cache_flags := Cache.FLAGS.NONE
+	if use_caching:
+		cache_flags = Cache.FLAGS.LOAD | Cache.FLAGS.SAVE
+
+	# Banner: combine two screenshots side-by-side when available.
+	if kind == LAYOUT.BANNER and screenshots.size() >= 2:
+		return await _fetch_banner_textures(item, screenshots, cache_flags)
+
+	var url := _url_for_layout(kind, cover_url, screenshots)
+	if url.is_empty():
+		logger.warn("URL for layout %s is empty (cover=%s)" % [str(kind), cover_url])
+		return null
+
+	logger.info("Fetching itch.io box art for: %s layout=%s url=%s" % [item.name, str(kind), url.left(LOG_URL_MAX_LEN)])
+	var texture: Texture2D = await _fetch_image(url, cache_flags)
+	if texture == null:
+		logger.warn("Image download returned null for: %s url=%s" % [item.name, url.left(LOG_URL_MAX_LEN)])
+	return texture
+
+
+## Enriches game metadata by scraping the itch.io page for screenshots.
+## Uses cached results when available.
+func _enrich_metadata(game: Dictionary, game_id: int, title: String) -> Dictionary:
 	var meta: Dictionary = _enriched_meta.get(game_id, {})
 	if meta.is_empty() and game.has("url"):
 		logger.info("Enriching metadata for: %s (url=%s)" % [title, game.get("url", "")])
@@ -150,42 +182,24 @@ func get_boxart(item: LibraryItem, kind: LAYOUT) -> Texture2D:
 		logger.info("Using cached metadata for: %s (%d screenshots)" % [title, meta.get("screenshots", []).size()])
 	else:
 		logger.warn("No URL in game dict for: %s" % title)
+	return meta
 
-	var screenshots: Array = meta.get("screenshots", []).map(_sanitize_url)
-	var cover_url := _sanitize_url(_best_portrait_url(game, screenshots))
-	if cover_url.is_empty():
-		logger.warn("No cover URL for: %s (id=%d)" % [title, game_id])
-		return null
-	logger.info("cover_url=%s" % cover_url.left(LOG_URL_MAX_LEN))
 
-	var cache_flags := Cache.FLAGS.NONE
+## Fetches two screenshots and combines them side-by-side for banner layout.
-	if use_caching:
+## Falls back to individual screenshots if the combine fails.
-		cache_flags = Cache.FLAGS.LOAD | Cache.FLAGS.SAVE
+func _fetch_banner_textures(item: LibraryItem, screenshots: Array, cache_flags: int) -> Texture2D:
-
+	logger.info("Fetching itch.io banner (2 screenshots) for: " + item.name)
-	# Banner: combine two screenshots side-by-side when available.
+	var tex_a := await _fetch_image(screenshots[0], cache_flags)
-	if kind == LAYOUT.BANNER and screenshots.size() >= 2:
+	var tex_b := await _fetch_image(screenshots[1], cache_flags)
-		logger.info("Fetching itch.io banner (2 screenshots) for: " + item.name)
+	if tex_a != null and tex_b != null:
-		var tex_a := await _fetch_image(screenshots[0], cache_flags)
+		return _combine_side_by_side(tex_a, tex_b)
-		var tex_b := await _fetch_image(screenshots[1], cache_flags)
+	# Banner combine failed — fall back to single-image path.
-		if tex_a != null and tex_b != null:
+	logger.warn("Banner screenshot fetch failed: tex_a=%s tex_b=%s" % [str(tex_a != null), str(tex_b != null)])
-			return _combine_side_by_side(tex_a, tex_b)
+	if tex_a != null:
-		# Banner combine failed — fall back to single-image path.
+		return tex_a
-		logger.warn("Banner screenshot fetch failed: tex_a=%s tex_b=%s" % [str(tex_a != null), str(tex_b != null)])
+	if tex_b != null:
-		if tex_a != null:
+		return tex_b
-			return tex_a
+	return null
-		if tex_b != null:
-			return tex_b
-
-	var url := _url_for_layout(kind, cover_url, screenshots)
-	if url.is_empty():
-		logger.warn("URL for layout %s is empty (cover=%s)" % [str(kind), cover_url])
-		return null
-
-	logger.info("Fetching itch.io box art for: %s layout=%s url=%s" % [item.name, str(kind), url.left(LOG_URL_MAX_LEN)])
-	var texture: Texture2D = await _fetch_image(url, cache_flags)
-	if texture == null:
-		logger.warn("Image download returned null for: %s url=%s" % [item.name, url.left(LOG_URL_MAX_LEN)])
-	return texture
 
 
 ## Finds the itch.io game dict from the library item's launch items.
@@ -298,6 +312,7 @@ func _build_ffmpeg_url(os_name: String, arch_name: String) -> String:
 ## Downloads bytes from a URL. Returns empty PackedByteArray on failure.
 func _download_url_bytes(url: String) -> PackedByteArray:
 	var http := HTTPRequest.new()
+	http.timeout = HTTP_TIMEOUT_MS / 1000.0  # HTTPRequest uses seconds.
 	add_child.call_deferred(http)
 	await http.ready
 
@@ -372,14 +387,30 @@ func _fetch_gif_as_texture(url: String, cache_flags: int) -> Texture2D:
 	if _animated_texture_cache.has(url):
 		return _animated_texture_cache[url]
 
+	# If another request for the same URL is in progress, wait for it.
+	if _pending_gif_requests.has(url):
+		return await _pending_gif_requests[url]
+
+	# Track this request to prevent duplicates.
+	_pending_gif_requests[url] = _do_fetch_gif_as_texture(url, cache_flags)
+	var result: Texture2D = await _pending_gif_requests[url]
+	_pending_gif_requests.erase(url)
+	return result
+
+
+## Internal implementation of GIF fetching. Wrapped by _fetch_gif_as_texture
+## to prevent concurrent downloads for the same URL.
+func _do_fetch_gif_as_texture(url: String, cache_flags: int) -> Texture2D:
+
 	var animated: bool = _is_animated_gifs_enabled()
 	var ffmpeg_bin := await _ensure_ffmpeg()
 	if ffmpeg_bin.is_empty():
 		return null
 
 	var globalized_dir := ProjectSettings.globalize_path(BOXART_DIR)
-	var gif_path := globalized_dir + "/_tmp.gif"
+	var url_hash := url.sha256_text().left(16)
-	var frames_dir := globalized_dir + "/_tmp_frames"
+	var gif_path := globalized_dir + "/_tmp_%s.gif" % url_hash
+	var frames_dir := globalized_dir + "/_tmp_%s_frames" % url_hash
 
 	# Download the GIF bytes via HTTPClient.
 	var body := await _download_gif_bytes(url)
@@ -399,7 +430,7 @@ func _fetch_gif_as_texture(url: String, cache_flags: int) -> Texture2D:
 	if animated:
 		result = await _build_animated_gif_texture(ffmpeg_bin, gif_path, frames_dir, body, url)
 	else:
-		result = await _build_static_gif_texture(ffmpeg_bin, gif_path, globalized_dir + "/_tmp.png", url)
+		result = await _build_static_gif_texture(ffmpeg_bin, gif_path, globalized_dir + "/_tmp_%s.png" % url_hash, url)
 
 	# Clean up.
 	DirAccess.remove_absolute(gif_path)
@@ -529,6 +560,8 @@ func _parse_gif_frame_delays(data: PackedByteArray) -> Array:
 ## Skips the GIF header, Logical Screen Descriptor, and Global Color Table.
 ## Returns the byte offset past all header data.
 func _skip_gif_header(data: PackedByteArray) -> int:
+	if data.size() < 13:
+		return data.size()
 	var pos := 13  # Header (6) + Logical Screen Descriptor (7).
 	var packed_byte: int = data[10]
 	if packed_byte & 0x80 != 0:
@@ -540,9 +573,11 @@ func _skip_gif_header(data: PackedByteArray) -> int:
 ## Parses an extension block starting at `pos`. Returns [delay_ms, new_pos].
 ## If the extension is not a Graphics Control Extension, returns [-1, new_pos].
 func _parse_gif_extension(data: PackedByteArray, pos: int) -> Array:
+	if pos + 2 >= data.size():
+		return [-1, data.size()]
 	var label: int = data[pos + 1]
 	var delay_ms := -1
-	if label == 0xF9:
+	if label == 0xF9 and pos + 5 < data.size():
 		var delay_cs: int = data[pos + 4] | (data[pos + 5] << 8)
 		delay_ms = delay_cs * 10 if delay_cs > 0 else GIF_DEFAULT_FRAME_DELAY_MS
 	pos += 2