Friendly0Fire / GW2Radial

A customizable radial menu overlay addon for Guild Wars 2.
MIT License
346 stars 55 forks source link

New folder C:\GUILD WARS 2 created after starting GW2 following a patch #255

Closed Charybdis closed 2 years ago

Charybdis commented 3 years ago

Bug Description So this is going to be a wierd one, so happy to help out however I can...

Guildwars 2 is installed into the folder C:\GW2 and, after a game client update, a new folder C:\GUILD WARS 2 is created when first run under DX9 following the patch. The folder owner is my user account.

This new folder contains a GW2Radial config file at C:\GUILD WARS 2\addons\gw2radial\config.ini. The file is 0 bytes long.

The date/time stamp of the folders and config.ini file matches the first time the game client is run under DX9 (game is defaulted to DX11 beta).

To Reproduce Steps to reproduce the behavior:

  1. Enable DX11 beta in GW2 64-bit
  2. Note DX9 dlls for arcdps, gw2radial and d912pxy are in the bin64 folder
  3. Download patch under DX11 (game starts normally, folder is not created)
  4. Run GW2 under DX9 with the -dx9 command option
  5. Note the C:\GUILD WARS 2 is created (game otherwise runs normally)
  6. End game. Delete folder C:\GUILD WARS 2
  7. Restart GW2 under -dx9
  8. Note folder is not recreated.

Not always easy to reproduce, this is my current best understanding.

Expected behavior The folder C:\GUILD WARS 2 is not created.

Screenshots n/a

System configuration (please fill in):

Additional context The folder being created would otherwise appear in the %USERPROFILE%\Documents folders. The folder GUILD WARS 2 does exist here.

Looking through the code (former developer here) I can see GetAddonFolders() in Utility.h creates a partial path if myDocuments = nullptr is assigned due to a FAILED SHGetKnownFolderPath.

The folder C:\GW2\addons\gw2radial does exist (which is checked first in GetAddonFolder()), so it appears that is not being found either.

There is also a new folder created for ARCDPS at C:\GUILD WARS 2\addons\arcdps, so I suspect it has the same issue.

Friendly0Fire commented 2 years ago

There's a pretty good chance that the -dx9 flag is causing the issue, for instance by restarting the game with the wrong path or with weird permissions. The fallback to Documents is there for the cases where Administrative permissions are required to write in the Program Files folder, but it could also occur if the permissions are lowered below that of a regular user for some reason.

Either way, I double-checked edge cases for the code I have implemented and couldn't find anything that would indicate they'd return C:\ as a fallback at any point, so unfortunately I don't think I can do anything.

Charybdis commented 2 years ago

Thanks for taking a look, and I appreciate that these kinds of bugs are difficult to triage and reproduce.

Having said that, the edge case is already present in the code and causing the issue: "nullptr is assigned due to a FAILED SHGetKnownFolderPath"

The code proceeds as if it has a valid folder path to work with, when a detectable error has already (silently) occured.

This is the code fragment in utility.h GetAddonFolders() that creates the bad path with myDocuments = nullptr:

const auto myDocumentsLocation = std::wstring(NULL_COALESCE(myDocuments, L"")) + L"\GUILD WARS 2\addons\gw2radial\";

The path created is "\GUILD WARS 2\addons\gw2radial\", and it defaults to the current drive (in my example C:\ because that is the drive where I have the game installed). If the game were installed on drive D it would be D:\ as the root folder.

Would you reconsider the logic here so that the config file isn't written in this scenario? SHGetKnownFolderPath has failed to obtain a well defined folder.

Thanks for looking at this again 👍

Friendly0Fire commented 2 years ago

Okay, I think I found the issue, it was actually a problem in CheckFolder which didn't check for existence before checking for write access, so it'd create a folder in the partial/wrong path. I've made a few changes to try to harden it against spurious file creation, but at the end of the day the fact it's not finding your Documents folder still implies something's weird/wrong about your Windows install or the way Guild Wars 2 restarts when toggling the flag which would probably be worth investigating in parallel.

This'll be in the next update.

Charybdis commented 2 years ago

Thanks for taking another look. That should do the job.