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

Skip demo recording on veto phase #189

Draft
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

FlowingSPDG
Copy link
Contributor

context: #180
plus: StartDemoRecording won't start recording if its already recording.

I'll check how it behaves on tournament we will have today.

@FlowingSPDG
Copy link
Contributor Author

Update: we couldn't have a tournament today cuz we only had 1team(lol).

@milkywayfarer
Copy link
Contributor

I can try it out tomorrow since we will have 2 teams ready, gonna build today

@milkywayfarer
Copy link
Contributor

@FlowingSPDG confirmed this change makes it that the map switches fine but the server crashes upon starting the knife round

@FlowingSPDG
Copy link
Contributor Author

@milkywayfarer thank you for confirming!

I believe that's when server start recording demo, so I'd say problem isn't solved yet...

I'm not sure if it's same/related bug or we just found a new bug🤔

@Iwhite67
Copy link
Contributor

on my side, i crash with version 0.12 but with 0.12 with fix he not crash, i test 3 times

@milkywayfarer
Copy link
Contributor

on my side, i crash with version 0.12 but with 0.12 with fix he not crash, i test 3 times

I tested everything with the latest releases of both MatchZy and CounterStrikeSharp for the time (0.7.13 and v247)

@Iwhite67
Copy link
Contributor

With 0.7.13 but with fix for tv start record ?

Can you confirm when veto is on progress, tv is not recording the demo ?

@milkywayfarer
Copy link
Contributor

With 0.7.13 but with fix for tv start record ?

Can you confirm when veto is on progress, tv is not recording the demo ?

Confirmed it does NOT crash on map veto and map change. It crashes when the demo starts recording (at the knife round start). I believe this PR was made after 0.7.13 released thus this release version applies

@Iwhite67
Copy link
Contributor

This PR has not made on 0.7.13 but on 0.7.12

If your matchzy version in game is 0.7.13, you are on a version without fix

@milkywayfarer
Copy link
Contributor

This PR has not made on 0.7.13 but on 0.7.12

If your matchzy version in game is 0.7.13, you are on a version without fix

@FlowingSPDG can you rebase this PR on master or the latest tag so that we can test that again this Wednesday?

@Iwhite67
Copy link
Contributor

Iwhite67 commented Jul 27, 2024

@FlowingSPDG

you need to add isDemoRecording = false;
in line 68

because he we not restart the server, isDemoRecording keep true and not record a new match

@Iwhite67
Copy link
Contributor

@milkywayfarer expect my issue, this is fix

i will make a PR on last version tomorrow

@FlowingSPDG
Copy link
Contributor Author

@milkywayfarer @milkywayfarer Sorry for the delay!

Merged latest dev and added isDemoRecording = false.

@milkywayfarer
Copy link
Contributor

milkywayfarer commented Aug 5, 2024

@FlowingSPDG hey! Just tested this version and sadly it still doesn't work.
Knife round completes normally, but server still segfaults after a side switch. Overall this PR achieves what needs to be done in terms of correctly starting a demo recording (there are no dangling previous map demos and knife rounds recorded), so you've got my approval to merge this :D

I actually came across a CS2 command that ignores TV delay when starting recording a demo, and adding it like that:

Server.ExecuteCommand("tv_record_immediate 1");
Server.ExecuteCommand($"tv_record ...");

didn't work. Maybe it needs to run just after the tv_enable 1 command in launch args, before the map selection, but I cannot test that until next week.

I believe our global problem is CS2's inability to correctly record demos from the dedicated servers having the tv_delay set as anything above 0, as I've seen issues on the official Valve repos about that. They all suggest to remove the delay or just disable the GOTV completely. The actual problem is that server crashes only when tv_enable == 1 and we use the map picking system of MatchZy. When you create a match with pre-picked maps (bo3 with 3 maps, for example), everything works flawlessly. Is there a difference between recording a demo for map picking scenario and without it? @shobhit-pathak fyi

I also tested out adding the CS2Fixes Metamod plugin to the set, as its' latest release specified that they fixed the crashes while recording demos. That didn't work.

P.S. server crashes before the restart on live (after LIVE! texts there is another restart, it doesn't go through)

@Iwhite67
Copy link
Contributor

Iwhite67 commented Aug 5, 2024

Strange because I made a full tournament with fix and we have 0 crash

@milkywayfarer
Copy link
Contributor

@Iwhite67 well there's my setup if you're curious:

OS: FreeBSD + k3s
Original image: joedwards/cs2
List of args from server logs:

-dedicated -ip 0.0.0.0 -port 27015 -console -usercon -maxplayers 10 +game_alias competitive +mapgroup mg_active +map de_inferno +sv_setsteamaccount ***+sv_lan 0 +net_public_addr *** +sv_logecho 1
protected command line arguments (stripped from above):
+rcon_password <protected> +sv_password <protected>

Running plugins and their versions:

  • Metamod: mmsource-1.11.0-git1155-linux, default config
  • CounterStrikeSharp: v253, default config
  • MatchZy: this PR published using dotnet 8.0.100, slightly modified config

I used this setup all the time, and playing without the online mappicks works, but with mappicks - doesn't.
Another environment was an Ubuntu 22.04 LTS Server VM and I didn't use k3s to run the server - still doesn't work.

@FlowingSPDG
Copy link
Contributor Author

Hmm it's storange... I guess it's CS2 server issue anyway so we can close this PR and move our discussion to somewhere else?

@milkywayfarer
Copy link
Contributor

PR is good, it would help with correct demo recordings, so I suggest that you push it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants