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

[FEAT] Remove Instance Overhead #32

Closed
jackdotink opened this issue Dec 29, 2023 · 2 comments · Fixed by #33
Closed

[FEAT] Remove Instance Overhead #32

jackdotink opened this issue Dec 29, 2023 · 2 comments · Fixed by #33
Assignees
Labels
enhancement New feature or request

Comments

@jackdotink
Copy link
Member

jackdotink commented Dec 29, 2023

Describe your feature

Currently instances are placed into a single large array, and then the index of the instance in that array is written to the buffer. This wastes the two bytes written to the buffer. As Zap serializes and deserializes in the same order, instances will be written to the array in the same order.

A type like this: Instance(BasePart) is currently outputted as the following:

function types.write_basePart(value: basePart)
	local pos = alloc(2)
	buffer.writeu16(outgoing_buff, pos, alloc_inst(value))
end
function types.read_basePart()
	local value;
	value = incoming_inst[buffer.readu16(incoming_buff, read(2))]
	assert(value ~= nil)
	return value
end

With this change implemented this will be changed to this:

function types.write_basePart(value: basePart)
	table.insert(outgoing_inst, value)
end
function types.read_basePart()
	local value;
	value = table.remove(incoming_inst)
	assert(value ~= nil)
	return value
end

Again, this is possible because we serialize and deserialize in the same order - so instances will be written and read from the inst buffer in the same order.

Alternatives

I have not considered any alternatives at this moment.

Implementation Details

This could cause issues if we ever have non-deterministic serialization or deserialization order in the future.

Additional context

N/A

@jackdotink jackdotink added the enhancement New feature or request label Dec 29, 2023
@jackdotink jackdotink self-assigned this Dec 29, 2023
@jackdotink
Copy link
Member Author

After testing it appears that using table.remove will not work, as it skips over nil indexes. Instead we will need to restructure the way we send instances and the way we read them.

Before sending the instance array, we will add a non-nil value to the end of the array to ensure no elements are stripped off the end. We can use the value of true as it only uses two bytes. When we receive an instance array we will get the length - 1, as that is the length of our instances. We will store the current index value in the table, and every time an instance is read this will decrement by one.

Once again, I wish Roblox gave us Instance Ids, this is a nightmare.

@jackdotink
Copy link
Member Author

Small correction, we don't need to add a non-nil value or decrement the index. Instead we increment the index.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant