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

Player commands overloading bugs #392

Open
KirillBorunov opened this issue Jan 10, 2022 · 1 comment · May be fixed by #393
Open

Player commands overloading bugs #392

KirillBorunov opened this issue Jan 10, 2022 · 1 comment · May be fixed by #393
Labels
area-Commands Issues related to player/RCON commands area-GM Issues related to SampSharp.GameMode bug
Milestone
0.11.0

Comments

@KirillBorunov
Copy link
Contributor

I created a two variants of same command:
A) /admin skin [targetPlayerId] [skinId]
B) /admin skin [skinId]

All parameters are int.

When the server loads in the console log I see both variants are loaded:

[SampSharp:DEBUG] Registering command /admin skin [skinId]
[SampSharp:DEBUG] Registering command /admin skin [targetPlayerId] [skinId]

In game if I write /admin skin the server prints Usage: /admin skin [skinId]
If I write /admin skin 33 it prints Usage: /admin skin [targetPlayer] [skinId]
If I write /admin skin 1 33 then it works OK with the "A" variant.
It is impossible to use the "B" variant.

The commands are declared as suggested here: https://sampsharp.net/player-commands#grouping-commands
I'm not using permissions nor overriding the default commands behavior.
I tried to swap the order of the command declaration, it swapped the order of the command in the server load log, but it was not changed the behavior.
@KirillBorunov
Copy link
Contributor Author

The problem seems to be in each of the ICommandParameterType.Parse(...) implementations.
For example this one:

SampSharp/src/SampSharp.GameMode/SAMP/Commands/ParameterTypes/IntegerType.cs

Lines 42 to 58 in d294bc0

var text = commandText.TrimStart();
output = null;
if (string.IsNullOrEmpty(text))
return false;
var word = text.Split(' ').First();
// Regular base 10 numbers (eg. 14143)
if (word.All(Base10Characters.Contains) && int.TryParse(word, out var number))
{
commandText = commandText.Substring(word.Length).TrimStart(' ');
output = number;
return true;
}

The commandText initially may contain a space at start (more on this later).
In the line 42 is doing a trim and the space is removed, so the text variable contains the same as commandText minus the initial space.
In the line 55 is doing the substring on commandText, but assuming the trim. So the resulting command text has the las symbol from the first argument, and the next run will use the remaining symbol as a second argument...

Now, the first space may or may not be present, that depends on from where the call is arriving.
The difference is inside the DefaultCommand implementation. It has a method "CanInvoke" and "Invoke".
The first one is sending the arguments without a trim, but the second is doing a trim.
That explains why the bug with the arguments is not affecting the command execution, but only the command candidate finding.

SampSharp/src/SampSharp.GameMode/SAMP/Commands/DefaultCommand.cs

Line 297 in d294bc0

commandText = commandText.Substring(name.Length);

SampSharp/src/SampSharp.GameMode/SAMP/Commands/DefaultCommand.cs

Line 330 in d294bc0

commandText = commandText.Substring(name.Value.Length).Trim();

Changing all the ICommandParameterType implementations to do a trim on the commandText instead of a new variable will fix the issue.
But also can be fixed by adding a trim to the CanInvoke mentioned line.

@KirillBorunov KirillBorunov linked a pull request Jan 10, 2022 that will close this issue
Open
@ikkentim ikkentim added bug area-Commands Issues related to player/RCON commands area-GM Issues related to SampSharp.GameMode labels Mar 26, 2022
@ikkentim ikkentim added this to the 0.11.0 milestone Mar 26, 2022
@ikkentim ikkentim mentioned this issue Apr 20, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-Commands Issues related to player/RCON commands area-GM Issues related to SampSharp.GameMode bug
Projects
None yet
Milestone
0.11.0
Development

Successfully merging a pull request may close this issue.

Fix commands overloading KirillBorunov/SampSharp
2 participants
@ikkentim @KirillBorunov