add bxt_tas_studio_norefresh_until_stop_frame #88
Conversation
|
I find this very useful when I want to do post-work like pitch change. I think this is better than setting where to start playing is because that needs you to bind two keys. The implementation is very scuffed because playback is in BunnymodXT so changing value of another cvar won't be as elegant. There is also another problem with overflow because unsigned. I believe this has something to do with loading the TAS and things aren't in place. |
|
Would it be possible to make bxt_tas_norefresh_until_last_frames do this by default? Perhaps adding some extra hook between BXT and bxt-rs. Just doesn't seem necessary to have an extra cvar for this when the original one does something undesirable in this case. |
khanghugo
force-pushed
the
studio-norefresh-until-stop-frame
branch
from
a4ddfb8
to
24bebd5
Compare
src/modules/tas_studio/mod.rs
Outdated
| fn set_effective_norefresh_until_stop_frame(marker: MainThreadMarker, editor: &Editor) { | ||
| let value = if BXT_TAS_STUDIO_NOREFRESH_UNTIL_STOP_FRAME.as_bool(marker) { | ||
| // Very sad | ||
| match editor.branch().frames.len().checked_sub(1) { |
There was a problem hiding this comment.
Isn't there always the initial frame there? Don't remember
There was a problem hiding this comment.
I am not sure what you mean. If you mean the frames length, then yea I have to subtract 1. That is what also happens to the status hud last frame pr
There was a problem hiding this comment.
What I mean is, can this checked sub ever fail?
There was a problem hiding this comment.
From what I remember when I did this, this check sub is here just because unsigned. If it fails, it gets to 0. In BunnymodXT PR, 0 for this will just load the entire script.
Now I think about it again, I don't really remember why I really did it beside the integer overflow. Probably was to make it extra safe. I don't think it will ever happen from my recent usage sessions.
There was a problem hiding this comment.
It is possible that I was scared out of index because I was working on other PR with bulk_idx function while also working on this so it carries over, maybe I guess there shouldn't be the check sub.
There was a problem hiding this comment.
FWIW bxt-rs enables integer overflow checks in release. If it ever breaks you will see because it will panic. Just IIRC frames always has at least one entry so checked should be unnecessary.
src/modules/tas_studio/mod.rs
Outdated
| fn set_effective_norefresh_until_stop_frame(marker: MainThreadMarker, editor: &Editor) { | ||
| let stop_frame = norefresh_until_stop_frame_frame_idx(marker, editor); | ||
|
|
||
| if stop_frame == 0 { |
There was a problem hiding this comment.
Should this logic be in norefresh_until_stop_frame_frame_idx() instead?
There was a problem hiding this comment.
It is very possible for those two functions to be just one because the set function does not return anything. But I still have both of them into different thing.
As for the stop frame logic thing, it is a rare case when stop frame is 0, which is usually full tas playback or starting to do the tas. It could just be a simple return but for some reasons it stop frame 0 and until last frames xxx does not work though I reset the override value after playback is done in BunnymodXT (so stop frame 0 will automatically pickup the until last frames value). That is why the extra set call i there.
Maybe I don't understand what you are saying here.
There was a problem hiding this comment.
What I am saying is, maybe the if stop_frame == 0 { subtract from total frames } else { subtract from stop frame } logic should be inside norefresh_until_stop_frame_frame_idx()?
|
Thanks! |
No description provided.