Building a best Cmd_TokenizeString/COM_Parse

[OmegaPhlare]

[Spike]

com_parse is a generic function that can be used for all sorts of things, including console, saved games, and entity lumps from bsps.
both saved games and entity lumps make extensive use of { and } chars, for instance. and something like '{classname worldspawn}' miss-parsing would be annoying when you're just trying to debug something quickly.

No, you don't need those chars for the console. But for generic text parsing (like doom3's various text-based data files), handling '(2,3)' as 5 separate tokens means that you can actually parse such text in a sane way without caring about specific chars or whitespace.

If you want to be fancy, you can have some 'if (strchr(punctuation, c))' line to check if c is one of your punctuation chars, and then just pass in a string as an argument to the function, allowing arbitary-but-single-char punctuation to be parsed as separate tokens.

[mh]

Way I see it is that there are two goals here.

Be robust.
Maintain compatibility.

If the second goal is not a concern of your's (i.e. you're not handling Quake data) then you can do what you like. Convert everything to XML and use an XML parser, even.

Otherwise you're going to find that you may need to retain what looks like bugs and weirdness in the original. Quake has a lot of odd cases where you might think that you've fixed what looks like a bug, but a few days/weeks/months down the line somebody's mod that relied on that behaviour is going to blow up.

Erring on the safe side and grabbing QuakeWorld's version of COM_Parse seems the best option here - at least it's known to work with the majority of Quake data, and is known to the community so you can ask for help if you run into trouble.

[OmegaPhlare]

Oh that's interesting. If you keep it in a way that creates single tokens using punctuation, then you can make files that are more easily human readable while avoiding having to write your own additional handling. Exactly like you said, Spike, if I had to parse a number coordinate like (2,8) then it would be a pain in the ass for a function using that data to have to strip the punctuation away from it and create it's own two number split, when it could have just been done already by creating those 5 tokens.

I had completely neglected to think about compatibility, and in my case I'm lucky that I don't actually need it. The developers were allowed to do so without any repercussions because there was no existing user content with backwards compatibility to be met. I agree it just wouldn't be wise to modify this code if I ever begin working on the Quake source. I'm going to take your advise and look up QuakeWorld's COM_Parse which sounds different.

Ultimately I think that I will end up making it more robust but incompatible, but what I learned from this is that there is not a good reason to create a better Cmd_TokenizeString or COM_Parse because doing so would bring more harm than good. Good judgement, I'll try to follow that.

[Spike]

pretty much.
if you don't need compatibility then keep it simple. seriously.
if you need a com_parse, make one, but don't make it part of the command parser.
If you do pass scripts via commands, just keep the original 'Cmd_Args()' result and parse that directly. Trying to do both commands and scripts with the same function (without some 'script' argument) is pointless as you cannot really all expectations.

Actually, skipping // comments in your command parser can be quite handy for stuffcmd-based extensions and not spamming people with old versions.
stuffcmd is evil though, if you choose to support it, it should have a separate set of commands from the regular console - an example bug: 'record pak0.pak' - yes, now you need to reinstall quake. Worst part is that example accepts multiple leading ../