Skip to content

Add multiple argument support to array.push()#62

Merged
Perryvw merged 5 commits intoTypeScriptToLua:masterfrom
zapp-brannigan-dota:array-push-multiple
Mar 11, 2018
Merged

Add multiple argument support to array.push()#62
Perryvw merged 5 commits intoTypeScriptToLua:masterfrom
zapp-brannigan-dota:array-push-multiple

Conversation

@zapp-brannigan-dota
Copy link
Copy Markdown
Contributor

If this is accepted the wiki can be updated too.

@lolleko
Copy link
Copy Markdown
Member

lolleko commented Mar 10, 2018

Might be better to do that in lualib instead of hacking it into the translation.

@zapp-brannigan-dota
Copy link
Copy Markdown
Contributor Author

Yeah that's much cleaner.

@lolleko
Copy link
Copy Markdown
Member

lolleko commented Mar 10, 2018

Might also be better to use tbl[#tbl + 1] = "newValue" instead of table.insert since its faster. Opinions @Perryvw?

And the tests should be updated to.

@Perryvw
Copy link
Copy Markdown
Member

Perryvw commented Mar 10, 2018

If tbl[#tbl + 1] is faster I don't see why not.

@lolleko
Copy link
Copy Markdown
Member

lolleko commented Mar 10, 2018

I think JIT probably already optimises that, but its defiantly faster on other Lua versions.

@lolleko lolleko self-requested a review March 10, 2018 21:18
@TestCase([1], [0, 1])
@TestCase([1, 2, 3], [0, 1, 2, 3])
@Test("array.push")
public arrayPush(inp: number[], expected: number[]) {
Copy link
Copy Markdown
Member

@Perryvw Perryvw Mar 10, 2018

Choose a reason for hiding this comment

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

Remove expected from the arguments and compare to [0].concat(inp)

@Perryvw Perryvw merged commit 8f692fd into TypeScriptToLua:master Mar 11, 2018
@zapp-brannigan-dota zapp-brannigan-dota deleted the array-push-multiple branch March 11, 2018 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants