Exposed FileLocationSettings in the Editor

[RhodeIfado]

I wanted to create a pull request to open the discussion on my changes.

Making FileLocationSettings non-static might be a breaking change for some users. If they used these static references in their own scripts, for example when you create additional log files or save any raw data from your experiment, they wont be able to access the static references in the same way as before.

I am unsure if this is actually a breaking change, and if so wether it's even worth it trying to implement the feature. It is essential to make FileLocationSettings non-static, to enable serialization.

[AdamBebko]

What version of Unity? I’ll try to confirm today. Sent from my iPhoneOn Apr 3, 2024, at 7:57 AM, Jakob Rhode-Brähler @.***> wrote: @RhodeIfado commented on this pull request.

In BML_TUX_Project/Packages/bmlTUX/Scripts/Settings/FileLocationSettings.cs:

-

• return tuxPath;
• }
• }
• public static string BaseTuxDocumentsFolderPath => Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.MyDocuments), tuxFolderName);
• public static string SessionFolder => BaseTuxDocumentsFolderPath;
• public static string DebugFolder => BaseTuxDocumentsFolderPath;
• public static string LastSessionSaveFilePath => Path.Combine(SessionFolder, LastSessionSaveFileName);
• public static string SessionLogFilePath => Path.Combine(SessionFolder, SessionLogFileName);
• }
• [Serializable]
• public class FileLocationSettings {

Thanks for your great advice! I did not know this feature existed. Testing this, I wasnt able to validate 6. It's possible I am not testing it correctly though. I updated a test project by copying a local version of the package and reimported everything. Not sure if it'll behave differently if it's getting an update from the package manager.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>

[RhodeIfado]

I added a fix for a bug I've introduced during session logging (1e284d70e48b8f497580aae6be13d5181ba95596)

[RhodeIfado]

I just found another bug, but I wont have time to fix it today. During LoadSessionDatain Session.cs, if an old lastSession.json is present without the newly added serialized fields saveFilePathand sessionFolder, the JsonUtility.FromJson will actually fill it with null. This causes an exception along the way.

This method could create a new Session object every time at the top and then use FromJsonOverwrite.

[RhodeIfado]

I've updated the samples to include the default reference to the FileLocationSettings Scriptable Object and added a quick fix regarding my recent comment. This fix doesnt feel like the smoothest way to deal with deserializing null values, but its alright. It's better than the fix mentioned in my previous comment though because its much clearer to read.

[RhodeIfado]

Have you had a chance to confirm it yet?

[AdamBebko]

Have you had a chance to confirm it yet?

sorry for the delay, I have a new baby. 👶

I got the same findings as you. I wrote a tool based on your branch to auto update user's settings using a dialog.

Could you give me permission to push a branch to your repo? I can PR to your master branch, you can take a look then we can pull it into bmlTUX. Maybe there's a better way for me to add my changes?

I had a chance to verify the file settings work. Looks good to me. I made one other small change to capitalize the default settings file name.

Thanks for your patience :D

[RhodeIfado]

Great to hear that you have become a dad!

I have added you as a collaborator on the fork, I am unsure if that is the correct kind of permission as I've never done anything like this on github.

[AdamBebko]

Great to hear that you have become a dad!

I have added you as a collaborator on the fork, I am unsure if that is the correct kind of permission as I've never done anything like this on github.

I created a PR for your review: https://github.com/RhodeIfado/TUX/pull/1

Once accepted and pushed to your main branch, it should automatically update your PR to my repo, and I'll do a couple small final tests, and we can merge.

[AdamBebko]

@RhodeIfado Merged into dev branch. Need to put it through the build pipeline, but should be released shortly. Thanks so much for your contribution. I'm sure it will make customization easier moving forward!

[AdamBebko]

:tada: This PR is included in version 5.0.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: