[REG-2105] Allow absolute paths to be used in segment and sequence loading

[vontell]

There was a bug where sequences and segments could not be loaded from the application data path in builds. I am not super familiar with all of the logic here so wanted to take a minute to explain my observations and decisions here:

• I was getting errors like this in production builds: Error reading Bot Sequence /Users/aaronvontell/Library/Application Support/DefaultCompany/normal/RegressionGames/Resources/BotSequences/Latest_Recording.json: System.Exception: Invalid path. Path must be relative, not absolute in order to support editor vs production runtimes interchangeably.
• In the editor, paths look like Assets/RegressionGames/Resources/BotSequences/asdasd.json
• When in a build, the paths look like /Users/aaronvontell/Library/Application Support/com.DefaultCompany.This--should-break-/RegressionGames/Resources/BotSequences/Latest_Recording.json
• The code for loading these sequences had an explicit catch for JSONs coming from an absolute path, and wouldn't allow it.
• The fix I found was to remove that check, but this seemed weird... because we are basically taking the absolute path, trimming off most of it in the ToResourcePath function, and then reconstructing it... and the LoadJsonResource function knows how to handle both scenarios.

An alternative to my change is to only call ToResourcePath inside of the UNITY_EDITOR scenarios and then use the path directly inside of the else block, so that the path doesn't need to be reconstructed. However, I wanted to check with @RG-nAmKcAz and @addisonbgross that this is still fine considering the "override" behavior, or if there is additional logic I need to consider.

At least on mac, I confirmed that this allows us to both create and see sequences that are in the application data path, and everything behaves as expected in builds and in editor. Note, however, that the sequences and segments inside the application data path do not show in the editor (which I believe is expected, but wanted to confirm).

[nAmKcAz]

@vontell none of that sounds right to me at first read.. i'll go recreate this and clarify

[addisonbgross]

@vontell I ran this locally on Windows, and everything seems to be working correctly in-editor and in a build.
I'm not entirely sure about the ramifications of this change, so I will leave that final call to @RG-nAmKcAz ?

[nAmKcAz]

@vontell On Windows, everything works without this change. Can you include the full call stack for the error you're getting so we can see what codepath on mac is producing an unexpected path into that call.

Taking these checks out is not valid and will prevent sequence loading errors from displaying correctly in a few other scenarios that require relative paths.

See new Update below after testing on Mac.

[nAmKcAz]

This works on Windows because the path is starting with C:\ not \ or /

Furthermore, looking into how this prompt was interacting with command line sequence runner options, I think this will be safe there as well and should support full or relative paths.

This check existed before I started auto normalizing everything to a resourcePath and then running it through the loading evaluator to make sure we always loaded the override copy, not accidentally something else.

[vontell]

Just to confirm, here is a loom recording and attached logs of a complete reproduction from scratch on RGUnityBots main: https://www.loom.com/share/e66397c926ff4f39acb20e8ac86195a7?sid=56362bb9-05ca-4fb7-8fb1-044fffe58cb1

Player.log

The C:\ explanation certainly makes sense now... Yeah I figured that this was a remnant of code left over from the before the recent changes, I noticed this code was about 3 weeks old.

@RG-nAmKcAz great point about the recent command line option... will this actually work with that? Because we do manipulate the path to ensure it starts with are particular path... what if my sequence is just located in a random folder? Or does that take another route? I'll take a look myself

Update: I do get an error when doing this, will be able to inspect more why that is.

Command (for mac): open -n Build.app --args -rgsequencepath "/Users/aaronvontell/Desktop/Can-do-this-in-editor.json" -rgsequencetimeout 7

Result

{{RG}} 2024-10-09T17:01:46:7900 INFO [1] --- Regression Games ECS Package not found, support for ECS won't be loaded
{{RG}} 2024-10-09T17:01:46:8153 INFO [1] --- Supported Formats for Readback
R8_SRGB
R8G8_SRGB
R8G8B8A8_SRGB
R8_UNorm
R8G8_UNorm
R8G8B8A8_UNorm
R16_UNorm
R16G16_UNorm
R16G16B16A16_UNorm
R16_SFloat
R16G16_SFloat
R16G16B16A16_SFloat
R32_SFloat
R32G32_SFloat
R32G32B32A32_SFloat
B5G6R5_UNormPack16
B10G11R11_UFloatPack32
{{RG}} 2024-10-09T17:01:46:8402 INFO [1] --- Regression Games bot sequence playback - loading and starting bot sequence from path: /Users/aaronvontell/Desktop/Can-do-this-in-editor.json , with timeout: 7
Regression Games bot sequence playback - exception loading bot sequence from path: /Users/aaronvontell/Desktop/Can-do-this-in-editor.json
NullReferenceException: Object reference not set to an instance of an object
 at RegressionGames.StateRecorder.BotSegments.Models.BotSequence.LoadJsonResource (System.String path) [0x000bd] in <067641b95b62473785e53a4c02ef4aa4>:0 
Rethrow as Exception: Exception reading json files from resource path: /Users/aaronvontell/Desktop/Can-do-this-in-editor
 at RegressionGames.StateRecorder.BotSegments.Models.BotSequence.LoadJsonResource (System.String path) [0x000e7] in <067641b95b62473785e53a4c02ef4aa4>:0 
 at RegressionGames.StateRecorder.BotSegments.Models.BotSequence.LoadSequenceJsonFromPath (System.String path) [0x00041] in <067641b95b62473785e53a4c02ef4aa4>:0 
 at RegressionGames.RGHeadlessSequenceRunnerBehaviour.Update () [0x0004f] in <067641b95b62473785e53a4c02ef4aa4>:0 
UnityEngine.DebugLogHandler:Internal_LogException(Exception, Object)
UnityEngine.DebugLogHandler:LogException(Exception, Object)
UnityEngine.Logger:LogException(Exception, Object)
UnityEngine.Debug:LogException(Exception)
RegressionGames.RGDebug:LogException(Exception, String, Object)
RegressionGames.RGHeadlessSequenceRunnerBehaviour:Update()

[nAmKcAz]

@vontell If you want to move the relative paths only check to the option loader that seems most appropriate...

but that option is NOT intended to work with arbitrary paths. Sequences/Segments MUST be loaded from the persistent data path or inside the build, thus the normalization to resource paths.

IF you want to re-write that concept you can, but this ticket seems to be moving from fix a bug on mac to add new functionality to the command line option

[vontell]

I see! Just want to confirm this before I merge then since the last time I thought about this was last week:

• My current fix is all good
• It won't work for arbitrary paths for the new command line parameter, which is fine (and we should document that the command line option should only be used for sequences contained in the build / persistent data path).

If that is correct, please provide a thumbs up and I will merge!

[nAmKcAz]

@vontell I would add a check to the option loader asserting that it is a relative path for now. Similar to the existing code, but maybe with an extra bit to save us from c: , e:, etc

Doesn't start with '/' or '' and doesn't have a second char of ':'

[nAmKcAz]

RGHeadlessSequenceRunner around line 110 ish , just before we return the path

[vontell]

@RG-nAmKcAz will do 👍

[vontell]

Apologies for the delay on this. @RG-nAmKcAz please see my latest commit for that change - confirmed that the sequence path works properly for relative paths and does print the error message in the logs when it is absolute (the Path utility I use here should properly handle both Windows disks and unix systems)

[nAmKcAz]

Changes look good. Just need a unique RC for that case

[nAmKcAz]

This should be a unique RC

[vontell]

Good catch, added