Skip to content

fix tiny var bug - #2547

Merged
fenhl merged 1 commit into
OoTRandomizer:Devfrom
ThamanVB:patch-1
Apr 3, 2026
Merged

fix tiny var bug#2547
fenhl merged 1 commit into
OoTRandomizer:Devfrom
ThamanVB:patch-1

Conversation

@ThamanVB

@ThamanVB ThamanVB commented Apr 2, 2026

Copy link
Copy Markdown

I was just looking through the repo to understand it better and noticed this error is using the wrong var

Testing

I did not test the thrown exception, and in fact I don't think this exception can even trigger due to the check on line 257, but for some reason the undefined var ref bothered me enough to bring attention to this lol.

The only reason I didn't remove the try catch in my PR is because if memory serves Windows can have os.path.isfile return true but still fail to open the file with a file not found error if the file path is too long or something

I was just looking through the repo to understand it better and noticed this error is using the wrong var
@mracsys

mracsys commented Apr 2, 2026

Copy link
Copy Markdown

Looks good. I fixed this when I noticed it in #2332 as well, but it’s not related and shouldn’t be held up by that.

@fenhl

fenhl commented Apr 3, 2026

Copy link
Copy Markdown
Collaborator

I think this could be triggered via TOCTOU, but regardless, I'd say this is minor enough that testing can be skipped. Thanks for the PR!

@fenhl fenhl added Type: Bug Something isn't working Component: Documentation Affects user-facing help messages or public API docs labels Apr 3, 2026
@fenhl fenhl added this to the next milestone Apr 3, 2026
@fenhl

fenhl commented Apr 3, 2026

Copy link
Copy Markdown
Collaborator

While I'm merging this, I'm also going to fix the indentation.

@fenhl
fenhl merged commit a4567d5 into OoTRandomizer:Dev Apr 3, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Documentation Affects user-facing help messages or public API docs Type: Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants