Player icon indicating copy to clipboard operation
Player copied to clipboard

Fix Mosaic and Wave, and other transition issues

Open Lt-knb opened this issue 2 months ago • 6 comments

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

Lt-knb avatar Oct 17 '25 21:10 Lt-knb

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 ^^)

Ghabry avatar Oct 29 '25 10:10 Ghabry

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?

Lt-knb avatar Oct 29 '25 12:10 Lt-knb

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).

Ghabry avatar Oct 29 '25 15:10 Ghabry

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

Lt-knb avatar Oct 29 '25 16:10 Lt-knb

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.

Ghabry avatar Oct 29 '25 20:10 Ghabry

(ignore the build failures, the builders will be okay with the next update)

carstene1ns avatar Oct 29 '25 22:10 carstene1ns

@Lt-knb is their any work left to do here or is this ready?

Ghabry avatar Dec 14 '25 14:12 Ghabry

@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?

Lt-knb avatar Dec 14 '25 15:12 Lt-knb

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.

Ghabry avatar Dec 24 '25 13:12 Ghabry

Alright, the black flicker deserves a separate PR then. Marked as ready

Lt-knb avatar Dec 24 '25 13:12 Lt-knb