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

Pause/Unpause events and forwards #626

Merged
merged 1 commit into from
Dec 26, 2021
Merged

Conversation

PhlexPlexico
Copy link
Collaborator

@PhlexPlexico PhlexPlexico commented Jan 22, 2021

Hi!

Per the issue #412, this PR creates some forwards and events for pausing and un-pausing in game, so that they can be used in other applications/plugins as well (such as a web-panel notifying if a match is paused).

I saw that PR #386 hasn't been acted upon since I last looked, and I figured I would take a stab at the fixed timeout issue mentioned, as well as provide some context for a pause.

The pause events also take in which team is making the call, and a reason, either tactical or technical, this is to provide some context and help with a forward to determine why a pause was called.

I tested a few of the different possibilities for pausing, including fixed timer and tech pauses, and I think the events successfully pause/unpause under every event.

@PhlexPlexico PhlexPlexico changed the title Pause/Unpause events and forwards (#412) Pause/Unpause events and forwards Jan 22, 2021
Copy link
Contributor

@Drarig29 Drarig29 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some suggestions on what to change.

And maybe you should explain more the changes you made, especially the Timer_UnpauseEventCheck().

Basically, tell that:

  • You cancel the timer if not pauseable
  • You cancel the timer if the pause time is unlimited
  • You wait while the match is not freezed (!InFreezeTime)
  • You trigger the unpause event when g_PauseTimeUsed is less than or equal to 0 (meaning the pause is finished).

@@ -38,6 +38,11 @@ enum SideChoice {
SideChoice_KnifeRound, // There will be a knife round to choose sides
};

enum PauseType {
PauseType_Tech, // Technical pause
PauseType_Tac, // Tactical Pause
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe say PauseType_Tactical instead, it's easier to read.

@@ -78,10 +84,8 @@ public void EventLogger_SeriesStart() {

public void EventLogger_MapVetoed(MatchTeam team, const char[] map) {
EventLogger_StartEvent();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try not to change code in places you are not doing anything important. And maybe follow the existant style.

Like, keep the same style with blank lines in the event logger functions for Pause and Unpause too. 😊

EventLogger_EndEvent("pause_command");
}

public void EventLogger_Unpause(MatchTeam team) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Command suffix is missing in EventLogger_UnpauseCommand.

Call_StartForward(g_OnMatchPaused);
Call_PushCell(team);
Call_PushCell(PauseType_Tac);
Call_Finish();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A blank line after this? 😅 (sorry, nitpicking)

Get5_MessageToAll("%t", "AdminForceTechPauseInfoMessage");
return Plugin_Handled;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not remove this line 😊

// Keep running timer until we are paused.
return Plugin_Continue;
} else {
if (g_PauseTimeUsed <= 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if instruction is badly indented.

CreateTimer(1.0, Timer_PauseTimeCheck, team, TIMER_REPEAT | TIMER_FLAG_NO_MAPCHANGE);
// Keep track of timer, since we don't want several timers created for one pause checking instance.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure you need to keep track of it? This would be the only case of that in the project.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! Thanks for taking the time review, it's much appreciated!

I think initially I wanted to keep track of this here because I had an initial assumption/testing that a timer would be created each time a person called the !pause command, which would create a new timer and erroneously cause another timer, and thus event firing, to be sent off when the timer finished. However, I don't think this is the case upon further review. I'll make that change and remove the handler and test that out here :) Thanks again!

@PhlexPlexico
Copy link
Collaborator Author

Hey @Drarig29!

So I think there may be an issue with pausing that I just encountered, and was wondering how to continue. I've removed the global handler from the timer counter, however I noticed that if you pause during the countdown to knifing (as an example, haven't tested without knife rounds), your pause count goes down, but the game won't pause when it moves to the next state. This will also increase the countdown timer for this functionality to *2 what it should be, and will also create additional timers, but also sends out however many EventLogger_UnpauseCommand calls in relation to the amount of !pauses called.

I think somewhere along one of my commits, I used the global handler to check if the timer was running (and also had g_PauseTimeUsed under that if statement) to avoid the new event from being set off twice. However, that's more of a band-aid fix than anything.

I also tested this pause issue with the latest commit on the master branch, and it has the same issue where a team's timeout gets taken away and the game does not go into a paused. Should a separate PR/issue be created for this? If so, I won't put a possible fix in this merge.

I think this is due to the fact that when both teams are ready, and the game state flips from warmup to knife round during the warmup countdown to knife. This can be observed here

ChangeState(Get5State_KnifeRound);

But then mp_warmuptime is called for 5 seconds.

https://github.com/splewis/get5/blob/master/scripting/get5/kniferounds.sp#L7

For these 5 seconds, a user on a team can call !pause and have one of their pauses taken away for the half, but not have freeze time within the knife round. Is there a possible way to check if the game is in a current countdown (would this still be m_bFreezePeriod?) then just return the plugin being handled again to avoid this?

Thanks!

@Drarig29
Copy link
Contributor

Drarig29 commented Mar 4, 2021

Because you tested the issue on the master branch, we are sure it's not your PR which added the bug. So yeah, IMO you should make another issue and a linked PR.

And sorry I'm not an expert in CS:GO timings, I'm not even perfectly familiar with the game.

What's more, I don't have a server to do the testing anymore, I have a new PC.

I just wanted to give a review to wake up the PR 😁

@PhlexPlexico PhlexPlexico force-pushed the pause-events branch 2 times, most recently from 9276503 to 1c872d2 Compare August 19, 2021 23:51
@PhlexPlexico
Copy link
Collaborator Author

Sorry to wake this old PR, but I was wondering if we could get any progress on it? It would be a nice addition to have so people with different plugins that record events could check if a match is live, or ping someone in discord/web if a match is paused.

Thanks!

@PhlexPlexico PhlexPlexico force-pushed the pause-events branch 2 times, most recently from 9ceca79 to b85bc86 Compare December 16, 2021 18:14
@PhlexPlexico
Copy link
Collaborator Author

@splewis Apologies for the ping, but I was wondering if this could get reviewed/merged? It would be nice from an event-based perspective to be notified when matches get paused if organizers are running more than one match at a time.

@splewis
Copy link
Owner

splewis commented Dec 21, 2021 via email

@splewis splewis merged commit fd8b3ef into splewis:master Dec 26, 2021
@PhlexPlexico PhlexPlexico deleted the pause-events branch December 27, 2021 16:34
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