feat(gta-streaming-five): implement ConVar for octant warnings

[d22tny]

Goal of this PR

To help server owners and developers diagnose octant warnings without overwhelming regular players, a new ConVar named enableOctantCheckWarning has been introduced. When enabled, this ConVar will log warnings when invalid octants are detected. It is disabled by default, meaning no warnings will be shown unless explicitly opted into.

This change ensures stability on newer builds while maintaining a clean experience for end users unless debugging is required.
...

How is this PR achieving the goal

A new ConVar named enableOctantCheckWarning has been added. When set to true, it enables logging of warnings when invalid octants are detected. This is primarily intended for developers and server owners who wish to diagnose those issues. The ConVar defaults to false, so regular players will not see any warnings unless it is manually enabled.
...

This PR applies to the following area(s)

FiveM Client

...

Successfully tested on

Game builds: ..
3258, 3407
Platforms: Windows, Linux

Checklist

• Code compiles and has been tested successfully.
• Code explains itself well and/or is documented.
• My commit message explains what the changes do and what they are for.
• No extra compilation warnings are added by these changes.

Fixes issues

#3401

[mori151]

Hello, sorry for jumping in.

My server is running game build 3095, and ever since the patch was applied, there has been a noticeable reduction in crash reports. This suggests that the benefits may extend beyond build 3407 to other game builds as well.

Based on that, I would propose the following:

• A version check based on the game build does not appear to be necessary; a conditional check solely on enableOctantCheckWarning should suffice.
• The default value for enableOctantCheckWarning can remain true, as before.

[ook3D]

Hello, sorry for jumping in.

My server is running game build 3095, and ever since the patch was applied, there has been a noticeable reduction in crash reports. This suggests that the benefits may extend beyond build 3407 to other game builds as well.

Based on that, I would propose the following:

• A version check based on the game build does not appear to be necessary; a conditional check solely on enableOctantCheckWarning should suffice.
• The default value for enableOctantCheckWarning can remain true, as before.

Yea, I'm not sure where this assumption it doesn't exist prior to 3407, but if a vehicle does not have octants and collides with something it will crash

[d22tny]

So from what i understood, on 3407 having a vehicle model with this problem would crash instantly, but on other game builds it wouldn't, that's where i assumed it's not needed on other game builds, since it impacts performance. So what you say is that on other game builds it maybe can crash sometimes ? If that's the case i'll remove the game build check and only leave the convar.

imo the end user shouldn't see spamming assets going on like this, that's why we need a convar, and i think it should default to false, if a server owner/dev wants to optimize his runtime performance, loading times etc, he can enable the warnings and probably delete those cars, or find good ones, because most assets sellers don't really care about performance.

[ook3D]

So from what i understood, on 3407 having a vehicle model with this problem would crash instantly, but on other game builds it wouldn't, that's where i assumed it's not needed on other game builds, since it impacts performance. So what you say is that on other game builds it maybe can crash sometimes ? If that's the case i'll remove the game build check and only leave the convar.

imo the end user shouldn't see spamming assets going on like this, that's why we need a convar, and i think it should default to false, if a server owner/dev wants to optimize his runtime performance, loading times etc, he can enable the warnings and probably delete those cars, or find good ones, because most assets sellers don't really care about performance.

Yea I agree, should be an option to disable these, no reason for the average user to be shown these, same for poly edge issues

[d22tny]

@prikolium-cfx please tell me if this change is all right and you agree with it, and if i should make a more generic ConVar ( showAssetValidationWarnings ) and include both octants and physics validations prints in it.

[FlawwsX]

I feel like these warning messages should only show when on Canary or Developer mode. Plus, this convar should be true by default, in my opinion, or it can be added to the F8 menu as a toggle. It’s getting awfully difficult to remember all these commands and convars 😭😭

[d22tny]

I feel like these warning messages should only show when on Canary or Developer mode. Plus, this convar should be true by default, in my opinion, or it can be added to the F8 menu as a toggle. It’s getting awfully difficult to remember all these commands and convars 😭😭

Should be true means - You shouldn't see the warnings on default behaviour or you should see them ?

[DaniGP17]

Should be true means - You shouldn't see the warnings on default behaviour or you should see them ?

I think that by default the value of the convar should be true (that is, warnings should be shown by default) because if someone has a crash related to this, I'm pretty sure they won't remember to enable this convar.

[Gogsi]

I think this can be made like some other logs - on the Release channel, it's only shown in the log file (for crash diagnostics) and not in the console. On Beta/Canary, it's also shown in the console for easy debugging. I'm not sure how this is done though 😄

[d22tny]

@Gogsi 's view seems the best to me so far. Can you tell me which log has that behaviour so i can dig into ?

[Gogsi]

Can you tell me which log has that behaviour so i can dig into ?

I think it's done with the trace() macro. You can see all of these init function ones that get shown in the console only on Beta/Canary. I think if you follow the logic it eventually ends up inCfxBigConsole::FilterLog, where if you're not on production, it only shows logs from certain components and scripts. Unfortunately I don't think you can get the message to show as a warning or error that way, only as a trace message.

EDIT: Actually looking at it again, I think the current console::PrintError call about the asset validation also passes through FilterLog. Is it possible this is already the current behavior - the error is only shown on Beta/Canary?

[d22tny]

Yea, i've checked. The warning does only appear in the log on Release.

But still, i get no crash on 3258 game build.

So i'm going back to my initial question. Why are we patching this on < 3407 if it only slows performance and it's not useful as a patch ?

@mori151 , what crash were you getting and stopped because specifically of this patch ?

[d22tny]

I've collided 2 vehicles with the error, nothing happened.

[tens0rfl0w]

The patch needs to be in place for all gamebuilds, while b3407 is more likely to crash with invalid octant vertex data due to some internal changes, this was already an issue on prior game builds and was one of the most common crashes. The fix itself is currently getting reworked, which should also result in less of these error prints, but I don't see the need for a ConVar as we're already filtering this out on release builds.

[d22tny]

Yup, convar definetly not needed. Okay then!