garrysmod icon indicating copy to clipboard operation
garrysmod copied to clipboard

Add table.Pack

Open Kefta opened this issue 6 years ago • 4 comments

Since https://github.com/Facepunch/garrysmod-requests/issues/585 isn't going to be done anytime soon, this is the 5.1 alternative. This packs varargs in a table and returns the proper length including nils, which is unreliable with the # operator.

Kefta avatar Apr 05 '19 22:04 Kefta

Is there any reason not to implement it to work identically to Lua 5.3's table.pack and to call it table.pack?

sohpeach avatar Apr 05 '19 22:04 sohpeach

I find it cleaner to have the length discreetly returned instead of it being a table member so that anyone using table.Pack in place of {...} won't run into issues using code similar to that on the vararg wiki page. Using a pairs loop will silently include the "n" key if it were included in the table. I named it table.Pack for this exact reason since it behaves differently than 5.3's table.pack.

I personally see 5.3's pack inferior to this design but I'd understand if conforming to that is the only way to see this merged. Let me know.

Kefta avatar Apr 05 '19 23:04 Kefta

well, others seem to do it mostly the same as you, just moving it directly into the table: compat53

I'm not exactly happy with how this is handled as well, if the len operator would simply be fixed to grab the amount of entries instead of relying on sequence order this wouldn't be a issue, or if it would at least read from that n member if present in a table, sort of a magic member.

But on it's own it's not solving anything, which is why there's an experiment on 5.4 work version going which marks nil as a normal value in a table and you explicitly need to call unset( table[] ) to remove a entry, of course this in itself has caused some controversity. For now this is not expected to land in the actual 5.4 release as it's breaking too much stuff.

So there's really no real elegant solution present currently.

neico avatar Apr 05 '19 23:04 neico

I agree that the length operator having undefined behaviour for tables with holes is a glaring fault in Lua design. Very interesting to hear they're even considering adding an unset method in 5.4 - I would think adding some kind of explicit hash or array structures would be their first solution.

On table.Pack, I think the way I implemented it is as intuitive as you're going to get in 5.1 other than outright packing the args:

function f(...)
	local tbl, count = table.Pack(...)
	
	for i = 1, count do
		...
	end
end

Cleaner and more consistent with the current Lua API than using tbl.n in place of count imo.

Kefta avatar Apr 06 '19 15:04 Kefta