Fix Mosaic and Wave, and other transition issues
See #3446
Fix the following:
- First frame not rendered because Update is called before Draw.
- Duration is wrong (too short).
- Stretching (bottom right extension) in Mosaic, due to the random offset range being incorrect.
- Background trailing in Wave due to blitting "nothing".
- Depth (horizontal movement) is 30 fps in Wave because Percentage is integer (truncation). Phase (vertical movement) is wrong in half of the frames for the same reason.
- Blind visibly starts 5 frames late, and pacing is 4+6 instead of 5.
And the other Percentage issues:
- Stripes are 30 fps like Depth.
- The scale is wrong in Zoom.
- Most of the other transitions (like Scroll and Combine/Division) are "choppy" (but not 30 fps like Stripes).
All transitions are frame-based now. Percentage was removed
Good job. While working on this I didn't notice that there is a frame counter (I only saw that percentage counter which was annoying to work with due to integer truncation). With the frame counter the problems I encountered at least look solvable. xD
In case you didn't notice yet: At the top of the file is Transition::GetDefaultFrames where you can configure the length of a transition in frames. So in case any transition has the wrong duration you can update it here.
(And feel free to fix as much transition stuff as you want as long as it only happens in the transition.cpp/.h and doesn't alter too much other stuff ^^)
Thank you! So, from my understanding, setting current_frame to -1 in the Init in transition.cpp (and applying offsets where needed) is the simplest solution for the off-by-one error, right?
Do all transitions have this off-by-1 problem? Starting from -1 is imo confusing.
Better increment the counter in the function I mentioned for everything by 1 which has the same effect. https://github.com/Lt-knb/EasyRPG-Player/blob/5d8891f13fdab4cae28657990b70519b0b71a8de/src/transition.cpp#L40
(or if not all transitions are affected add new cases to this function with the correct numbers).
Frame 0 is always skipped due to current_frame++ in Update (called before Draw).
In case you missed it, I explained the frame count behavior in this comment
ooh, yeah I missed that comment, only briefly checking the github stuff right now. xD
Yes in this case setting it to -1 and adding a comment above the assignment that mentions this off-by-1 error because of Update before Draw sounds reasonable.
(ignore the build failures, the builders will be okay with the next update)
@Lt-knb is their any work left to do here or is this ready?
@Ghabry There's one remaining issue that affects all transitions. There's 3 extra frames in Hide+Show: A black one after Hide, a "normal" one after Show, and one more black frame between Hide and Show. Those are not there in RPG_RT. So all transitions have a black flicker in the middle right now, even Wave and Mosaic. What's causing those frames to appear, and how do we get rid of them?
Imo, even though it is not 100% perfect this could be already reviewed and merged.
It fixes alot of stuff already and the "unsolved" problems are not regressions (they are also present in the upstream easyrpg-player build)
Main motivation for "lets merge it" is that I have no idea how to get rid of the black frame. xD
The black flicker is another race condition it seems.
In graphics.cpp the transition is considered for one frame as IsErasedNotActive so the screen is cleared.
if (transition.IsActive()) {
min_z = transition.GetZ();
} else if (transition.IsErasedNotActive()) {
min_z = transition.GetZ() + 1;
dst.Clear();
}
Not sure how to correctly fix this currently.
Alright, the black flicker deserves a separate PR then. Marked as ready