Skip to content

Conversation

@mgaertne
Copy link
Contributor

added functionality to switch suggested players by default and have both !veto the switch or an admin !dont it. Controlled by cvar qlx_balanceSwitchOptIn

added functionality to switch suggested players by default and have both !veto the switch or an admin !dont it. Controlled by cvar qlx_balanceSwitchOptIn
@em92
Copy link
Collaborator

em92 commented Jul 20, 2021

Hi, @mgaertne!
Name of cvar "qlx_balanceSwitchOptIn" is confusing. How about naming it as qlx_balanceAutoSwitch?

Also, it would be good, to add documentation on this cvar after approving cvar name.

Comment on lines 512 to +519
"""Forces a suggested switch to be done."""
if self.suggested_pair:
self.execute_suggestion()
if not self.suggested_pair:
return

if not self.switch_opt_in:
return

self.execute_suggestion()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Player joins on round countdown, script notices that next round switch will be executed, admin wants to execute it right now, types "!do" and says "wtf?".

Not to have this situation, I suggest to return previous version of cmd_do

Comment on lines -505 to +536
if self.suggested_pair and not all(self.suggested_agree):
p1, p2 = self.suggested_pair
if not self.suggested_pair or not self.switch_opt_in or all(self.suggested_agree):
return
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would move swtich_opt_in checking to top of cmd_agree function. To indicate that this command is not used on auto switch enabled.

Comment on lines +558 to +559
if not self.suggested_pair or self.switch_opt_in:
return
Copy link
Collaborator

Choose a reason for hiding this comment

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

@em92 em92 marked this pull request as draft July 20, 2021 08:45
@mgaertne
Copy link
Contributor Author

mgaertne commented Jul 20, 2021 via email

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.

2 participants