Skip to content

Conversation

Qrox
Copy link
Contributor

@Qrox Qrox commented Feb 16, 2023

According to microsoft documentation on the dwReserved0 member of _WIN32_FIND_DATAW,

If the dwFileAttributes member includes the FILE_ATTRIBUTE_REPARSE_POINT attribute, this member specifies the reparse point tag.

Otherwise, this value is undefined and should not be used.

reparse_tag_from_INFO uses dwReserved0 without checking the FILE_ATTRIBUTE_REPARSE_POINT attribute, which causes directory_iterator to use a stale dwReserved0 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 checking dwReserved0 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.

Fix directory iterator treating all files subsequent to a symlink as symlink on Windows
@gulrak
Copy link
Owner

gulrak commented Feb 17, 2023

Thank you for the fix!

@gulrak gulrak merged commit 655b0b3 into gulrak:master Feb 17, 2023
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.


![image](https://github.com/user-attachments/assets/26e5d655-6a31-49eb-8514-3374b41f72dd)

---------

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants