[in-game] Save file format issues

I've been using serialized tables for a long time as a file format, but have gone back to the idea of binary formats. Thanks to theoriginalbit's help and a thing he posted I missed, I was able to get it somewhat functional, but the save does not resemble the map.
Here's the pastebin:
http://pastebin.com/hgucfn0K
I know this code is shitty, it was intended for a 1-use thing, then I decided to make game-engine, and didn't want to start all over until I'd gotten some things working. I want to get this save format working and fix bugs with painting before copying & pasting them and starting the rest from scratch. I've made it pretty obvious where the troublesome code is
If there's any crashes or inefficiencies not related to nsave or nload, please don't tell me. I guarantee you I already know about them, and I'm ditching the rest of this anyway.
The issues are currently in the nsave function.
By the way, I know for an absolute fact it's not the rest of the level editor that's the issue. Just the 2 functions.

I suggest you read this http://www.catb.org/esr/writings/taoup/html/textualitychapter.html
I sincerely suggest you not implement a binary format. You'd be better off with a textual format.

in the end, it really is just text. Or can be losslessly expressed as text, anyhow.
honestly, I'm too tired to read that right now, but it seems to be for Unix. Computercraft is nowhere near that level.
Updated the pastebin with a bug fix… it works better but is still broken
edit: I've figured it out, nevermind this for now, will post the bugs for my solution when finished

Your "license" is confusing and contradicts itself. I suggest either using a proper one, or getting rid of it. :P/>

A thing that's been confusing me for a long time is this: given that functions should either serve to speed up your code, reduce it, or make it more readable, why do people insist on declaring bunches of the things in situations where they'll do none of the above?

For example, take this:

packByte(colors.white,true,true,true,true)

That results in something like 13 calls. You could've just used the number 240, which is what it'll resolve to every single time. And you're looping it. Argh.

And why don't people use more line breaks in their code! If I omitted the use of paragraphs in this post, people would complain about it being an unreadable wall of text. Then these same people turn around and code that way! Blagarg!

And while I'm complaining about random stuff, why do people prefer the later versions of Windows? The accessories have turned to rubbish. For example, Calculator - want access to binary/decimal conversions? Say goodbye to fractional support! You can't have both at once anymore. Why? Who knows!

And Paint! Paint! They ruined it! And don't get me started on Explorer! Aaaargh!

But I digress.

While there's a lot of "strange" logic in there, the only "incorrect" bit I could spot was in your loading function, when starting a new row. You reset "col" to 1, forgetting that you increased it by a further 1 on every iteration - meaning it should've been reset to 0 instead.

Although I've not tested them, here are re-writes of the two main functions in concern. They should at the least execute much faster:

Spoiler

local function nsave(filename, map)
	local file, lastY = fs.open(filename, "wb"), 0

	for ycount, row in pairs(map) do
		for i = lastY + 1, ycount do file.write(240) end  -- Let's not recompute static values.
		lastY = ycount
		local lastX = 0
		
		for xcount, col in pairs(row) do
			for i = lastX + 2, xcount do file.write(224) end
			lastX = xcount

			local bitPos = 0
			while bit.blshift(1, bitPos) < col.c do bitPos = bitPos + 1 end  -- Likely executes faster than solving then dividing by logarithms.
			
			file.write((col.s and 128 or 0) + (col.e and 64 or 0) + (col.w and 32 or 0) + (col.p and 16 or 0) + bitPos)  -- Absolutely executes faster than a dozen linked function calls.
		end
		
		-- "sleep" always pauses at least a 20th of a second. So instead:
		os.queueEvent("moo")
		os.pullEvent("moo")
	end
	
	file.close()
end

local function nload(filename)
	local map, file, row, col = {}, fs.open(filename, "rb"), 0

	while true do
		local byte = file.read()
		
		if not byte then
			break
		elseif byte == 240 then  -- white, true on all, which is a newline thingy
			row = row + 1
			col = 0
			os.queueEvent("moo")
			os.pullEvent("moo")
		elseif byte ~= 224 then
			map[row] = map[row] or {}
			map[row][col] = {["c"] = bit.blshift(1, bit.band(byte, 15)),
				["s"] = bit.band(byte, 128) == 128,
				["e"] = bit.band(byte, 64) == 64,
				["w"] = bit.band(byte, 32) == 32,
				["p"] = bit.band(byte, 16) == 16}
		end

		col = col + 1
	end
	
	currentMap = map  -- ???
	file.close()
	
	return map
end

Sora the Hedgehog, on 17 January 2015 - 04:10 AM said:

I sincerely suggest you not implement a binary format. You'd be better off with a textual format.

Erm… why?

From where I'm sitting, using textutils to serialise a table of data is simpler… but horribly inefficient, storage-wise.

thanks, I'll use some of that. I've actually changed the saving function a lot, considering that pairs() does not go in any particular order. It now builds an ordered table that can be used in a for i=1,#x loop.
Btw, keep in mind I will sacrifice saving time for quick loading. These maps will only need to be exported in this format before being released & uploaded, otherwise they'll be a serialized table.
–
updated the PB using that meathod. I've some some testing by actually drawing what it sees to the screen, and found the issue is the spacing functions. With the Y, a Yspace is needed every time, however, when an X is drawn, it automatically goes to the next space, meaning 1 less is needed. Since for i=1,1 will run 1 time, these are also both minused by 1.
The issue is, it's totally messed up. I don't even know what it's doing.
edit: it was because the y tables where not orginized.
Alright, I've updated the pastebin with a weird bug: the all table resets all table values called ordered to {1}.

Instead of posting useless bump messages, perhaps expound on exactly what you meant with the last line of your previous actually useful post.

well, after the X loop that packs one of the game map's y tables's x values into a sequential table in a lowest to greatest fashion, somewhere else that data is reset to a table with a the date: 1 = 1. I have proven it works in the first place by outputting the X table into Game-engine(currently unreleased)'s console system.
Somewhere in the Y table it's getting reset, but I do not know where.

Exactly what is the point of the "all" table? Are you trying to turn a table full of gaps into a table without gaps, or… what's the go?

Does it in any way help to know that table.maxn(tableName) returns the highest existing numeric index in a given table?

Okay, if you run pairs() on a table, it will not go through it in any particular order; it will might go from 1 to 9 to 6 to 2. I fix this by creating a list that can be used in a for i=1,#table loop, and this list is in order from least to greatest
The issue lies in the fact that every time the code in the Y loop runs, it seems to set every other value in all to {doesn'tchangethis,{1}}

Last I checked, pairs() does get the numeric indexes in order.

But, if that's not the case, you could always do this:

for i=1,table.maxn(myTable) do if myTable[i] then
  -- do whatever
end end