X Tutup
Skip to content

Add back I/O error-handling to FileAccessPack constructor#113150

Merged
Repiteo merged 1 commit intogodotengine:masterfrom
mihe:file-access-pack-errors
Nov 25, 2025
Merged

Add back I/O error-handling to FileAccessPack constructor#113150
Repiteo merged 1 commit intogodotengine:masterfrom
mihe:file-access-pack-errors

Conversation

@mihe
Copy link
Contributor

@mihe mihe commented Nov 25, 2025

This adds an ERR_FAIL_* check to the FileAccessPack constructor, to make sure we don't crash in the non-sparse case, by calling FileAccess::seek on a potentially null reference.

As far as I can tell, this was inadvertently changed/removed as part of #105984.

I also rephrased the error messages there to log not only the PCK name but also the filename itself.

@mihe mihe requested a review from bruvzg November 25, 2025 13:31
@mihe mihe requested a review from a team as a code owner November 25, 2025 13:31
@github-project-automation github-project-automation bot moved this to For team assessment in Core Issue Triage Nov 25, 2025
@AThousandShips AThousandShips added this to the 4.6 milestone Nov 25, 2025
@mihe mihe force-pushed the file-access-pack-errors branch from ab35c1e to 985a5e9 Compare November 25, 2025 13:38
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change make sense, this should not happen, but I guess can be possible if PCK was deleted (or become unreadable in some other why) while the process is running.

@mihe
Copy link
Contributor Author

mihe commented Nov 25, 2025

this should not happen

Agreed. I'm looking into a bug right now that might possibly be forcing this error in some weird way though. I figured it wouldn't hurt to add this.

@Repiteo Repiteo merged commit f34c270 into godotengine:master Nov 25, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 25, 2025

Thanks!

@mihe mihe deleted the file-access-pack-errors branch November 25, 2025 20:21
@Ivorforce Ivorforce moved this from For team assessment to Done in Core Issue Triage Feb 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants

X Tutup