-
-
Notifications
You must be signed in to change notification settings - Fork 632
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
Fixes #493 and #490 #1807
Fixes #493 and #490 #1807
Conversation
Thanks for this, finally someone who can understand the Lua bindings a bit and can help with good code! :) I do have some concerns about the code, I'll comment on it in a moment. |
{ | ||
cLuaState LuaState(tolua_S); | ||
std::string str = (std::string)tolua_tocppstring(LuaState, 1, 0); | ||
std::string delim = (std::string)tolua_tocppstring(LuaState, 2, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a nice template function cLuaState::GetStackValues()
that is better at doing this - it can handle strings with embedded NULs etc. I'd prefer if you used it instead.
Thanks for the feedback. |
while ((cutAt = str.find_first_of(delim, Prev)) != str.npos) | ||
{ | ||
AString current = str.substr(Prev, cutAt - Prev); | ||
if ((current.at(0) == '"') || (current.at(0) == '\'')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is std::string::at preferred, as opposed to unchecked [] operators? I understand that since it throws exceptions and no exception handlers exist around here, it provides no benefits whilst negatively impacting speed.
What about std::string::front instead of [0]? (simply to have the same syntax as in src/StringUtils.cpp Line 176. |
If you want, though I wouldn't be surprised if the compiler identified it for optimisation anyway. |
Performance improvements for #1807
I hope this should fix the referenced issues properly?
(Tested only on Windows; Linux to be done)