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

Create menu if multiple nom matches found #983

Merged
merged 9 commits into from Aug 1, 2019

Conversation

@JoinedSenses
Copy link
Contributor

JoinedSenses commented Apr 23, 2019

This change checks the nomination against the map arraylist. If the nomination matches multiple results, those results are then added to a menu to allow the client to select available maps. The maps added to the menu go against the same checks as the normal nomination menu and wont allow nomination of disabled maps. There is also a defined MATCHED_INDEXES_MAX of 7, which means there will only be one page of matched maps. Update: Added convar sm_nominate_maxfound.

The main reason for this change is to allow for more flexibility of map nominations. In its current state, the plugin only attempts to nominate the very first match.

Example image of /nominate jump_b
Nom Menu

Image demonstrating that disabled maps cannot be selected.
Command used: /nominate jump_
Nom Menu2

I would also like feedback on the idea of registering command "sm_nom" with the same callback as "sm_nominate".

This change checks the nomination against the map arraylist. If the nomination matches multiple results, those results are then added to a menu to allow the client to select available maps. The maps added to the menu go against the same checks as the normal nomination menu and wont allow nomination of disabled maps.

Example image of /nominate jump_b
https://i.imgur.com/ZdzB0gk.png

I would also like feedback on the idea of registering command "sm_nom" with the same callback as "sm_nominate".
@JoinedSenses JoinedSenses changed the title Create menu is multiple nom matches found Create menu if multiple nom matches found Apr 23, 2019
Copy link
Member

Headline left a comment

Some general changes, but I really like this

plugins/nominations.sp Outdated Show resolved Hide resolved
plugins/nominations.sp Show resolved Hide resolved
plugins/nominations.sp Outdated Show resolved Hide resolved
plugins/nominations.sp Outdated Show resolved Hide resolved
plugins/nominations.sp Outdated Show resolved Hide resolved
plugins/nominations.sp Outdated Show resolved Hide resolved
plugins/nominations.sp Show resolved Hide resolved
plugins/nominations.sp Outdated Show resolved Hide resolved
@Headline

This comment has been minimized.

Copy link
Member

Headline commented Apr 23, 2019

Last thing that could probably use some discussion (aka don't implement it yet), we might want to add a check so player console & server console behavior is unaffected. I think this should only happen when a player executes !nominate <incomplete map name> in chat.

If reply source is console, menus wont be generated and it will attempt to nominate as normal
@JoinedSenses

This comment has been minimized.

Copy link
Contributor Author

JoinedSenses commented Apr 24, 2019

I went ahead and went against your message and added an implementation to demonstrate what I had in mind for checking source.

If reply source is console, menus wont be generated and it will attempt to nominate as normal.

Copy link
Member

Headline left a comment

A few questions

plugins/nominations.sp Show resolved Hide resolved
plugins/nominations.sp Outdated Show resolved Hide resolved
Copy link
Member

Headline left a comment

I'm okay with this, but I'd like to wait for another smdev to double check it before a merge

@asherkin can I get your 👀 on this?

Copy link
Member

asherkin left a comment

🚢

Copy link
Member

KyleSanderson left a comment

Yikes. I too like the idea but the exploit should be fixed before merging.

plugins/nominations.sp Show resolved Hide resolved
plugins/nominations.sp Show resolved Hide resolved
@Headline Headline requested a review from KyleSanderson Aug 1, 2019
@asherkin asherkin merged commit b8fd7db into alliedmodders:master Aug 1, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@asherkin

This comment has been minimized.

Copy link
Member

asherkin commented Aug 1, 2019

Thanks for this!

BotoX added a commit to BotoX/sourcemod that referenced this pull request Sep 10, 2019
This change checks the nomination against the map arraylist. If the nomination matches multiple results, those results are then added to a menu to allow the client to select available maps. The maps added to the menu go against the same checks as the normal nomination menu and wont allow nomination of disabled maps.

Example image of /nominate jump_b
https://i.imgur.com/ZdzB0gk.png

If reply source is console, menus wont be generated and it will attempt to nominate as normal.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.