Skip to content
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

Allow Portable Installs #2556

Merged
merged 9 commits into from
May 9, 2024

Conversation

RicardoLuis0
Copy link
Contributor

This allows data to be stored in a PortableAppdata folder instead of in persistentDataPath if an empty Portable.txt file is in the same folder as the executable. If the install is portable, it also
opens the browse window on the exe folder and stores paths as relative in the ini.

@RicardoLuis0 RicardoLuis0 changed the title Allow storing data in exe folder rather than Appdata Allow Portable Installs Dec 31, 2023
@RicardoLuis0 RicardoLuis0 marked this pull request as draft December 31, 2023 23:32
@RicardoLuis0 RicardoLuis0 marked this pull request as ready for review January 1, 2024 00:21
@KABoissonneault
Copy link
Collaborator

I've seen a few people comment that this change would be useful to them, so we should probably review this.
I don't know about asking users to create a dummy file to enable the feature, but I'm admittedly not familiar with how this is usually done - I assume through an installer wizard setting that does something similar.
I think the feature might be good for now, I just need to find the best way to test that no outside files get created. We can figure out about the user experience enabling this later

@RicardoLuis0
Copy link
Contributor Author

from what i've checked when writing it, everything that fetches/stores data goes through PersistentDataPath/MyDaggerfallPath/MyDaggerfallUnitySavePath/MyDaggerfallUnityScreenshotsPath, which i've addressed, but if i've missed something, it should be simple enough to add

@KABoissonneault KABoissonneault added this to the DFU 1.0.1 milestone Feb 3, 2024
Copy link
Collaborator

@KABoissonneault KABoissonneault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial selection should have a way to back out of the current folder

Assets/Scripts/Game/UserInterface/FolderBrowser.cs Outdated Show resolved Hide resolved
the `RefreshFolders` call on `Update` also broke the return path, so i moved it into RefreshFolders itself
Copy link
Collaborator

@KABoissonneault KABoissonneault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change mostly works as expected now. Except, we still generate a Player.log in the AppData folder, Unity's idea of the Persistent Path. I'm testing some workarounds, but until we resolve that, I think this PR fails at achieving the goal of not modifying the environment outside the installation

@KABoissonneault KABoissonneault removed this from the DFU 1.0.1 milestone Feb 14, 2024
@KABoissonneault KABoissonneault mentioned this pull request Mar 3, 2024
# Conflicts:
#	Assets/Scripts/Game/UserInterface/FolderBrowser.cs
#	Assets/Scripts/SettingsManager.cs
@KABoissonneault
Copy link
Collaborator

I've updated this feature with the new "DaggerfallUnityApplication", so that log files are properly in the portable settings folder. The feature should be good to go.

@KABoissonneault KABoissonneault added this to the DFU 1.1.1 milestone Apr 12, 2024
@KABoissonneault
Copy link
Collaborator

KABoissonneault commented Apr 14, 2024

Actually, one thing about this feature is that it saves the Daggerfall install and the save/screenshot folders as a relative path. However, I don't think we should do that if those folders are outside the DFU installation. Relative for inner folders is fine, since we want them to relocate if users move those folders. Gonna rework the feature to actually check for that before saving as relative.

Note that it's already possible to put your DF installation under StreamingAssets/GameFiles.

@RicardoLuis0
Copy link
Contributor Author

Relative for inner folders is fine, since we want them to relocate if users move those folders.

ah, sorry, i didn't take into account that someone might do that 😅 (point it to an outside folder)

@KABoissonneault
Copy link
Collaborator

In addition to the changes described above, I've reverted the changes to the FolderBrowser (and associated changes to ListBox.cs). The change was made with the idea that people would use this to embed their DF install into the DFU installation, but the proper way to do this is to put the DF install in StreamingAssets/GameFiles. There is no reason to make the FolderBrowser behave differently otherwise.

I have a test Win64 build here for reviewers, with the Portable.txt file already included. Simply run it, it shouldn't affect your global DFU settings at all.
https://mega.nz/file/f1Z0lJaQ#dpJYkImQhRX4NczAmkSshuWzvVWquAKm86eM1nJQzGs

@KABoissonneault
Copy link
Collaborator

I'd really like to get this merged for the v1.1.1 official release soon. I have demo builds you can download, should be really easy to test

@KABoissonneault KABoissonneault merged commit 6d62f17 into Interkarma:master May 9, 2024
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.

3 participants