Skip to content
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

bug fix: hoverbike jittering #176

Merged
merged 5 commits into from
Sep 22, 2023
Merged

bug fix: hoverbike jittering #176

merged 5 commits into from
Sep 22, 2023

Conversation

rafccq
Copy link

@rafccq rafccq commented Sep 18, 2023

Fixes #78
However, see remarks below.
I tested a bit, the jittering is fixed and I didn't observe any side effect, but definitely more testing is needed since this touches the frame time calculation code, which can be very sensitive and affect tons of things.

#ifdef PLATFORM_N64
if (g_Vars.lvframe60 > hov->prevframe60)
#endif
{
Copy link
Author

@rafccq rafccq Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the jittering when using the combat boost.
For some reason which I don't understand yet, when the combat boost is enabled g_Vars.lvframe60 == hov->prevframe60 every other frame, causing the transform to be updated only every other frame therefore the jittering. I'll keep investigating this.

diffframe60 = (g_Vars.lostframetime60t + diffframet + CYCLES_PER_FRAME / 2) / CYCLES_PER_FRAME;
diffframe240 = (g_Vars.lostframetime240t + diffframet + CYCLES_PER_FRAME / 2 / 4) / (CYCLES_PER_FRAME / 4);
diffframe60 = (diffframet + CYCLES_PER_FRAME / 2) / CYCLES_PER_FRAME;
diffframe240 = (diffframet + CYCLES_PER_FRAME / 2 / 4) / (CYCLES_PER_FRAME / 4);
} while (diffframe60 < g_Vars.mininc60);

g_Vars.lostframetime60t = g_Vars.lostframetime60t + diffframet - diffframe60 * CYCLES_PER_FRAME;
Copy link
Author

@rafccq rafccq Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the random jittering without the combat boost.
I don't fully understand what is the use for this g_Vars.lostframetime60t here, but i noticed that the way its calculated is actually redundant (I wonder if this is one of the decompilation quirks).
For clarity, lets nameg_Vars.lostframetime60t as L, diffframet as D and diffframe60 as D60. In line 46, if we expand diffframe60 to the value from line 42, we get:
L = L + D - ((L + D + CYCLES_PER_FRAME/2) / CYCLES_PER_FRAME) * CYCLES_PER_FRAME
This whole thing simplifies to -CYCLES_PER_FRAME/2. So in the next frame when we plug that into line 42, it will cancel out with the term CYCLES_PER_FRAME/2.

@fgsfdsfgs
Copy link
Owner

Like you already mentioned, since this is a change in the timing code, it needs some thorough testing.
While your reasoning for the simplification makes sense in normal math, we'd also need to consider things like overflows and that stuff like (x / y) * y has a somewhat different effect when all variables involved are integers.

@rafccq
Copy link
Author

rafccq commented Sep 18, 2023

Actually I tested more and noticed that the changes to frametimeCalculate() are not needed, I just reverted it. Removing the conditional in hovTick() fixes it with and without the combat boost.

@fgsfdsfgs
Copy link
Owner

fgsfdsfgs commented Sep 18, 2023

Wonder if maybe a >= instead of > would make more sense there. There is another condition exactly like this one in hovUpdateGround. This is more of a cosmetic thing though, it won't matter either way.
In general it sure sounds like lvframe60 sometimes doesn't increase by 1 per frame when it should. Wonder if it's because the game runs too fast and the frametime calculation comes up slightly short or something.

@rafccq
Copy link
Author

rafccq commented Sep 18, 2023

Yes I still don't understand why lvframe60 sometimes (or every other frame, when combat boost is active) doesn't increment.
About the conditional, in my tests it was always either greater than or equal, so I considered the whole conditional redundant.
Also, that's the only place that sets hov->prevframe60 = g_Vars.lvframe60

@RyanDwyer
Copy link

The conditions are likely needed because the tick function is called for each player. If one player has already ticked the hover object on this frame then it skips this part of the function for the second player. You should test your changes in co-op mode with a human buddy.

lostframetime is a remainder value of the number of CPU ticks. The game keeps track of these and applies it on the next frame to make sure time is accounted for correctly. If you remove it you might find in-game time drifting from reality. Or because PC is pretty much always 60 FPS, it might not be noticeable.

lvframe60 being zero is related to running a frame within 1/60th of a second of the previous frame. The game can run a frame with lvframe240 = 3, which means lvframe60 can be 0. On the next frame even if lvframe240 is as low as 1, lvframe60 would be 1 because it's accounted for the remainder. So zero values are expected at high frame rates.

If removing those conditions causes issues in co-op, an alternative fix would be to set a flag on the hov object to note that it's been ticked this frame, and clear it at the start of each frame. Eg. add a new HOVFLAG constant.

@rafccq
Copy link
Author

rafccq commented Sep 19, 2023

Makes sense now, thanks for the explanation Ryan!
Ok I'll test in co-op mode and make the changes if necessary.

@rafccq
Copy link
Author

rafccq commented Sep 19, 2023

I tested co-op (and counter-op as well), no issues. With 2 limitations though: currently no way to test using combat boost, and 2 hoverbikes (maybe when we have custom maps, like solo levels in MP)

@Phnod
Copy link

Phnod commented Sep 19, 2023

You could maybe edit one of the setup level scripts so that it spawns and 2nd bike? In the same level you could change the startup so it gives you combat boosts.

intro_weapon(WEAPON_COMBATBOOST, -1)
ammo(AMMOTYPE_BOOST, 2)```

@rafccq
Copy link
Author

rafccq commented Sep 19, 2023

Ah yes of course thanks for the tip, I forgot the setup data isn't being read from the ROM, but is in code.

@fgsfdsfgs
Copy link
Owner

fgsfdsfgs commented Sep 19, 2023

It is actually being read from the ROM. However you could change it in the decomp and then use the resulting setup file data as a replacement, though you'll need to strip off the ELF header and other stuff. If you can somehow manage to build a new ROM without it being too different from the OG you can also just feed it to pd.exe instead of the normal one and it'll probably work.
Maybe it is time to make it easier to convert these things, though that is unrelated.

@rafccq
Copy link
Author

rafccq commented Sep 20, 2023

Alright, I'll try it later. It would be neat if the game used the setup data from the c files, but I dont know how complex it is to change that.

@fgsfdsfgs
Copy link
Owner

fgsfdsfgs commented Sep 20, 2023

Neat on one hand, on the other hand it would make them a bit harder to replace and make the asset file loading non-uniform.

If you choose to edit them in the decomp, you can then build specifically the setup file like this (this one is for Defection as an example):
make build/ntsc-final/assets/files/UsetupameZ
then you copy build/ntsc-final/assets/files/UsetupameZ to data/files/UsetupameZ in the port and it should work.
Note that this requires that you have one of the compilers that can build the decomp, like mips-elf-gcc.

You could also probably edit it in GE Setup Editor and then re-zip it using tools/rarezip. Would probably be something like this (again using Defection as an example):

  1. In Setup Editor, go to File -> Open Setup Uncompressed, then select PD/pdsetup/UsetupameZ.set and the pads file it asks for.
  2. Go to Edit -> Edit Intro Block, make your changes.
  3. Go to File -> Save Setup Uncompressed, save the .set file somewhere, it doesn't matter if you save the pads file or not.
  4. Run tools/rarezip path/to/set/file/you/saved.set > UsetupameZ.
  5. Copy the resulting UsetupameZ into data/files/UsetupameZ.

Using this approach I was able to replace the starting weapon in Defection just fine, though I don't know if it broke anything else:
image

@rafccq
Copy link
Author

rafccq commented Sep 20, 2023

Thank you so much for the guide.
Ok I managed to test using combat boost (and slo-mo cheat as well) and 2 hoverbikes, no issue:

2023-09-20.19-56-36.mp4

@fgsfdsfgs
Copy link
Owner

I suppose I'll merge this then.

@fgsfdsfgs fgsfdsfgs merged commit 205032c into fgsfdsfgs:port Sep 22, 2023
2 checks passed
@rafccq
Copy link
Author

rafccq commented Sep 22, 2023

If we ever run into issues related to hoverbikes and multiplayer in the future, will remember to revise this (I think is unlikely though).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jittery movement on the hoverbike
4 participants