-
Notifications
You must be signed in to change notification settings - Fork 192
Fix directory iterator treating all files subsequent to a symlink as symlink on Windows #162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix directory iterator treating all files subsequent to a symlink as symlink on Windows
Thank you for the fix! |
xTVaser
pushed a commit
to open-goal/jak-project
that referenced
this pull request
Jul 31, 2025
The context for this is that i was symlinking files from a mod installation that were identical to the unmodded game, for space considerations, because i remembered having success with that in the past, before the project made the switch to ghc's filesystem library, it seems. <sub>(I was also much more pressed for space back then, but well, i thought writing a small program to check for matches and link them for me would be a good, not-so-difficult exercise...)</sub> However i quickly started getting weird errors that it couldn't find GAME.CGO or KERNEL.CGO even when i hadn't touched them. Initially since i found that it occurred only when symlinking any files that were earlier alphabetically, i assumed it was some kind of index mismatch, but it was actually caused by a bug in an older version of filesystem.hpp causing the `directory_iterator` to read everything after a symlink as also a symlink, making them fail to be read properly: gulrak/filesystem#162 Adding `f.is_symlink()` is not as much of a change to the loop's condition from the previous behavior as it might first appear. It seems that even without that condition the iterator *was* reading symlinks before, just incorrectly (and before ghc::filesystem, they were being read just fine, iirc). With the new version, though, symlinks do have to be accounted for explicitly. Given that the library's major and minor version are both the same, i don't expect there should be any breaking changes to the API, and i didn't find any when i was testing but there might need to be more investigation into that before merging. Also, without the added check for `f.exists()`, the behavior is exactly the same as if, for some reason, a fakeiso file the game needs just doesn't exist in the first place (which has no real reason to happen in a normal installation of the game): there's a decent chance it will either crash at some point later during runtime, or something will happen like a level failing to load causing Jak to trip infinitely. I thought it might be helpful to have it fail right away in the case that, say, someone moved their vanilla installation and staled their symlinks, but i'll defer to maintainers' judgement on that point.
dallmeyer
added a commit
to OpenGOAL-Mods/OG-Randomizer
that referenced
this pull request
Aug 25, 2025
* game: default MSAA to off (#3943) There's a suspicion that MSAA might be what causes the black screen problem for some users, additionally this can cause perf issues for people with really bad PCs so this is probably a better default. Needs testing before merging, i think this is all inclusive though. * New Crowdin updates (#3942) * Fix Jak II ISO hashes (#3946) Fixes the other Jak II hashes after the fix in open-goal/jak-project#3937 * [jak3] Add BRANCH grain, make handler IDs unique (#3950) Two changes that fix some Jak 3 audio bugs. In some areas, sound effects would seem to cut off randomly. This was caused by the sound code reusing IDs, but overlord didn't realize. It would try to kill a sound effect by ID that had already died, and the ID was reassigned to a new sound. To fix this, I made the handle IDs always increment. I also tested starting the ID at large numbers and confirmed that worked. I also added support for the BRANCH grain. This makes the `wastelander-shot` and possibly other sound effects work. It doesn't support all the features, but this is probably enough. Fun fact: the wastelander shot is supposed to have 7 variations, but you always get the same one because they set the sound mask wrong. I added a line you can uncomment if you want to experience the other 6 variations. --------- Co-authored-by: water111 <awaterford1111445@gmail.com> * remove some spammy overlord prints (#3952) removing some print statements left over from debugging. Co-authored-by: water111 <awaterford1111445@gmail.com> * [jak2/3] Support yakows in custom levels (#3951) I discovered that `yakow`s are kinda broken if you try to use them in custom levels in jak 2/3. It's due to the missing `yakow-lod0` texture and associated fix, replacing it with `yak-medfur-end`. This solution works fine for the decompiler on vanilla levels. But for building custom levels, the requested art-groups were being handled before the textures were, and so it was impossible to have `yak-medfur-end` on hand to do the replacement. You'd hit an exception here because `idx_in_level_texture` would still be `INT32_MAX`: https://github.com/open-goal/jak-project/blob/c08118509b84feba002bd9e208f49162b4218556/decompiler/level_extractor/extract_merc.cpp#L806 My fix was just to swap the order when building custom levels, and handle the textures first. I only made the changes for jak2/3, because I see @Hat-Kid has a slightly different implementation for jak1. There's one other small change relating to the `combo_id` / `pc_combo_tex_id` short-circtuiting - I think `pc_combo_tex_id` is always 0 for vanilla textures? So initially `yakow-lod0` actually ended up matching the `combo_id` of the checkerboard texture from the test-zone GLB. I just added another sanity check here that the texture names match too. (I also added yakows in the test-zone.jsonc files 🐄) * [gfx] Fix eye-related crash, clear color logic (#3957) When building the eye texture, the background is first set to the top corner of the iris texture. A long, long time ago, I implemented this by peeking at the data in the texture itself. This doesn't work if the iris texture is animated since the texture will only on the GPU. To fix this, this PR changes the eye renderer to draw a square over the entire eye texture using iris texture's top corner, avoiding the need for getting the texture data on the CPU. I don't remember why I didn't do this in the first place, but this seems better in every way. Also `input_data` in TexturePool not being initialized, which was leading to hard-to-debug crashes when it was randomly initialized to a sometimes invalid but non-null pointer. Co-authored-by: water111 <awaterford1111445@gmail.com> * [jak1] Fix speedrun mode bug where loading a save is counted as new run (#3960) Fixes a small bug introduced in #3902. Without this change, whenever you load a save it is treated as if you started a new game and resets the autosplitter back to 0s. * [jak3] Subtitle typo (#3961) * [jak2] Fix Fortress Crash by Patching Mips2C Function (#3963) Fixes #3151 by applying the same fix as #2686 to another Mips2C function. Tested by mashing Jak's body into the bad collision walls for 30 minutes without a single crash. * Fail building custom level if long_name is too long (#3966) If long_name is too long, it will result in the level being invisible but the collide loading, lets detect this and point the user in correct direction to solve it with a clear error message. --------- Co-authored-by: Hat Kid <6624576+Hat-Kid@users.noreply.github.com> * Some jak 3 transcription fixes (#3893) * Updating UK English spelling - Cutscene (#3962) Fixing some of the warrior's spelling for British English. * CI: Periodic Controller Database Update (#3953) Updating Controller Database Co-authored-by: OpenGOALBot <OpenGOALBot@users.noreply.github.com> * Assert - added missing macros (#3968) Compilation with the NO_ASSERT flag set results in errors. This happens because some the macros are not defined in the else branch of #ifndef NO_ASSERT. * CI: Periodic Controller Database Update (#3973) Updating Controller Database Co-authored-by: OpenGOALBot <OpenGOALBot@users.noreply.github.com> * optional linting: only fetch master branch (#3972) * game: fix level select for untranslated languages and fix infinite recursion edge-case in `lookup-text!` (#3969) The main bug this fixes is not being able to use level select when your language isn't translated. Why? Because they use the result from `lookup-text!` to determine if a menu item should be drawn or not. No text found, no menu item. However this uncovered a long-standing bug in my fallback extension to this code. If you call this function with a text-id that doesnt even have an english string, it will recursively keep calling itself until it crashes. Of course the first few tasks in the game are dummies and have no valid text-id so this had to be fixed. Should be fixed for all 3 games. * display: be more generous when filtering resolutions by refresh rate (#3974) Some monitors are very specific about their reported refresh rates: ``` [54:37] [debug] [DISPLAY]: Skipping 640x480 as it requires 360.11hz but the monitor is currently set to 360hz ``` The current code truncated the display mode, but would compare against the reported refresh rate. As per the log, they are obviously not equal. - Retain the float component and only round it off when sending the value to GOAL - Make the comparison more generous, +/- 1.0hz * deps: update to SDL 3.2.16 (#3975) * g/jak1: Extract ambient data to json (#3945) I added extraction of ambients to json files when extracting the levels. All of the ambient json files are written to the same folder as actors are. Ambients aren't used in jak2 and 3 so it only is used on jak1.  --------- Co-authored-by: Tyler Wilding <xtvaser@gmail.com> * CI: Periodic Controller Database Update (#3977) Updating Controller Database Co-authored-by: OpenGOALBot <OpenGOALBot@users.noreply.github.com> * game: fix refresh rate filtering comparison (#3984) It helps to compare in the right direction -- was having the inverse effect. * [jak1] Allow skipping credits in speedrunner mode (#3981) Speedrunners have seen the Jak 1 credits once or twice, I think we can let them skip * CI: Periodic Controller Database Update (#3982) Updating Controller Database Co-authored-by: OpenGOALBot <OpenGOALBot@users.noreply.github.com> * CI: Periodic Controller Database Update (#3989) Updating Controller Database Co-authored-by: OpenGOALBot <OpenGOALBot@users.noreply.github.com> * Replace PCRTC framebuffer blit with screen quad draw (#3980) Taking the suggestion from @Calinou (open-goal/jak-project#3943 (comment)), this replaces the resolve/render framebuffer -> window framebuffer blit with an actual drawn tri-strip which covers the entire viewport, which the PCRTC blackout already does. It appears we have no guarantee what state the internal window framebuffer will be in, so drawing an actual primitive and letting the fragment shader do all the work seems to be the more compatible/functional solution here. Thanks for the suggestion! * Update filesystem.hpp dependency and un-break symlinks (#3990) The context for this is that i was symlinking files from a mod installation that were identical to the unmodded game, for space considerations, because i remembered having success with that in the past, before the project made the switch to ghc's filesystem library, it seems. <sub>(I was also much more pressed for space back then, but well, i thought writing a small program to check for matches and link them for me would be a good, not-so-difficult exercise...)</sub> However i quickly started getting weird errors that it couldn't find GAME.CGO or KERNEL.CGO even when i hadn't touched them. Initially since i found that it occurred only when symlinking any files that were earlier alphabetically, i assumed it was some kind of index mismatch, but it was actually caused by a bug in an older version of filesystem.hpp causing the `directory_iterator` to read everything after a symlink as also a symlink, making them fail to be read properly: gulrak/filesystem#162 Adding `f.is_symlink()` is not as much of a change to the loop's condition from the previous behavior as it might first appear. It seems that even without that condition the iterator *was* reading symlinks before, just incorrectly (and before ghc::filesystem, they were being read just fine, iirc). With the new version, though, symlinks do have to be accounted for explicitly. Given that the library's major and minor version are both the same, i don't expect there should be any breaking changes to the API, and i didn't find any when i was testing but there might need to be more investigation into that before merging. Also, without the added check for `f.exists()`, the behavior is exactly the same as if, for some reason, a fakeiso file the game needs just doesn't exist in the first place (which has no real reason to happen in a normal installation of the game): there's a decent chance it will either crash at some point later during runtime, or something will happen like a level failing to load causing Jak to trip infinitely. I thought it might be helpful to have it fail right away in the case that, say, someone moved their vanilla installation and staled their symlinks, but i'll defer to maintainers' judgement on that point. * binaries --------- Co-authored-by: Tyler Wilding <xTVaser@users.noreply.github.com> Co-authored-by: water111 <48171810+water111@users.noreply.github.com> Co-authored-by: water111 <awaterford1111445@gmail.com> Co-authored-by: massimilianodelliubaldini <8584296+massimilianodelliubaldini@users.noreply.github.com> Co-authored-by: ZedB0T <89345505+Zedb0T@users.noreply.github.com> Co-authored-by: Hat Kid <6624576+Hat-Kid@users.noreply.github.com> Co-authored-by: Aloqas <102683375+Aloqas@users.noreply.github.com> Co-authored-by: MuchWhittering <149691053+MuchWhittering@users.noreply.github.com> Co-authored-by: OpenGOAL Bot <99294829+OpenGOALBot@users.noreply.github.com> Co-authored-by: OpenGOALBot <OpenGOALBot@users.noreply.github.com> Co-authored-by: cla.grisenti <91843204+clagrisenti@users.noreply.github.com> Co-authored-by: tripp <86533397+trippjoe@users.noreply.github.com> Co-authored-by: Soggy_Pancake <54160598+Soggy-Pancake@users.noreply.github.com> Co-authored-by: Tyler Wilding <xtvaser@gmail.com> Co-authored-by: Alex <7569514+ManDude@users.noreply.github.com> Co-authored-by: twi <shado0909@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
According to microsoft documentation on the
dwReserved0
member of_WIN32_FIND_DATAW
,reparse_tag_from_INFO
usesdwReserved0
without checking theFILE_ATTRIBUTE_REPARSE_POINT
attribute, which causesdirectory_iterator
to use a staledwReserved0
value for all files subsequent to a symlink, causing all of these files to be treated as symlink incorrectly.This patch checks
FILE_ATTRIBUTE_REPARSE_POINT
before checkingdwReserved0
to correctly get the symlink status of the files. Also replaced a hack that uses structure size to determine structure type with proper templates.This patch is tested on CleverRaven/Cataclysm-DDA#63612 and cherrypicked from that PR.