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.

Sign up for GitHub

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

Jump to bottom

Fix fixbot command #216

Merged
merged 1 commit into from
Aug 3, 2020
Merged

Fix fixbot command #216

merged 1 commit into from
Aug 3, 2020

Conversation

sneak-it
Copy link
Member

@sneak-it sneak-it commented Aug 1, 2020

Fix duplicate and breaking fixbot permissions check, allow to be used via server console

@Kyli3Boi
Copy link
Contributor

Kyli3Boi commented Aug 1, 2020

Thanks sneak

olokos
olokos approved these changes Aug 2, 2020
View reviewed changes
Copy link
Collaborator

@olokos olokos left a comment

Choose a reason for hiding this comment

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

Thank you very much!
As far as I'm aware there are checks being done on client before this command can even be called, so this nested if statement is possibly reduntant.

@sneak-it
Copy link
Member Author

sneak-it commented Aug 2, 2020 •
edited
Loading

As far as I'm aware there are checks being done on client before this command can even be called, so this nested if statement is possibly reduntant.

The permission assigned to it is being checked in the registered command:
https://github.com/surftimer/Surftimer-olokos/blob/a3a8aaf897991afbe6ee327dfb28f7a72de88074/addons/sourcemod/scripting/surftimer/commands.sp#L111
The nested checks were not only redundant, but also horribly hard-coded and didn't allow using SourceMod's overrides system properly (as discussed in Discord).

@qawery-just-sad
Copy link
Contributor

the same is true for all vip commands (just saying)

@sneak-it
Copy link
Member Author

sneak-it commented Aug 2, 2020

@qawery-just-sad commented on Aug 2, 2020, 6:01 AM EDT:

the same is true for all vip commands (just saying)

Oh gross, that "IsPlayerVip" stock needs to be nuked from orbit. Looks like that's used on a lot of VIP commands, not so much the case with fixbot, which is outside the scope of this PR.

@olokos
Copy link
Collaborator

olokos commented Aug 2, 2020

@sneak-it
Feel free to self-approve this if you think this PR can be closed already.
Possibly a good idea would be to create a new issue when this PR is closed, just to remember about the vipCommands/removal of vipCommands.

@sneak-it sneak-it merged commit 78a0fbc into surftimer:dev Aug 3, 2020
@sneak-it sneak-it mentioned this pull request Aug 3, 2020
Closed
dPexxIAM
dPexxIAM reviewed Jan 30, 2021
View reviewed changes
Copy link
Contributor

@dPexxIAM dPexxIAM left a comment

Choose a reason for hiding this comment

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

lgtm

Bara added a commit that referenced this pull request Sep 5, 2021
@LubricantJam @tomtko
@tahvoo @Bara @sneak-it @FR33K1LL
Merge some fixes from the dev branch to master (#293)
9f89157 
* Fixed silent spec

* Fix for tier issue #201

* display center speed OnClientPutInServer  so doesnt have to be re-enabled in options

* Fix incorrect percentage value

* Revert "Fix incorrect percentage value"

This reverts commit bb92e72.

* Fix incorrect percentage value

* Fix math

* Fix duplicate and breaking fixbot permissions check, allow to be used via server console (#216)

* Fixed scoreboard ordering for unranked players

* Fix 2 memory leaks

* Update commands.sp

This puts the freestyle menu option back in !top

* Minor fix for vote-extend

Co-authored-by: Lubricant Jam <[email protected]>
Co-authored-by: fluffyst <[email protected]>
Co-authored-by: fatfuk <[email protected]>
Co-authored-by: Bara <[email protected]>
Co-authored-by: sneak-it <[email protected]>
Co-authored-by: FR33K1LL <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dPexxIAM dPexxIAM dPexxIAM left review comments

@olokos olokos olokos approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sneak-it @Kyli3Boi @qawery-just-sad @olokos @dPexxIAM