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

Fix small overhead in argument parser for strings #3294

Merged
merged 5 commits into from
Jun 16, 2024
Merged

Fix small overhead in argument parser for strings #3294

merged 5 commits into from
Jun 16, 2024

Conversation

TheNormalnij
Copy link
Member

lua_strlen looks unnecessary.

lua_tolstring returns string length in third argument.
lua_tostring is macro lua_tolstring(L, (i), NULL)

@TheNormalnij TheNormalnij added enhancement New feature or request bugfix Solution to a bug of any kind and removed enhancement New feature or request labels Jan 16, 2024
@TheNormalnij TheNormalnij added this to the 1.6.1 milestone Jan 16, 2024
@TheNormalnij
Copy link
Member Author

fixed

Nico8340
Nico8340 approved these changes Apr 7, 2024
View reviewed changes
Copy link
Contributor

@Nico8340 Nico8340 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :shipit:

@Dutchman101
Copy link
Member

Reviewed by 3 people, and all code reviews were addressed; ready to merge

@Dutchman101 Dutchman101 merged commit d20582d into multitheftauto:master Jun 16, 2024
6 checks passed
@TheNormalnij TheNormalnij deleted the TheNormalnij/fix_parser branch June 16, 2024 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tederis tederis tederis left review comments

@Pirulax Pirulax Pirulax left review comments

@Nico8340 Nico8340 Nico8340 approved these changes

Assignees
No one assigned
Labels
bugfix Solution to a bug of any kind
Projects
None yet
Milestone
1.6.1
Development

Successfully merging this pull request may close these issues.
5 participants
@TheNormalnij @Dutchman101 @tederis @Pirulax @Nico8340