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.

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

Correct description of global unpack's parameters #341

Merged
merged 5 commits into from
Dec 15, 2023
Merged

Correct description of global unpack's parameters #341

merged 5 commits into from
Dec 15, 2023

Conversation

dv-extrarius
Copy link
Contributor

Correct the description of the unpack parameters.

Changes

The description of unpack's j parameter was wrong. It is not a count, it is the index of the last element to unpack. Updated the description of the i parameter to match the new wording of the j parameter.

Checks

By submitting your pull request for review, you agree to the following:

  • This contribution was created in whole or in part by me, and I have the right to submit it under the terms of this repository's open source licenses.
  • I understand and agree that this contribution and a record of it are public, maintained indefinitely, and may be redistributed under the terms of this repository's open source licenses.
  • To the best of my knowledge, all proposed changes are accurate.

Correct the description of the unpack parameters.
@dv-extrarius dv-extrarius requested a review from a team as a code owner December 11, 2023 01:21
@github-actions github-actions bot added the engine reference Changes the Engine API Reference documentation label Dec 11, 2023
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hi @dv-extrarius, thanks so much for helping improve the Roblox creator documentation! Our technical writing team will review your pull request soon. In the meantime, please ensure you've read through the README.md, contribution guidelines, and style recommendations.

@github-actions github-actions bot added the changes requested This pull request has changes requested prior to merging label Dec 15, 2023
IgnisRBX
IgnisRBX previously approved these changes Dec 15, 2023
@IgnisRBX
Copy link
Contributor

I reverted the suggestion of the "j" parameter, which isn't the "index of the last element to unpack", but rather the number of indices to unpack (default to the total number of indices) starting from the given index number "i" (default of the first index).

@IgnisRBX IgnisRBX merged commit 4e37d5a into Roblox:main Dec 15, 2023
3 checks passed
@dv-extrarius
Copy link
Contributor Author

dv-extrarius commented Dec 16, 2023

I reverted the suggestion of the "j" parameter, which isn't the "index of the last element to unpack", but rather the number of indices to unpack (default to the total number of indices) starting from the given index number "i" (default of the first index).

That's not how it functioned for me in roblox. I had an array and wanted slices so I tried with i=1, 11, 21, etc and j=10 and only the first slice had elements. When I made j=10, 20, 30, etc it functioned as I wanted, slicing 10 elements at a time.

In roblox, this code:

local values = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}

local slice1 = {unpack(values, 1, 3)}
print("Slice1 length:", #slice1)

local slice2 = {unpack(values, 4, 3)}
print("Slice2 length:", #slice2)

local slice3 = {unpack(values, 7, 3)}
print("Slice3 length:", #slice3)

produces this output:

Slice1 length: 3  -  Server - Script:4
Slice2 length: 0  -  Server - Script:7
Slice3 length: 0  -  Server - Script:10

whereas this code in roblox:

local values = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}

local slice1 = {unpack(values, 1, 3)}
print("Slice1 length:", #slice1)

local slice2 = {unpack(values, 4, 6)}
print("Slice2 length:", #slice2)

local slice3 = {unpack(values, 7, 9)}
print("Slice3 length:", #slice3)

produces

Slice1 length: 3  -  Server - Script:4
Slice2 length: 3  -  Server - Script:7
Slice3 length: 3  -  Server - Script:10

which shows j is not, in fact, a count.

@Hexcede
Copy link

Hexcede commented Dec 17, 2023

rather the number of indices to unpack

This is incorrect. Unpack takes two indices, the second argument is not the count but rather the last index. The second argument does default to the length of the input table, but this is because the last index of the table matches the length, not because the argument is a count. This is confirmed by both the Lua 5.1 and Lua 5.4 documentation as well as through empirical testing.

@gyfdb
Copy link

gyfdb commented Dec 17, 2023

rather the number of indices to unpack (default to the total number of indices)

I can see how the misunderstanding with the unpack function can be seen here. While yes, parameter j is defaulted to total number of elements in a table, it's defaulted to that because you're grabbing from start of i and ending at j. j is wherever you want unpacking to stop. table.unpack/unpack is basically like javascript's slice prototype function.

https://github.com/luau-lang/luau/blob/e76802f2ce698ca090a793b24c07e336b21ade9f/VM/src/ltablib.cpp#L269C18-L269C18
Dosen't explain much but this is the pushed global cfunction of unpack.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes requested This pull request has changes requested prior to merging engine reference Changes the Engine API Reference documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants