iw4x / iw4x-client

GNU General Public License v3.0
123 stars 42 forks source link

[PlayerMovement] Tackle some issues with Dvar flags #154

Closed diamante0018 closed 3 weeks ago

diamante0018 commented 3 weeks ago

Closes #147 Dvars that are server side do not need to be cheat protected because the server will correct any out of sync client

diamante0018 commented 3 weeks ago

@Rackover kindly requesting a review

Rackover commented 3 weeks ago

What is the exact use case for DVAR_CHEAT ? It is for client-only dvars of which any change from the default value must be approved by the server? Or is there any other use-case? Should server-controlled variables ever be set to something different than CODINFO ?

diamante0018 commented 3 weeks ago

It is for client-only dvars of which any change from the default value must be approved by the server? Or is there any other use-case?

The change for cheat protected dvars is never approved by the server, that's the reasoning for the cheat flag. Additionaly, values are reset when joining a new servers because the servers that change these cheat dvars from default value will try to tell the client to update it so they are in sync.

Should server-controlled variables ever be set to something different than CODINFO ?

Example of server side dvars that are not supposed to be codinfo are server info dvar and system info dvar (see getinfo packet response builder function)

All this PR does is revert the change you made some months ago to what were the values that had been used safely for years.

Rackover commented 3 weeks ago

This PR does not need further justification, I simply asked for clarification about Dvar flags, which this PR changes. Imo this change makes sense, I just want to make sure to understand what each flag is for as to make sure we're not uncheating something we shouldn't - and also, considering whether other Dvars should also be uncheated along with those.

diamante0018 commented 3 weeks ago

This PR does not need further justification, I simply asked for clarification about Dvar flags, which this PR changes. Imo this change makes sense, I just want to make sure to understand what each flag is for as to make sure we're not uncheating something we shouldn't - and also, considering whether other Dvars should also be uncheated along with those.

No problem, I did try to put some very basics comments in the structs header files alongside the dvar flag definition but now I saw that there is a wiki so perhaps all dvars can be expanded on there with lengthier explanations because not all behaviour is documented. Some dvar flags seem unused too like what was labelled as DVAR_INTERNAL

Yes part of issue 61 was original a request to uncheat a specific dvar that is server-side (BG) however the list is probably longer and it could be a matter of uncheating flags upon request when a need is justified or it is brought up in a feature request.