splewis / csgo-multi-1v1

CS:GO Sourcemod plugin to create multi-1v1 arena servers
GNU General Public License v3.0
278 stars 68 forks source link

Remove radar timer & logic, add new Valve built-in cvar #225

Closed sneak-it closed 4 years ago

sneak-it commented 5 years ago

This PR replaces the current logic which removes the radar for players. Valve introduced a cvar a few updates back (sv_disable_radar) which would replace this function, resulting in a tiny optimization, but also allows server owners to choose if they want to use radar or not, instead of being hardcoded.

I've included the new cvar in the game_cvars.cfg config file to replicate original behavior.

splewis commented 5 years ago

Do you actually have a reasonable use case for this plugin where you'd want radar?

I can see how the flexibility is nice (and doing it this way is obviously preferable), but merging this in would cause any user that only updates the plugin binary (and not the game_cvars.cfg file) to have radars re-added. That would happen to any user of the automatic updates.

sneak-it commented 5 years ago

Some arena maps have proper radar overviews, however I personally would not enable it, where some might. This PR would also solve the issue of radar popping up when dead/in spectator.

As you said, the downside would definitely be a change that server operators would need to pay attention for. Not quite breaking, but would require an extra eye, which I think server owners should have the responsibility for in the first place. Radar is used in normal gameplay, so it's not exactly a horrendous thing if a server operator cannot add the cvar right away. This change can be noted in a release or on AM. The benefits outweigh this small inconvenience in my opinion.

melkor217 commented 4 years ago

I've just tested this change on my server. It works smoothly and also fixes black square shown on radar's place for spectators.

splewis commented 4 years ago

also fixes black square shown on radar's place for spectators.

I had no doubts this would "work", just that it didn't have enough tangible benefit for users to require them to update their configs. But... I suppose on another thought, fixing the spectator black square is a strong enough improvement. I'll go ahead and merge this in.