TTT-2 / TTT2

Trouble in Terrorist Town 2 for Garry's Mod (gmod)
https://steamcommunity.com/sharedfiles/filedetails/?id=1357204556
179 stars 77 forks source link

Server cvar message constantly showing up in chat #1632

Closed mexikoedi closed 16 hours ago

mexikoedi commented 2 months ago

Your version of TTT2 (mandatory)

Describe the bug (mandatory)

The following message shows up in the chat each map on the first round: "Server cvar 'ttt2_enable_map_prefix_gm' changed to 0".

To reproduce

Steps to reproduce the behaviour:

  1. Go into a round
  2. Start the round (maybe change the map a couple of times)
  3. See described issue

Expected behaviour

This message shouldn't appear each time in the chat.

Context (please provide as much as you can)

Histalek commented 2 months ago

I assume you have set ttt2_enable_map_prefix_gm to 0 manually once and this issue has occurred since then?

TimGoll commented 2 months ago

I know why this happens. I dynamically create the convar AFTER the file loaded, maybe a tenth of a second later or so. It then detects this as a SetConVar instead of a CreateConVar

I used this because I thought that it is a nice simplification here. But I have overseen the chat massage. Checking if the convar already exists will resolve this issue

Histalek commented 2 months ago

I know why this happens. I dynamically create the convar AFTER the file loaded, maybe a tenth of a second later or so. It then detects this as a SetConVar instead of a CreateConVar

I used this because I thought that it is a nice simplification here. But I have overseen the chat massage. Checking if the convar already exists will resolve this issue

Are you sure this isn't happening because of the cvars.AddChangeCallback call here ?

Besides that i think we should not create the _gm convar dynamically anyway as two gm maps are included with the gmod server installation and we should expect them to be there, right?

TimGoll commented 2 months ago

Are you sure this isn't happening because of the cvars.AddChangeCallback call here ?

Setting a global bool doesn't trigger the convar change message

Besides that i think we should not create the _gm convar dynamically anyway as two gm maps are included with the gmod server installation and we should expect them to be there, right?

I create this convar dynamically for every prefix, gm, css, ttt, de, etc. By default all of them are enabled and you can disable the maps with that prefix in the UI. If I'd remove that convar, to which group the gm_ maps should be moved? And why should the gm maps be handled differently than the ttt, css etc maps?

I'm asking because I'm not sure if you misunderstood the reason of the convar or if we're talking about something completely different here.

Histalek commented 2 months ago

Are you sure this isn't happening because of the cvars.AddChangeCallback call here ?

Setting a global bool doesn't trigger the convar change message

Hmm but what is triggering this then? CreateConVar should never change an already existing convar and only return it, right?

Besides that i think we should not create the _gm convar dynamically anyway as two gm maps are included with the gmod server installation and we should expect them to be there, right?

I create this convar dynamically for every prefix, gm, css, ttt, de, etc. By default all of them are enabled and you can disable the maps with that prefix in the UI. If I'd remove that convar, to which group the gm_ maps should be moved? And why should the gm maps be handled differently than the ttt, css etc maps?

My bad for the wording. We do create the _de _cs and _test convars on file load and disable them. Under the assumption that this only happens for convars that are created dynamically only, we could fix the issue at least for the _gm convar by also creating it on file load. Given that this is the most likely case someone will run into this issue, i feel like this would be a reasonable approach

TimGoll commented 2 months ago

Hmm but what is triggering this then? CreateConVar should never change an already existing convar and only return it, right?

I have to test this, but my guess is that using CreateConVar at runtime instead of at file load causes this to happen.

My bad for the wording. We do create the _de _cs and _test convars on file load and disable them. Under the assumption that this only happens for convars that are created dynamically only, we could fix the issue at least for the _gm convar by also creating it on file load. Given that this is the most likely case someone will run into this issue, i feel like this would be a reasonable approach

Ah, I get it now. I disable them by default because I assumed no TTT player actually wants to play a GM map. Changing the default might not fix it, if the player then manually sets it to disabled. I have to test this

mexikoedi commented 2 months ago

I assume you have set ttt2_enable_map_prefix_gm to 0 manually once and this issue has occurred since then?

I think I have never set this manually.

TimGoll commented 2 months ago

I assume you have set ttt2_enable_map_prefix_gm to 0 manually once and this issue has occurred since then?

I think I have never set this manually.

Yes, my bad, Gmod is set to false by default as Histalek noted

TimGoll commented 1 month ago

okay, aparantly that isn't such an easy fix. Because I do not set this convar when the files are loaded, it is announced to chat. There is no way around it as far as I can see.

But this poses the general question: Should convar changes be announced in chat at all?

mexikoedi commented 1 month ago

okay, aparantly that isn't such an easy fix. Because I do not set this convar when the files are loaded, it is announced to chat. There is no way around it as far as I can see.

But this poses the general question: Should convar changes be announced in chat at all?

I think not. Feels weird if everyone sees it in the chat.

Histalek commented 16 hours ago

This should be fixed by #1669