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

Replace Game.GameTime with GET_GAME_TIMER in RPH #152

Closed
justalemon opened this issue Dec 19, 2023 · 3 comments
Closed

Replace Game.GameTime with GET_GAME_TIMER in RPH #152

justalemon opened this issue Dec 19, 2023 · 3 comments
Labels
priority: p1 high A bug that might cause crashes or a highly needed feature status: completed The feature has been completed of the bug has been patched type: bug This is a bug
Milestone

Comments

@justalemon
Copy link
Member

Why is that a UINT if the game reports it as an INT?

@justalemon justalemon added priority: p1 high A bug that might cause crashes or a highly needed feature type: bug This is a bug status: acknowledged We are aware of the bug or feature request labels Dec 19, 2023
@justalemon justalemon added this to the 2.0 milestone Dec 19, 2023
@kagikn
Copy link
Contributor

kagikn commented Dec 19, 2023

Actually, RPH returns the game time in the same way as how rage::fwTimer::GetTimeInMilliseconds instended, which returns an unsigned int. While it seems that all the other frameworks avoid using unsigned values for the native. well, for the SHVDN case, it looks like crosire didn't bother to opt for unsigned values for any game APIs.

A decompiled code of GetTimeInMilliseconds of a build of roadpathsgenerator_win64_beta with pdb info (probably built in 2022):

__int64 __fastcall rage::fwTimer::GetTimeInMilliseconds()
{
  return rage::fwTimer::sm_gameTime.m_Time; // `sm_gameTime` is a `rage::fwTimeSet`
}

The equivalent code should be included in GTA5.exe as well. You can't tell if the return value is signed just by looking at how the function is defined because the function only contains mov and ret in the x64 assembly code. However, since some of the callers of GetTimeInMilliseconds compares and do branch jumps with jnb or jbe, which are unsigned, it would be safe to assume that m_Time of rage::fwTimeSet is supposed to be unsigned.

Another reason why I think m_Time is an unsigned int is, that signed integer overflows falls into undefined behaviors in C++ (so the compiler can do anything) while unsigned integer overflows are well defined, but there's no really a good reason to choose a signed integer over unsigned one for a monotonically increasing timer. For example, In C#, you can calculate the time difference with right - left with no problems even if they're signed and an overflow happens, but the C++ compiler can ignore the case where an signed overflow happens.

@justalemon
Copy link
Member Author

Would you recommend switching everything to uint or go back to int? I guess uint would be the right choice.

@justalemon
Copy link
Member Author

I ended up switching to long values.

justalemon added a commit that referenced this issue Dec 19, 2023
@justalemon justalemon added status: completed The feature has been completed of the bug has been patched and removed status: acknowledged We are aware of the bug or feature request labels Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p1 high A bug that might cause crashes or a highly needed feature status: completed The feature has been completed of the bug has been patched type: bug This is a bug
Projects
None yet
Development

No branches or pull requests

2 participants