Conversation
This comment was marked as resolved.
This comment was marked as resolved.
f667900 to
58e3a96
Compare
SuuperW
left a comment
There was a problem hiding this comment.
(and there might be stuff to update regarding the recent addition of LuaCATS documentation, but I haven't looked at that)
| { | ||
| if (!Guid.TryParseExact(id, format: "D", out var parsed)) | ||
| { | ||
| Log($"not a valid UUID: {id}"); |
There was a problem hiding this comment.
Something indicating that this message is about a branch operation would be good. Seeing this pop up in the output when running a Lua script, the user (even if they are the script author) might have no idea what this is about or what a UUID is. "not a valid branch id: {id}" would be better, I think.
| var index = Tastudio.CurrentTasMovie.Branches.Count - 1; | ||
| var branch = Tastudio.CurrentTasMovie.Branches[index]; | ||
| branch.UserText = text; | ||
| Tastudio.BranchSavedCallback?.Invoke(index); |
There was a problem hiding this comment.
I'd prefer changing the Branch method to return the instance, instead of grabbing it by index here.
Also, I think the Branch method should handle calling BranchSavedCallback. It looks like from reading related code that the callback isn't called if you branch with the context menu item, so you'd be fixing that too.
There was a problem hiding this comment.
The other existing callsites use the index.
The callback can't be moved to the end of Branch because of this:
BizHawk/src/BizHawk.Client.EmuHawk/tools/TAStudio/BookmarksBranchesBox.cs
Lines 227 to 229 in a2e8eed
There was a problem hiding this comment.
The other existing callsites use the index.
So? They don't grab the branch, and updating Branch to return the branch will not require changing any existing uses.
The callback can't be moved to the end of Branch because of this:
Because of what? The branch not having text at the end of Branch? It should be easy enough to place a call to EditBranchTextPopUp inside Branch depending on a parameter.
58e3a96 to
dc7c820
Compare
|
Is there a reason you decided to keep "UUID" in the error message? |
|
BizHawk/src/BizHawk.Client.EmuHawk/tools/Lua/Libraries/TAStudioLuaLibrary.cs Lines 537 to 539 in dc7c820 That is an accurate description of the failure. |
|
But at a level of detail most users will not understand. You changed the "UUID" in the Lua documentation to just "ID", so I thought this one might be an oversight. |
resolves #4073