Skip to content
This repository was archived by the owner on Jun 23, 2023. It is now read-only.

Conversation

@LeShred
Copy link
Contributor

@LeShred LeShred commented Apr 5, 2015

Allowing using double quotes to set commands parameters with spaces.
eg: /admin move player position "John Doe" 100 100 100

Use double quotes twice in order to set include a double quote in the parameter.
eg: /admin move player position "John""Doe" 100 100 100 (for player John"Doe)

Allowing using double quotes to set commands parameters with spaces.
eg: /admin move player position "John Doe" 100 100 100
@dodexahedron
Copy link
Member

Thanks for this.
I was going to write the command parser as a state machine, but you went ahead and did it. :)
Have you tested it in all commands and in the automated cleanups, yet?

@LeShred
Copy link
Contributor Author

LeShred commented Apr 5, 2015

You're welcome!
I didn't tested it with the automated cleanup. I wasn't aware there is an automated cleanup, I started working on the plugin sources this morning and didn't read all the sources currently. How can I test with the automated cleanup?

@dodexahedron
Copy link
Member

Automated cleanups can be found in the settings for Essentials.
They're timed or triggered events that use scan commands to delete objects based on what the scan returns.
Any change in behavior to that will result in a lot of people having unexpected ship deletions, which is always fun.
So we just need to be sure that the behavior of the new parsing logic behaves the same as the old logic, for unquoted values.

@LeShred
Copy link
Contributor Author

LeShred commented Apr 5, 2015

OK I see, it's the Timing and triggered cleanups. I will check the behavior with those.

@dodexahedron
Copy link
Member

Awesome.
Thanks

dodexahedron added a commit that referenced this pull request Apr 6, 2015
Improved chat commands parsing.
@dodexahedron dodexahedron merged commit d3c40ce into SEServerExtender:master Apr 6, 2015
dodexahedron added a commit to SEServerExtender/SEServerExtender that referenced this pull request Apr 6, 2015
@dodexahedron
Copy link
Member

I tested this out myself, and it seems to work well.
I then merged it and realized that it would be useful to standardize command parsing among plugins and SESE itself, so I backed the change out and then put a modified version of the code in SESE, in a utility class.
The behavior is the same, but it is just a little more efficient.
Thanks for this contribution!

@LeShred
Copy link
Contributor Author

LeShred commented Apr 6, 2015

I just had a look before seeing your message and realized that timed and triggered cleanups are using command.Split( ' ' ) and thus the parsing has to be moved to an utility class :)
Did you also modified ProcessTimedItem and ProcessTriggerItem in order to use the new parser you implemented in SESE?

@LeShred
Copy link
Contributor Author

LeShred commented Apr 6, 2015

Oh and thanks for the code improvement! I did saw that you used a StringBuilder and just found the performance improvement it makes compared to simple string manipulation. I wasn't aware of that!

@LeShred
Copy link
Contributor Author

LeShred commented Apr 6, 2015

I drilled into the code of the plugin and found that some utility classes like CubeGrids use a string splitter - General.SplitString() - with double-quotes management and using the new parser with some commands can lead to unexpected behaviors. The included parser handles double-quotes but it doesn't handles double double-quotes which can be a problem.

For example in CubeGrid.ScanGrids() the command string is rebuilt in the first 2 lines and then analyzed. This will cause a problem with the command - /admin scan grids functional "hasdisplayname:My ""Super ship"":exact" - as it will split the displayname parameter into 3 chunks and so the command will not work as expected.

Please have a look at that and if you agree I would recommend removing the use of this string splitter. I can do it if you want.

yajiedesign pushed a commit to yajiedesign/SEServerExtender that referenced this pull request Dec 5, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants