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

fix incorrect command reply in nominations #1161

Merged
merged 1 commit into from
Feb 7, 2020

Conversation

thorgot
Copy link
Contributor

@thorgot thorgot commented Jan 20, 2020

When a map is nominated and the max nominations have been reached, the wrong text is returned if the map hasn't been nominated yet. Instead of printing the "Max Nominations" text, it prints out the "Map Already Nominated" text instead.

When a map is nominated and the max nominations have been reached, the wrong text is returned if the map hasn't been nominated yet. Instead of printing the "Max Nominations" text, it prints out the "Map Already Nominated" text instead.
@Headline Headline added the Bug general bugs; can be anything label Feb 7, 2020
@Headline
Copy link
Member

Headline commented Feb 7, 2020

Thanks for contributing!

Looking over this, I'm seeing an explicit check for maps that have already been nominated. I believe something more nefarious is going on than a simple logic issue. The part you have edited, from what I've observed, is only possible to be hit when the map list is full, indicating to me that the original message is correct

if ((status & MAPSTATUS_EXCLUDE_NOMINATED) == MAPSTATUS_EXCLUDE_NOMINATED)
{
ReplyToCommand(client, "[SM] %t", "Map Already Nominated");
}

@thorgot
Copy link
Contributor Author

thorgot commented Feb 7, 2020

Thanks for contributing!

Looking over this, I'm seeing an explicit check for maps that have already been nominated. I believe something more nefarious is going on than a simple logic issue. The part you have edited, from what I've observed, is only possible to be hit when the map list is full, indicating to me that the original message is correct

if ((status & MAPSTATUS_EXCLUDE_NOMINATED) == MAPSTATUS_EXCLUDE_NOMINATED)
{
ReplyToCommand(client, "[SM] %t", "Map Already Nominated");
}

MAPSTATUS_DISABLED is the status flag when the map is disabled in the nominate menu. The block you linked to is reached when the map has been nominated already, is the current map, or is in the exclude list (which is the last X maps). A map which has not been nominated, has not been played recently, and is not the current map will skip that entire if block and reach the line I changed.

@Headline
Copy link
Member

Headline commented Feb 7, 2020

Oh; I see, my mistake.

Thank you for clarifying, I was a little confused about what was going on. I was under the impression that Max Nominations was being replaced with Map Already Nominated for some reason, but this all appears correct.

Thank you very much!

Copy link
Member

@Headline Headline left a comment

Choose a reason for hiding this comment

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

🚢

@Headline Headline merged commit 452338d into alliedmodders:master Feb 7, 2020
asherkin pushed a commit that referenced this pull request Feb 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug general bugs; can be anything
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants