sourcemod

sourcemod copied to clipboard

Added the following stocks:

• IsStringNumber
• IsStringFloat
• StringToLowerCase
• StringToUpperCase
• IndexOfChar
• LastIndexOfChar
• CopyC

Tested: Test plugin source link: https://pastebin.com/hqfSZNxY Test results: https://i.imgur.com/4CawNKq.png

I have simplified the StringToUpper and StringToLower case. Regarding the FindCharInString, I did not know about that function, I can change IndexOfChar and LastIndexOfChar to wrap the FindCharInString. IsStringNumber and IsStringFloat do not work intended when they wrap StringToInt and StringToFloat. Results of the test with those: https://i.imgur.com/4l1JzGd.png

That's why dvander suggested using the Ex variants of those natives. They return the number of characters consumed for the number, so checking if the whole string was consumed does what you want.

stock bool IsStringInteger(const char[] str, int nBase=10)
{
	int result;
	return StringToIntEx(str, result, nBase) == strlen(str);
}

stock bool IsStringFloat(const char[] str)
{
	float result;
	return StringToFloatEx(str, result) == strlen(str);
}

If you want to be very strict, you can do:

stock bool IsStringFloat(const char[] str)
{
	float result;
	return StringToFloatEx(str, result) == strlen(str) && !IsStringInt(str);
}

But I'm not sure I'm in favor of this. Integers are a subset of floats, and it's kind of arbitrary to demand that decimals are added for whole numbers.

Changed the code to apply your advices.

Changed IsStringNumber and IsStringFloat to wrap StringToIntEx and StringToFloatEx. Changed IndexOfChar to wrap FindCharInString. Added CopyC

Tested, everything works: Test plugin source link: https://pastebin.com/hqfSZNxY Test results: https://i.imgur.com/4CawNKq.png

@milutinke I've made some adjustments that I thought were appropriate. What do you think?

@milutinke any update on this?

@milutinke any update on this?

Sorry for replying this late, I've not received a notification about this for some reason back then. CopyC does not exist in Source Mod, at least to my knowledge. Also, I think that return StringToFloatEx(str, result) == strlen(str); will not work properly in IsStringFloat without && FindCharInString(str, '.') != -1; if I remember correctly.

If the goal is to add more string stocks for common stuff, I feel like borrowing a few more ideas from python would make sense. Such as IsStringAlpha/Numeric/AlNum, IsStringASCII, IsStringPrintable, StringEndsWith. There's already char versions of some of those, so making string equivalents doesn't seem unreasonable.

If the goal is to add more string stocks for common stuff, I feel like borrowing a few more ideas from python would make sense. Such as IsStringAlpha/Numeric/AlNum, IsStringASCII, IsStringPrintable, StringEndsWith. There's already char versions of some of those, so making string equivalents doesn't seem unreasonable.

+1 I've missed StringEndsWith in almost every other plugin I made.

Sorry to bump, can we get some update on this? I feel like converting String case is a very fundamental operation and it would be nice to have SM ship with it.

Would adding wrappers for other available stock functions really help or make the API more confusing?

I've reviewed this again and am in favor of adding more string utility stocks as suggested, but am hesitant on adding wrappers like IndexOfChar -> FindCharInString. Having multiple functions doing the same thing doesn't help readability of code. But FindCharInString is a mouthful and not the best choice of name for what it does after all.

Would adding wrappers for other available stock functions really help or make the API more confusing?

I've reviewed this again and am in favor of adding more string utility stocks as suggested, but am hesitant on adding wrappers like IndexOfChar -> FindCharInString. Having multiple functions doing the same thing doesn't help readability of code. But FindCharInString is a mouthful and not the best choice of name for what it does after all.

People who come from other programming languages are used to IndexOfChar name, so it would not hurt having it in my opinion. But yeah, I am fine without it being added too.

I have an assortment of issues with the current functions, but I'm told I should put the improvements here instead of making a competing PR. Sorry in advance.

Bug

• IsStringNumber/IsStringFloat both incorrectly consider an empty string as valid.

Naming

• IsStringNumber only detects integers so it should be called IsStringInteger
• IsStringFloat is misleading. As dvander mentioned it should (and does) handle integer values too (usually what's important is "can it convert to a float"). IsStringDecimal would be better as StringToFloat requires base-10 numbers anyway.
• StringToLowerCase should be StringToLower both because it's shorter and because it matches IsCharLower and CharToLower
• Ditto for StringToUpperCase

Style

• Hungarian notation (prefixing with the type) should be removed from params and names should be made more consistent with the other functions. e.g. szInput -> str
• Variables also should not use hungarian notation
• EOS is never used anywhere in sourcemod's codebase except a single test case. I'm not sure if it's considered bad to use compared to '\0' but it might be.
• Both the StringToLower/Upper functions could've just been for-loops for readability sake: for (int i = 0; str[i] != '\0'; ++i) is a common idiom.
• In IndexOfChar's docstring, "Do we need the last index (Optional, default: false)" should probably borrow the description from FindCharInString's parameter: "False (default) to search forward, true to search backward."

Misc

• IsStringNumber is missing the nBase parameter in its docstring
• Docstrings have incorrect @return descriptions--the meaning of the return values (not the type) is supposed to be there. So for IsStringFloat for example, @return Boolean should be changed to something like @return True if string is a valid float number.
• @return is not needed for void functions.
• "Input string for conversion and also the output" is unclear and wordy. You could borrow from CharToLower/Upper: "Converts each character in a string to its lowercase counterpart."
• "Supports negative values" should not need to be clarified for IsStringNumber/IsStringFloat since not supporting them is incorrect.
• The docstrings for every single function have the incorrect parameter names for every parameter.
• For IndexOfChar, "index of the found character" is unclear because it actually finds the first occurrence of the character.
• (Picky) const char cCharacter doesn't really need to be const since scalar values are always passed by-value (i.e. copied) anyway.