-
Notifications
You must be signed in to change notification settings - Fork 428
Do not disable recording mode when loading a branch. #4475
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
base: master
Are you sure you want to change the base?
Conversation
Loading a branch in rec mode is the same as loading a savestate without tastudio and then continuing to record. Pretty standard workflow. Obviously loading a branch changes inputs, but it's not a part of manual editing. |
Does loading a branch always pause? |
No. |
Maybe it should when recording mode is active. |
Is that suggestion based on actual TASing workflow that someone uses all the time? |
This fallacy again? I'll bite: #2144 (comment), and I'm sure I've also seen users confused by recording mode clearing inputs. But to actually reason through my suggestion, it's weighing the "false positives"—pausing when the user intended to keep recording immediately after loading the branch, requiring the user unpause—against the "false negatives"—clearing inputs in the new branch, requiring the user pause, undo (see #2143), possibly disable recording mode, and unpause. We may not know which is more common, but the former is obviously easier for the user to remedy. Another possibility would be to disable recording mode if loadstating while unpaused, but re-enabling that is no easier than unpausing, and based on #2641 would probably be the more annoying requirement. If the user was using recording mode on the old branch then they're probably aware that it's enabled, and they probably intend to continue recording once they've reoriented themself in the new branch. |
You're making it sound like asking for a proper explanation and practical reasoning is something that should be avoided. It's not fallacy. It's how this entire tool was initially built in fceux. Workflow is everything. |
The later is not any harder to do than the former. The issue you linked is irrelevant since the bug has been fixed (oops, that should be closed now) and also because undo is simply not required: the user can just load the branch again. I do not think keeping it on will be a significant annoyance even if one wants it off. Unless people have complained about recording mode being kept on when loading branches in the past, or someone has a real workflow where keeping it on is a significant hindrance, I think we should keep it on. Because the recent complaints in Discord show there are real workflows where users want it kept on. Edit: and regarding pausing on branch load: please no. I would be annoyed by that. It's not uncommon for me to load branch A while unpaused, watch a few seconds, then load branch B, watch a few seconds, and maybe go back and forth a couple times to visually compare two strategies. |
That's the conclusion I came to as well. I hadn't thought of loading the branch again, but unpausing is still less effort IMO.
While recording though? |
Asking for more reasoning: good, expected |
Where? |
Automatic pausing upon branch load is more likely to be perceived by end-users as an unexplained inconsistency rather than a protective measure.
I'm of two minds. As someone who uses recording mode rarely and with great caution, I'd somewhat inclined to suggest this PR's changes should be optional, with a TAStudio config option On the other hand, ideally TAStudio should mostly be an enhancement to traditional movie rerecording. Under that view, loading states traditionally preserves recording mode, so loading branches definitely should. And with the new undo/redo changes, unfixable recording mode blunders should be less common. (As a side note, it's hard to properly test this branch while this new regression remains: #4477 ) |
I see TAStudio as a complete replacement for traditional movie recording. |
Sort of off-topic, but I don't think TAStudio can or should replace traditional rerecording. TAStudio cannot currently fulfill the role of being lightweight, reliable, and consistent with traditional rerecording workflows that many existing users have been using for years. Major bugs like #4058 went unnoticed for years, and I still run into TAStudio-specific bugs often and I imagine there are many undiscovered edge cases. (Getting even more off-topic, I've personally thought for years it'd be better to just rewrite TAStudio from scratch with robust testcases and special attention to workflow efficiency. But perhaps it's too ambitious for me as one person, and I haven't written much actual code -- I have been slowly building up a library of personal notes about implementation details, abstract testcases, and backwards compatibility notes though.) |
It's both an upgrade and a replacement, depending on one's preferences. It being an upgrate helps people get used to it and potentially migrate over. But if not, it should at least not be annoying as an enhancement. |
And with 1c0c915, it won't even mess up any undo history.
If this is to be the case, I think it's pretty clear that loading a branch should not turn off recording mode. |
Multiple people (at least 2) in Discord have brought up a recent change, thinking it may be a bug: Loading a branch disables recording mode, but did not in 2.10. This indicates that users find the change surprising and prefer the old behavior. This PR will restore that old behavior.
The change was originally done as part of making TAStudio more consistent. Editing inputs in a way that is not recording a frame should disable recording mode, but it used to be that not all edits would do this. Loading a branch is one way of changing inputs, and so it now (in master) disables recording mode.
Although disabling recording mode on branch load makes sense to me, I can see how it would be a nuisance to people who actually regularly use recording mode as part of TASing.
Check if completed: