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

[shortcuts] Pass valid commands through if target and command is valid #2238

Merged

Conversation

WinterSolstice8
Copy link
Contributor

I found the real root cause of #2225

Apparently that was being caused by low player IDs, which explains why it would not happen on retail.

Using the menu to use Cure 1 yourself results in a /magic "Cure" <id> but shortcuts interpreted that as cure 5 because the player ID I was had was 5.
image

This PR adds a check for a "complete" normal command with a valid target and passes it straight through. Wanted behavior, such as //cure 5 results in /ma "cure v" <me> as expected.

@RubenatorX RubenatorX requested a review from Byrth October 26, 2022 17:43
Copy link
Contributor

@Byrth Byrth left a comment

Choose a reason for hiding this comment

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

I don't want to merge this because it is only necessary due to a private server bug (defined as anything that makes the private server different from retail.)

I recommend fixing the private server instead of adding weird unexplained logic to shortcuts. Lord knows it already has enough.

@Byrth Byrth closed this Nov 8, 2022
@paulframe
Copy link
Contributor

paulframe commented Nov 8, 2022

@Byrth this is incorrect. We have encountered retail players with exceptionally low IDs. It is not a private server only issue. It simply affects more of the private server population than retail.

@TeoTwawki
Copy link

I don't want to merge this because it is only necessary due to a private server bug (defined as anything that makes the private server different from retail.)

False. It's just that getting a low id on retail is rare nowadays due to the games age.

I recommend fixing the private server instead of adding weird unexplained logic to shortcuts. Lord knows it already has enough.

There isn't anything to fix.

@WinterSolstice8
Copy link
Contributor Author

WinterSolstice8 commented Nov 9, 2022

I recommend fixing the private server instead of adding weird unexplained logic to shortcuts.

An ID 1 >= and <= 6 is not a bug. All this PR is doing is optimizing shortcuts, it already knows the command and target is valid. There is nothing unexplained or weird, unless you would like to enlighten me because I am genuinely curious about that. If anything, this is a PR to optimize shortcuts. You are essentially claiming your own functions are unexplained and weird, shortcuts already knows that the command and the target are valid, I'm just adding a check to pass it through.

@Byrth Byrth reopened this Nov 13, 2022
@Byrth
Copy link
Contributor

Byrth commented Nov 13, 2022

Ok then!

@Byrth Byrth merged commit 4fed9b2 into Windower:dev Nov 13, 2022
z16 pushed a commit that referenced this pull request Nov 4, 2024
…_low_player_IDs

[shortcuts] Pass valid commands through if target and command is valid
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.

4 participants