[Mods]: Clear fs_game when leaving servers

[diamante0018]

What does this PR do?

Closes #139

How does this PR change IW4x's behaviour?

Unloads the mod, if loaded, when leaving a server Fixes some bugs I spotted(unterminated Cbuf strings) in the mod list code

Anything else we should know?

As explained in the code #70 does not allow me to unload a mod when leaving a private match

Did you check all the boxes?

• [X] Focus on a single fix or feature; remove any unrelated formatting or code changes
• [X] Mention any related issues (put closes #XXXX in comment to auto-close issue when PR is merged)
• [X] Follow our coding conventions
• [X] Minimize the number of commits

[diamante0018]

Hi @Rackover, main feature test showcase on YouTube (Link sent in DMs)

Private Match tested off-camera but works as intended and is explained above.

[Rackover]

Is there a reason why ou don't call ClearMods as you originally intended? 🤔 It looks like it would do the trick for your case. Doesn't it?

[diamante0018]

Yes. See comment in code. Besides clear mods is a lambda function so anonymous functions can't be invoked directly. Well actually, just compare the two codes. And see what is missing in clear mods that's not suitable for me

[Rackover]

How fool I am to ask you to explain to you why you did something a certain way.

Let me ask again. Consider that I have read your code, and your comments, because in fact I have. Consider that I have already compared the two codes, because in fact I have.

• Why is the code from ClearMods repeated instead of just calling ClearMods?
  • Keep in mind that C++ allows you to write code before making a function call, so the extra bit at line 110 is absolutely not reason enough to copypaste existing code from ClearMods here instead of calling ClearMods
  • Also keep in mind that such code (the local server check) could also exist in the ClearMods function if that wasn't the case (it wouldn't impact negatively the functionality of ClearMods as far as I can see)
• ClearMods does additional handling to vid_restart or not depending on the settings of the user. Why do you repeat the code without that functionality instead of just calling ClearMods? It is clearly intended by cl_modVidRestart that switching mods should not trigger a vid_restart when that dvar is set, so why bypass this behaviour?
• "Anonymous functions cannot be invoked directly" - well my friend if that function needs to be called elsewhere, I'm sure the C++ syntax has something we can work with! Surely it can be made a standalone function that the scriptmenu lambda can call? You are aware of that feature, so why dismiss it as if it simply could not be?

I would like you to assume that when reviewing your code, people (or at least I) don't do it without reading the code - and that therefore, when asking for additional clarifications, telling me to just "read again and see" is pretty pointless. Such comments are a waste of time for everybody involved.

[diamante0018]

Clear mods is used by the menu code, it's a UIScript. So it's only displayed when fs_game is set.

https://github.com/iw4x/iw4x-client/issues/139#issuecomment-2419950420 (menu code was posted here)

So the user has to manually trigger this when going to the menu if a mod was loaded for this to display.

Now this PR is about unloading a mod once leaving a server so a few things need to be checked out

• fs_game is set
• we are not leaving a private game
• unload the mod without checking cl_modVidRestart

all reasons as to why code from clearmod cannot be reused.

i invite you to check out the clearmod as it's displayed in the iw4x menus as it should make sense why there checking for private match wouldn't make sense (we are on the main menu). The button is at the right bottom corner. To me the clearmods UI script c++ code is specific to cater to this functionality and therefore i'd rather reuse the code in a new function rather than make the clearmod c++ code more complicated since it has it's own execution flow for this specific case.

While my newly added code also has a very specific execution flow because it is run when a user leaves a server and not when we are at the main menu. because of these two differences, I added new code instead of reusing old code.

[Rackover]

Yes, I know where that button is and what function is tied to. This simply does not answer my questions:

• Why do you need to "unload the mod without checking cl_modVidRestart" ... ? If cl_modVidRestart is set to a certain value, that value should be respected. When you join a server with cl_modVidRestart set to false, you don't get a vid restart (see Download.cpp) so why should you get one when leaving...? can you explain that to me?
• I'm sure you're aware that in C++, you can create new functions when you need to. If the UIScript ClearMods, and the leave server event, have functionality in common, such functionality can be put in a new function (UnloadMod, or ClearMod, or whatever you desire) and then that function can be called from both the UIScript lambda and from the leave server event.

Reduce, reuse, recycle - less duplicates of codes that do the same thing (why would "new code" be preferable to "old code"? does code rot?), less stuff to maintain - only one correct way to unload a mod, one function that can be called from multiple places that that will unload a mod if necessary, vid_restart if necessary, and you can add the local server check specificity to your event if you want before calling that function.

You know all this! Why pretend you don't, and have me lay it all out? ☹️

[Rackover]

Additionally - have you checked that ->modified = true needs to be set after calling Dvar_SetString? Im not really up to date on Dvars so I might be wrong on this. But i feel like this modified flag is set by Dvar_SetString at 0x6478B7. Tell me if this is mistake on my end, i have only very partially labeled this function

[diamante0018]

I see 0x6478B7, I guess it is indeed unnecessary and a bad copy and paste on my end.

Well, if that's what you desire, I will now modify the PR to do what you are suggesting.

[diamante0018]

I have completed testing once again. All is good on my end.

On a side note, it's impossible to find a server that does not have a gazillion menu errors ahah. I'm will be so glad once this PR is merged because who knows how broken their menus are outside of normal gameplay

[Rackover]

unrelatedly - there's a crash currently when you leave a game and then try to open the server browser. It's very old and i'm gonna open an issue for it because it definitely hinders usability (currently the game crash if i leave a game then i click "join game")

[Rackover]

Tested and it seems to run gracefully, through map rotation aswell, merging