Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove FCVAR_SERVER_CAN_EXECUTE from disconnect #663

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ASpoonPlaysGames
Copy link
Contributor

@ASpoonPlaysGames ASpoonPlaysGames commented Feb 11, 2024

Servers should use the NSDisconnectPlayer script function instead. (clients can just ignore the disconnect command, it's more a suggestion)

Closes #553

@GeckoEidechse
Copy link
Member

To be clear, do clients already ignore disconnect?

How is it handled in regards to vanilla? Like what happens if you run this PR in vanilla?

@ASpoonPlaysGames
Copy link
Contributor Author

ASpoonPlaysGames commented Feb 11, 2024

To be clear, do clients already ignore disconnect?

Normal clients do not ignore disconnect, but a malicious one very much could. Therefore using disconnect to disconnect a client is a bad idea and we shouldn't allow for it. The purpose of disconnect is for a client to disconnect themselves from a server.

How is it handled in regards to vanilla? Like what happens if you run this PR in vanilla?

Nothing, this is reverting disconnect back to vanilla behaviour. Vanilla doesn't expect disconnect to be called by servers because they could just choose to ignore it, vanilla uses other native functions to properly disconnect a client.

@GeckoEidechse
Copy link
Member

To be clear, do clients already ignore disconnect?

Normal clients do not ignore disconnect, but a malicious one very much could. Therefore using disconnect to disconnect a client is a bad idea and we shouldn't allow for it. The purpose of disconnect is for a client to disconnect themselves from a server.

Aight, thanks for the clarification <3

The only thing I need to keep in mind then in that regard is to give modders a heads-up in case they somehow made a server-mod that sends disconnect to client.

Man I wish I already had a system in place to crawl Thunderstore mods for this sorta stuff ^^"

@ASpoonPlaysGames
Copy link
Contributor Author

The only thing I need to keep in mind then in that regard is to give modders a heads-up in case they somehow made a server-mod that sends disconnect to client.

I think when we added NSDisconnectPlayer we did a similar notification already tbh

@GeckoEidechse
Copy link
Member

So I got around writing a quick Thunderstore scraper and based on my local dataset, the following mods at their latest version will be affected

  • Zanieon-Matchmaking_Mixtape_Menu-1.2.0
  • SoftSleeper-Attrition_Extended_Overhaul-1.2.11
  • VoyageDB_Modding_Home-Attrition_Extended-1.9.3
  • NanohmProtogen-VanillaPlus-2.4.1
  • VoyageDB_Modding_Home-Grunt_Mode-3.3.2
  • VoyageDB_Modding_Home-Nessie_Temp_Fix-1.3.16
  • The_Peepeepoopoo_man-Titanframework-2.4.2
  • Okudai-loeb-1.0.1
  • Capt_Diqhedd-GoMortyYourself-1.0.1
  • VoyageDB_Modding_Home-Modded_Bleedout_Game-1.0.3
  • VoyageDB_Modding_Home-The_Spawner_English-1.2.0
  • VoyageDB_Modding_Home-The_Spawner_English-1.2.0
  • taskinoz-EnhancedMenuMod-1.14.0
  • zxcPandora-DeveloperMode-1.0.1
  • EladNLG-RoguelikeBETA-0.3.102
  • EladNLG-Roguelike-0.3.102

checked by running:

$ find thunderstore -type f -exec grep -l '"disconnect[ "]' {} +
thunderstore/Zanieon-Matchmaking_Mixtape_Menu-1.2.0/mods/MixtapeMenu/mod/scripts/vscripts/ui/menu_lobby.nut
thunderstore/SoftSleeper-Attrition_Extended_Overhaul-1.2.11/mods/ATTExtendOverhaul/mod/scripts/vscripts/burnmeter/sh_burnmeter.gnut
thunderstore/VoyageDB_Modding_Home-Attrition_Extended-1.9.3/mods/AITdmExtended/mod/scripts/vscripts/burnmeter/sh_burnmeter.gnut
thunderstore/NanohmProtogen-VanillaPlus-2.4.1/mods/NP.VanillaPlus/mod/scripts/vscripts/ui/menu_lobby.nut
thunderstore/VoyageDB_Modding_Home-Grunt_Mode-3.3.2/mods/GruntModeNS/mod/scripts/vscripts/burnmeter/sh_burnmeter.gnut
thunderstore/VoyageDB_Modding_Home-Nessie_Temp_Fix-1.3.16/mods/NessieTempFix/mod/scripts/vscripts/burnmeter/sh_burnmeter.gnut
thunderstore/The_Peepeepoopoo_man-Titanframework-2.4.2/mods/peepee.Titanframework/mod/scripts/vscripts/sh_loadouts.nut
thunderstore/Okudai-loeb-1.0.1/mods/okudai.loeb/mod/scripts/vscripts/ui/menu_lobby.nut
thunderstore/Capt_Diqhedd-GoMortyYourself-1.0.1/mods/Diq.GoMortyYourself/mod/scripts/vscripts/cl_mortyyourself.gnut
thunderstore/VoyageDB_Modding_Home-Modded_Bleedout_Game-1.0.3/mods/Modded.Bleedout/mod/scripts/vscripts/burnmeter/sh_burnmeter.gnut
thunderstore/VoyageDB_Modding_Home-The_Spawner_English-1.2.0/mods/Super.Mixed.Game.English/mod/scripts/vscripts/burnmeter/sh_burnmeter.gnut
thunderstore/VoyageDB_Modding_Home-The_Spawner_English-1.2.0/mods/Super.Mixed.Game.English/mod/scripts/vscripts/sh_loadouts.nut
thunderstore/taskinoz-EnhancedMenuMod-1.14.0/mods/Enhanced.Menu.Mod.Northstar/mod/scripts/vscripts/sp/sp_training.nut
thunderstore/zxcPandora-DeveloperMode-1.0.1/mods/DeveloperMode/mod/scripts/vscripts/sh_loadouts.nut
thunderstore/EladNLG-RoguelikeBETA-0.3.102/mods/EladNLG.Roguelike/mod/scripts/vscripts/sp/_savegame.gnut
thunderstore/EladNLG-Roguelike-0.3.102/mods/EladNLG.Roguelike/mod/scripts/vscripts/sp/_savegame.gnut

@GeckoEidechse
Copy link
Member

What would you say is the best way to proceed? Ping mod authors and release PR as part of minor release?

@ASpoonPlaysGames
Copy link
Contributor Author

So I got around writing a quick Thunderstore scraper and based on my local dataset, the following mods at their latest version will be affected

  • Zanieon-Matchmaking_Mixtape_Menu-1.2.0
  • SoftSleeper-Attrition_Extended_Overhaul-1.2.11
  • VoyageDB_Modding_Home-Attrition_Extended-1.9.3
  • NanohmProtogen-VanillaPlus-2.4.1
  • VoyageDB_Modding_Home-Grunt_Mode-3.3.2
  • VoyageDB_Modding_Home-Nessie_Temp_Fix-1.3.16
  • The_Peepeepoopoo_man-Titanframework-2.4.2
  • Okudai-loeb-1.0.1
  • Capt_Diqhedd-GoMortyYourself-1.0.1
  • VoyageDB_Modding_Home-Modded_Bleedout_Game-1.0.3
  • VoyageDB_Modding_Home-The_Spawner_English-1.2.0
  • VoyageDB_Modding_Home-The_Spawner_English-1.2.0
  • taskinoz-EnhancedMenuMod-1.14.0
  • zxcPandora-DeveloperMode-1.0.1
  • EladNLG-RoguelikeBETA-0.3.102
  • EladNLG-Roguelike-0.3.102

Note that only mods that do this on server are affected by this, which reduces this count quite a lot.

@GeckoEidechse GeckoEidechse self-assigned this Feb 18, 2024
@GeckoEidechse
Copy link
Member

Note that only mods that do this on server are affected by this, which reduces this count quite a lot.

Good point. I manually looked through the mods a bit and this is what I ended up with:

Client
thunderstore/Zanieon-Matchmaking_Mixtape_Menu-1.2.0/mods/MixtapeMenu/mod/scripts/vscripts/ui/menu_lobby.nut
thunderstore/NanohmProtogen-VanillaPlus-2.4.1/mods/NP.VanillaPlus/mod/scripts/vscripts/ui/menu_lobby.nut
thunderstore/Okudai-loeb-1.0.1/mods/okudai.loeb/mod/scripts/vscripts/ui/menu_lobby.nut
thunderstore/Capt_Diqhedd-GoMortyYourself-1.0.1/mods/Diq.GoMortyYourself/mod/scripts/vscripts/cl_mortyyourself.gnut


Server
thunderstore/SoftSleeper-Attrition_Extended_Overhaul-1.2.11/mods/ATTExtendOverhaul/mod/scripts/vscripts/burnmeter/sh_burnmeter.gnut
thunderstore/VoyageDB_Modding_Home-Attrition_Extended-1.9.3/mods/AITdmExtended/mod/scripts/vscripts/burnmeter/sh_burnmeter.gnut
thunderstore/VoyageDB_Modding_Home-Grunt_Mode-3.3.2/mods/GruntModeNS/mod/scripts/vscripts/burnmeter/sh_burnmeter.gnut
thunderstore/VoyageDB_Modding_Home-Nessie_Temp_Fix-1.3.16/mods/NessieTempFix/mod/scripts/vscripts/burnmeter/sh_burnmeter.gnut
thunderstore/The_Peepeepoopoo_man-Titanframework-2.4.2/mods/peepee.Titanframework/mod/scripts/vscripts/sh_loadouts.nut	
thunderstore/VoyageDB_Modding_Home-Modded_Bleedout_Game-1.0.3/mods/Modded.Bleedout/mod/scripts/vscripts/burnmeter/sh_burnmeter.gnut
thunderstore/VoyageDB_Modding_Home-The_Spawner_English-1.2.0/mods/Super.Mixed.Game.English/mod/scripts/vscripts/burnmeter/sh_burnmeter.gnut
thunderstore/VoyageDB_Modding_Home-The_Spawner_English-1.2.0/mods/Super.Mixed.Game.English/mod/scripts/vscripts/sh_loadouts.nut
thunderstore/zxcPandora-DeveloperMode-1.0.1/mods/DeveloperMode/mod/scripts/vscripts/sh_loadouts.nut


No clue
thunderstore/taskinoz-EnhancedMenuMod-1.14.0/mods/Enhanced.Menu.Mod.Northstar/mod/scripts/vscripts/sp/sp_training.nut
thunderstore/EladNLG-RoguelikeBETA-0.3.102/mods/EladNLG.Roguelike/mod/scripts/vscripts/sp/_savegame.gnut
thunderstore/EladNLG-Roguelike-0.3.102/mods/EladNLG.Roguelike/mod/scripts/vscripts/sp/_savegame.gnut

Looking at it further the majority of the server sided mods that call disconnect do so cause they modify existing Northstar scripts

From the look of it, they are called on server but I might be mistaken.

If they are, I guess we'd have to update the corresponding mod code as well ^^"

@GeckoEidechse GeckoEidechse removed their assignment Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

disconnect command shouldn't be FCVAR_SERVER_CAN_EXECUTE
3 participants