-
Notifications
You must be signed in to change notification settings - Fork 218
Fix Mosaic and Wave, and other transition issues #3482
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
to fix truncation. Part of EasyRPG#3446
to fix starting frame and percentage truncation
to fix truncation. Part of EasyRPG#3446
to fix truncation. Part of EasyRPG#3446
to fix truncation. Part of EasyRPG#3446
to fix percentage truncation. Part of EasyRPG#3446
to fix truncation and remove percentage. Part of EasyRPG#3446
|
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 (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! |
|
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 |
It's one frame in RPG_RT. Part of EasyRPG#3446
|
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. |
First step to fix the off-by-one error. Fix EasyRPG#3446
|
(ignore the build failures, the builders will be okay with the next update) |
First step to fix the frame count. Part of EasyRPG#3446
Fix the off-by-one error and frame count. Fix EasyRPG#3446
|
@Lt-knb is their any work left to do here or is this ready? |
|
@Ghabry |
|
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 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. |
See #3446
Fix the following:
Phase (vertical movement) is wrong in half of the frames for the same reason.
All transitions are frame-based now.
Percentage was removed