Skip to content

Fix local traditional mapping not taking effect#27

Closed
fro0116 wants to merge 1 commit intoTimMangan:developfrom
fro0116:fix-local-traditional-mapping
Closed

Fix local traditional mapping not taking effect#27
fro0116 wants to merge 1 commit intoTimMangan:developfrom
fro0116:fix-local-traditional-mapping

Conversation

@fro0116
Copy link

@fro0116 fro0116 commented Sep 13, 2024

Hi there!

I have an app with all of the local redirections set to "traditional" to make sure all app state gets persisted in the same central location.

The app still ended up writing some files to the real Documents folder for some reason, so I dug into the code and came up with this change that seems to redirect those files into VFS/Profile/Documents in my local build.

That still doesn't feel quite right since I'd assume it would redirect to VFS/Personal directly judging from the description in the readme, whereas in this case, it seems to be disabling the rule completely and the redirection only happens because of the Profile traditional redirect.

That said, the current behavior still works for me, and I didn't want to make any more drastic changes without consulting with you first.

Let me know whether this behavior is expected, happy to make any changes you'd like to see to get it merged. Cheers!

@TimMangan TimMangan self-assigned this Sep 13, 2024
@TimMangan
Copy link
Owner

I am going to do a deeper dive on the code when I get a chance - I am also a little cautious. But the default behavior of MfrFixup should be to direct anything writes to the package documents folder to the real documents folder outside of the container, and configuration would be needed to not do that. I haven't played with that code in a while, but would have thought that disabled is what you wanted; possibly the issue is really a documentation issue.

VFS\Personal is the correct reference we want to use, and VFS\Profile\Documents should be considered an alias for read purposes - not clear in the code but enforced by orderings in the mappings.

@TimMangan
Copy link
Owner

TimMangan commented Sep 17, 2024

I don't believe the proposed change will have any effect, because the new_map is also set to be disabled. I think that the new mapping should be enabled, but the RedirectionFlags changed. There are two possibly valid values, however (2 and 3). The effective differences are as follows:

  • prefer_redirection_containerized = 0x0002 would cause redirection to any document file, whether in the package or in the native location from some other app, while leaving the native file unchanged.
  • prefer_redirection_if_package_vfs = 0x0003 would cause redirection to any package file opened for write, but a file in the user's normal documents folder would not be redirected.

My instinct is to use 3. But I want to think about this a bit. What did you have in mind for how this would work?

@fro0116
Copy link
Author

fro0116 commented Sep 18, 2024

Hey Tim, appreciate the follow up. :)

I'm not a C++ expert by any measure, so I'm not quite sure why the change has this effect either, but for a package that has all local redirections set to "traditional" like so:

             "overrideLocalRedirections": [
                {
                    "name" : "ThisPCDesktopFolder",
                    "mode" : "traditional"
                },
                {
                    "name" : "Personal",
                    "mode" :  "traditional"
                },
                {
                    "name" : "Common Desktop",
                    "mode" :  "traditional"
                },
                {
                    "name" : "Common Documents",
                    "mode" :  "traditional"
                }
             ],
  • with the current release build of MFR, my packaged app ends up writing to the user's real document folder
  • with a local build for this branch, my packaged app ends up writing to WritablePackageRoot/VFS/Profile/Documents, which I believe is the same behavior as if I'd set mode to "disabled"

My understanding from reading the docs is that "traditional" mode should give these the same behavior as the "traditional" redirections, which would be to write everything to WritablePackageRoot/VFS/${FolderName}, which in this case should be WritablePackageRoot/VFS/Personal.

My use case involves an app that writes its own internal app state to the user's Documents folder, so I'd like to have it fully encapsulated inside the WritablePackageRoot, so prefer_redirection_containerized sounds like a better fit.

Btw, please feel free to close this PR if it's the wrong approach (sounds like it probably is). Just wanted to get some clarity on the expected behavior here, so if you have ideas for a proper fix, please go for it. 🙏

@TimMangan
Copy link
Owner

Closing this PR; After some thought, it will be replaced with another PR that changes the mapping correctly to prefer_redirection_if_package_vfs.

@TimMangan TimMangan closed this Sep 18, 2024
@TimMangan
Copy link
Owner

Closing this PR; After some thought, it will be replaced with another PR that changes the mapping correctly to prefer_redirection_if_package_vfs.

Version v.6.0.0 altered the default settings by making the following folders default to local:

  • Documents
  • Desktop
  • Music
  • Library
  • Video

I'll also note that the TEMP folders (user and global) are in a disabled state, meaning don't ever redirect and don't look for a package version.

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