This is a read-only snapshot of the ComputerCraft forums, taken in April 2020.
Himself12794's profile picture

Help with Clean-up of code for a little turtle OS with building templates

Started by Himself12794, 27 January 2014 - 04:22 PM
Himself12794 #1
Posted 27 January 2014 - 05:22 PM
I'm currently working on a turtle OS that will have some useful programs. So far, what I have works, but I can't help but think that the code could be shorter and a little less complicated. Any tips on simplification is appreciated. There's three codes so far that I have.

The startup code, which displays all the programs in a directory as multiple choice options, with configurable visibility:
Spoiler

choice=fs.list('')
newTable=fs.list('')
hidden={'startup','rom','KeyNumb'}
function hideValues(array,hidden)
for i,v in ipairs(array) do
  for i2,v2 in ipairs(hidden) do
   if v==v2 then
	table.remove(array, i)
	return hideValues(array,hidden)
   end
  end
end
end
function lineClear()
x,y=term.getCursorPos()
term.clearLine()
term.setCursorPos(1,y)
end
function printCentered(str)
x,y=term.getSize()
x2,y2=term.getCursorPos()
newX=math.ceil(x/2)-math.ceil(string.len(str)/2)
term.setCursorPos(newX,y2)
print(str)
end
function replaceDelim(str, delim)
result=''
for i in string.gmatch(str, '%a+') do
  result=result..i..delim
end
return result
end
function spacifyTable(array)
for i,v in ipairs(array) do
  array[i]=replaceDelim(v,' ')
end
end
function getArraySize(array)
count=0
for _ in ipairs(array) do
  count=count+1
end
return count
end
function multipleChoice(choices, initial)
size=getArraySize(choices)
if not initial then
  sel=1
else
  sel=initial
end
while true do
  header()
  for i,v in ipairs(choices) do
   if i==sel then
	printCentered('[ '..v..']')
   else
	printCentered('  '..v..' ')
   end
  end
  event,key=os.pullEvent('key')
  if (key==208 or key==31) and sel<=size then sel=sel+1 end
  if (key==208 or key==31) and sel==size+1 then sel=1 end
  if (key==200 or key==17) and sel>0 then sel=sel-1 end
  if (key==200 or key==17) and sel==0 then sel=size end
  if key==28 or key==57 then break end
  sleep(0.01)
end
return sel
end
function header()
term.clear()
term.setCursorPos(1,1)
print("HimCo Industries' Turtley 2.0")
print()
printCentered('Select a template:')
print()
end
hideValues(choice,hidden)
spacifyTable(choice)
file=multipleChoice(choice)
hideValues(newTable,hidden)
shell.run(newTable[file])

Then the tower code, which just builds a square tower with configurable width and height:
Spoiler

--args={...}
--local width=tonumber(args[1])
--local height=tonumber(args[2])
term.clear()
term.setCursorPos(1,1)
function lineClear()
local x,y=term.getCursorPos()
term.setCursorPos(1,y)
term.clearLine()
end
function dichotomy(a,B)/>/>/>
sel=true
while true do
  if sel then
   write("--> "..a.."	 "..B)/>/>/>
  else
   write("	"..a.." --> "..B)/>/>/>
  end
  event,key=os.pullEvent()
  if event=="key" and key==203 then sel=not sel end
  if event=="key" and key==205 then sel=not sel end
  if event=="key" and key==28 then break end
  sleep(0.01)
  lineClear()
end
print()
return sel
end
function numbersOnly(pre)
local x,y=term.getCursorPos()
if pre then write(pre) end
local num=read()
if not tonumber(num) then
  term.setCursorPos(x,y)
  term.clearLine()
  write('Please insert a number only')
  sleep(1.5)
  term.setCursorPos(x,y)
  term.clearLine()
  return numbersOnly(pre)
end
return tonumber(num)
end
function returnToBase(height)
turtle.turnRight()
turtle.turnRight()
turtle.forward()
for i=1, height+1 do
  turtle.down()
end
end
function getDimensions()
term.clear()
term.setCursorPos(1,1)
print('Give the values for the dimensions of the tower.')
print()
width=numbersOnly('Please give a value for the width: ')
height=numbersOnly('Please give a value for the height: ')
if width<0 or height<0 or height>256 then
  print()
  print('Check your numbers. The dimensions must be greater than 0, and the height cannot be greater than world heigh. (256)')
  sleep(2)
  return getDimensions()
end
return width,height
end
function tower(width, height)
for i1=1, height do
  turtle.up()
  for i2=1, 4 do
   for i3=1, width-1 do
	turtle.forward()
	if not selectResource() then
	 print()
	 print('There are no available resources.')
	 print('Please add more to continue.')
	 while not selectResource() do
	  sleep(3)
	 end
	 term.clear()
	 term.setCursorPos(1,1)
	 print('Construction will continue')
	end
	turtle.placeDown()
   end
   turtle.turnRight()
  end
end
print('Build Complete')
if toBase then
  returnToBase(height)
end
end
function calculateBlocks(width, height)
local shell=(width^2*height)-((width-2)^2*height)
local stacks=(shell-(shell%64))/64
local extra=shell%64

term.clear()
term.setCursorPos(1,1)
print('Would you like the turtle to return to the tower base when finished?')
if dichotomy('Yes','No') then
  toBase=true
else toBase=false
end
term.clear()
term.setCursorPos(1,1)

print('You will need 64x'..tostring(stacks)..'+'..tostring(extra)..' blocks for this tower.')
print()
print('When the turtle has depleted one slot, it will select the next available resource regardless of the type.')
print()
print('Add the required amount to the turtle, then select continue to proceed')
if dichotomy('Continue','Quit') then
  print()
  print('Construction will begin')
  sleep(1.5)
  term.clear()
  term.setCursorPos(1,1)
else
  os.reboot()
end

if countResources()<shell then
  term.clear()
  term.setCursorPos(1,1)
  print('You still do not have enough resources to complete this build.')
  print()
  print('If the turtle runs out, it will stall until more are added.')
  print()
  print('Continue anyway?')
  if dichotomy('Continue','Quit') then
   print('Construction will begin')
   sleep(1.5)
   term.clear()
   term.setCursorPos(1,1)
  else
   os.reboot()
  end
end
return shell
end
function countResources()
local count=0
for i4=1, 16 do
  count=count+turtle.getItemCount(i4)
end
return count
end
function selectResource()
for i5=1, 16 do
  if turtle.getItemCount(i5)>0 then
   turtle.select(i5)
   return true
  end
end
return false
end
width,height=getDimensions()
calculateBlocks(width,height)
tower(width,height)
os.reboot()

And finally a program that will build a gregtech industrial grinder, and dig out the area for it if requested:
Spoiler

diggingMode=false
function lineClear()
x,y=term.getCursorPos()
term.clearLine()
term.setCursorPos(1,y)
end
function dichotomy(a,B)/>/>/>
sel=true
while true do
  if sel then
   write("--> "..a.."	 "..B)/>/>/>
  else
   write("	"..a.." --> "..B)/>/>/>
  end
  event,key=os.pullEvent()
  if event=="key" and key==203 then sel=not sel end
  if event=="key" and key==205 then sel=not sel end
  if event=="key" and key==28 then break end
  sleep(0.01)
  lineClear()
end
print()
return sel
end
function enableDiggingMode()
print('Do you wish to enable digging mode?')
print('If you do so, the turtle will clear a 3x3x3 area for the casing.')
print('Turtle with digging tool required. (Obviously)')
print()
print('Enable digging mode?')
if dichotomy('Yes','No') then
  diggingMode=true
else
  diggingMode=false
end
end
function placeSlot(side,slot)
if turtle.getItemCount(slot)==0 then
  print('Insufficient resources to complete build. Please place an item in slot '..tostring(slot))
  while turtle.getItemCount(slot)==0 do
   sleep(0.01)
  end
end
turtle.select(slot)
placeSide(side)
end
function placeSide(side)
if not side then
  turtle.place()
elseif side=='up' then
  turtle.placeUp()
elseif side=='down' then
  turtle.placeDown()
elseif side=='left' then
  turtle.turnLeft()
  turtle.place()
  turtle.turnRight()
elseif side=='right' then
  turtle.turnRight()
  turtle.place()
  turtle.turnLeft()
elseif side=='front' then
  turtle.place()
elseif side=='back' then
  turtle.turnRight()
  turtle.turnRight()
  turtle.place()
  turtle.turnRight()
  turtle.turnRight()
end
end
function caveLayers(final)
layer1Reverse=false
for i=1, 3 do
  turtle.dig()
  turtle.forward()
  turtle.digUp()
  turtle.dig()
  turtle.forward()
  turtle.digUp()
  if not layer1Reverse and i~=3 then
   turtle.turnRight()
   turtle.dig()
   turtle.forward()
   turtle.digUp()
   turtle.turnRight()
  elseif layer1Reverse and i~=3 then
   turtle.turnLeft()
   turtle.dig()
   turtle.forward()
   turtle.digUp()
   turtle.turnLeft()
  end
  layer1Reverse=not layer1Reverse
end
if not final then
  turtle.digUp()
  turtle.up()
  turtle.turnRight()
  turtle.turnRight()
  turtle.digUp()
else
  turtle.turnRight()
  turtle.turnRight()
end
end

function layer1()
if not diggingMode then
  turtle.turnLeft()
  turtle.forward()
  turtle.turnRight()
  turtle.up()
  turtle.forward()
end
placeSlot('down',1)
for i2=1, 4 do
  for i3=1, 2 do
   turtle.forward()
   placeSlot('down',1)
  end
  turtle.turnRight()
end
turtle.forward()
turtle.turnRight()
turtle.forward()
placeSlot('down',1)
turtle.forward()
turtle.turnLeft()
turtle.forward()
turtle.turnRight()
turtle.turnRight()
end
function layer2()
turtle.up()
for i2=1, 4 do
  for i3=1, 2 do
   turtle.forward()
   placeSlot('down',2)
  end
  turtle.turnRight()
end
turtle.forward()
turtle.turnRight()
turtle.forward()
placeSlot('down',3)
turtle.forward()
turtle.turnLeft()
turtle.forward()
turtle.turnRight()
turtle.turnRight()
end
function layer3()
turtleInvert=false
turtle.forward()
for i2=1, 3 do
  placeSlot('front',1)
  for i=1, 2 do
   if not turtleInvert then turtle.turnRight() else turtle.turnLeft() end
   turtle.forward()
   if not turtleInvert then turtle.turnLeft() else turtle.turnRight() end
   placeSlot('front',1)
  end
  turtleInvert=not turtleInvert
  if i2~=3 then
   turtle.turnLeft()
   turtle.turnLeft()
   turtle.forward()
   turtle.turnRight()
   turtle.turnRight()
  else
   turtle.turnLeft()
   turtle.forward()
   turtle.turnLeft()
   turtle.forward()
   turtle.down()
   placeSlot('back',4)
   turtle.down()
   turtle.turnRight()
   turtle.turnRight()
   turtle.forward()
   turtle.turnRight()
   turtle.turnRight()
  end
end
end
function info()
term.clear()
term.setCursorPos(1,1)
enableDiggingMode()
term.clear()
term.setCursorPos(1,1)

print('Please place 18 standard machine casings in slot 1, 8 reinforced casings in slot 2, water in slot 3, and an industrial grinder in slot 4.')
print()
print('Add the required materials to the turtle, then select continue to proceed')
if dichotomy('Continue','Quit') then
  print()
  print('Construction will begin')
  sleep(1.5)
  term.clear()
  term.setCursorPos(1,1)
else
  os.reboot()
end

if countResources()<28 then
  term.clear()
  term.setCursorPos(1,1)
  print('There is a discrepency in the number of required materials.')
  print()
  print('If the turtle runs out, it will stall until more are added.')
  print()
  print('Continue anyway?')
  if dichotomy('Continue','Quit') then
   print('Construction will begin')
   sleep(1.5)
   term.clear()
   term.setCursorPos(1,1)
  else
   os.reboot()
  end
end
end
function countResources()
local count=0
for i4=1, 16 do
  count=count+turtle.getItemCount(i4)
end
return count
end
info()
if diggingMode then
turtle.turnLeft()
turtle.forward()
turtle.turnRight()
turtle.dig()
turtle.forward()
turtle.digUp()
caveLayers()
caveLayers(true)
end
layer1()
layer2()
layer3()
os.reboot()

I'm relatively new to coding, so I haven't got the ability for aesthetically pleasing code yet. Any suggestions or tips are greatly appreciated!
CometWolf #2
Posted 27 January 2014 - 05:46 PM
Some indentation and simple comments go a long way when it comes to making nice code. That is, simple as in only explain the more complex/weird/strange things you do. I usually structure my code into sections, one for variables/tables and the like, another for functions and finally one for the actual program. These sections should ofcourse be commented to mark what is what in a clean and nice way.

Have a look at my currently largest program to see what i mean
http://pastebin.com/Kt4D8uyE
My cleanliness does suffer a bit as it goes on though, lol.

As for the actual function of your code, i'll take a look tomorrow, since im going to bed now :P/>
Himself12794 #3
Posted 27 January 2014 - 06:18 PM
Some indentation and simple comments go a long way when it comes to making nice code. That is, simple as in only explain the more complex/weird/strange things you do. I usually structure my code into sections, one for variables/tables and the like, another for functions and finally one for the actual program. These sections should ofcourse be commented to mark what is what in a clean and nice way.

Have a look at my currently largest program to see what i mean
http://pastebin.com/Kt4D8uyE
My cleanliness does suffer a bit as it goes on though, lol.

As for the actual function of your code, i'll take a look tomorrow, since im going to bed now :P/>
Well I am nowhere near that level of expertise, but that looks magnificent. All I know about coding has come from doing computercraft.
CometWolf #4
Posted 28 January 2014 - 10:47 AM
I gave your first program a quick once over. afew things to note is to always use local variables unless you have a good reason not to. It's also good practice to start variable names with lower case of the first letter of the type (t for table, b for boolean, n for number, s for string). Personally i just do this for tables though :P/>. While we're on the subject of variables, if you're getting multiple variables from a function which you don't intend to use, start their name with an underscore.

A neat trick to keep in mind when it comes to function arguments is that when you're checking the variable for nil(not used) you can do the following

local function test(variable)
  variable = variable or derp
  print(variable)
end
This will print the variable if it is given, otherwise it will print derp

Another thing to note is how you use your if statements. If you're checking the same condition multiple times, in this case your key presses, you should put the sub conditions within the if statement instead. Also, when you're checking the same variable for different values, use elseif statements.
Finally, you should know that return will end the function anyways, as such the break which then goes to a return is not really nessacary. Neither is sleep in a os.pullEvent loop, as os.pullEvent yields anyways.

--tables and variables
local choice = fs.list('')
local newTable = fs.list('')
local hidden = {
  'startup',
  'rom',
  'KeyNumb'
}
--functions
function hideValues(array,hidden)
  for i,v in ipairs(array) do
    for i2,v2 in ipairs(hidden) do
	  if v==v2 then
		 table.remove(array, i)
		 return hideValues(array,hidden) --this is called a recursive call, and should be avoided where possible. There is probably a better way to do this, but i haven't really gotten into it.
	   end
	  end
    end
  end
end
 
local function lineClear()
  local _x,y=term.getCursorPos()
  term.clearLine()
  term.setCursorPos(1,y)
end
 
local function printCentered(str)
  local termX, _termY=term.getSize()
  local _cursX, cursY=term.getCursorPos()
  local newX = math.ceil(termX/2)-math.ceil(string.len(str)/2)
  term.setCursorPos(newX,cursY)
  print(str)
end
 
local function replaceDelim(str, delim)
  local result = ''
  for i in string.gmatch(str, '%a+') do
    result=result..i..delim
  end
  return result
end
 
local function spacifyTable(array)
  for i,v in ipairs(array) do
    array[i]=replaceDelim(v,' ')
  end
end
 
local function getArraySize(array)
  local count = 0
  for _ in ipairs(array) do
    count=count+1
  end
  return count
end

local function header()
  term.clear()
  term.setCursorPos(1,1)
  print("HimCo Industries' Turtley 2.0")
  print()
  printCentered('Select a template:')
  print()
end
 
local function multipleChoice(choices, sel)
  local size = getArraySize(choices)
  sel = sel or 1 --initial selection
  while true do
    --render screen
    header()
    for i,v in ipairs(choices) do
	  if i == sel then
	    printCentered('[ '..v..']')
	  else
	    printCentered('  '..v..' ')
	  end
    end
    --user input
    local event,key=os.pullEvent('key')
    if key == 208 --down arrow
    or key == 31 then -- S
	  sel = sel+1
	  if sel > size then
	    sel = 1 --botom reached, moving back to top
	  end
    elseif key == 200 -- up arrow
    or key == 17 then -- W
	  sel = sel-1
	  if sel < 1 then
	    sel = size -- top reached, moving to bottom
	  end
    elseif key == 28 -- enter
    or key == 57 then --space
	  return sel --input complete
    end
  end
end
 
--actual program
hideValues(choice,hidden)
spacifyTable(choice)
file=multipleChoice(choice)
hideValues(newTable,hidden)
shell.run(newTable[file])
Himself12794 #5
Posted 28 January 2014 - 01:25 PM
I gave your first program a quick once over. afew things to note is to always use local variables unless you have a good reason not to. It's also good practice to start variable names with lower case of the first letter of the type (t for table, b for boolean, n for number, s for string). Personally i just do this for tables though :P/>. While we're on the subject of variables, if you're getting multiple variables from a function which you don't intend to use, start their name with an underscore.

A neat trick to keep in mind when it comes to function arguments is that when you're checking the variable for nil(not used) you can do the following

local function test(variable)
  variable = variable or derp
  print(variable)
end
This will print the variable if it is given, otherwise it will print derp

Another thing to note is how you use your if statements. If you're checking the same condition multiple times, in this case your key presses, you should put the sub conditions within the if statement instead. Also, when you're checking the same variable for different values, use elseif statements.
Finally, you should know that return will end the function anyways, as such the break which then goes to a return is not really nessacary. Neither is sleep in a os.pullEvent loop, as os.pullEvent yields anyways.
Spoiler

--tables and variables
local choice = fs.list('')
local newTable = fs.list('')
local hidden = {
  'startup',
  'rom',
  'KeyNumb'
}
--functions
function hideValues(array,hidden)
  for i,v in ipairs(array) do
	for i2,v2 in ipairs(hidden) do
	  if v==v2 then
		 table.remove(array, i)
		 return hideValues(array,hidden) --this is called a recursive call, and should be avoided where possible. There is probably a better way to do this, but i haven't really gotten into it.
	   end
	  end
	end
  end
end

local function lineClear()
  local _x,y=term.getCursorPos()
  term.clearLine()
  term.setCursorPos(1,y)
end

local function printCentered(str)
  local termX, _termY=term.getSize()
  local _cursX, cursY=term.getCursorPos()
  local newX = math.ceil(termX/2)-math.ceil(string.len(str)/2)
  term.setCursorPos(newX,cursY)
  print(str)
end

local function replaceDelim(str, delim)
  local result = ''
  for i in string.gmatch(str, '%a+') do
	result=result..i..delim
  end
  return result
end

local function spacifyTable(array)
  for i,v in ipairs(array) do
	array[i]=replaceDelim(v,' ')
  end
end

local function getArraySize(array)
  local count = 0
  for _ in ipairs(array) do
	count=count+1
  end
  return count
end

local function header()
  term.clear()
  term.setCursorPos(1,1)
  print("HimCo Industries' Turtley 2.0")
  print()
  printCentered('Select a template:')
  print()
end

local function multipleChoice(choices, sel)
  local size = getArraySize(choices)
  sel = sel or 1 --initial selection
  while true do
	--render screen
	header()
	for i,v in ipairs(choices) do
	  if i == sel then
		printCentered('[ '..v..']')
	  else
		printCentered('  '..v..' ')
	  end
	end
	--user input
	local event,key=os.pullEvent('key')
	if key == 208 --down arrow
	or key == 31 then -- S
	  sel = sel+1
	  if sel > size then
		sel = 1 --botom reached, moving back to top
	  end
	elseif key == 200 -- up arrow
	or key == 17 then -- W
	  sel = sel-1
	  if sel < 1 then
		sel = size -- top reached, moving to bottom
	  end
	elseif key == 28 -- enter
	or key == 57 then --space
	  return sel --input complete
	end
  end
end

--actual program
hideValues(choice,hidden)
spacifyTable(choice)
file=multipleChoice(choice)
hideValues(newTable,hidden)
shell.run(newTable[file])
For the code:

function hideValues(array,hidden)
  for i,v in ipairs(array) do
	for i2,v2 in ipairs(hidden) do
		  if v==v2 then
				 table.remove(array, i)
				 return hideValues(array,hidden)
		   end
	  end
	end
  end
end
The only way I could get it to work was to call it recursively. Otherwise, if there were two values in a row to be hidden, it would skip the second one and not remove it.
Edited on 28 January 2014 - 12:29 PM
surferpup #6
Posted 28 January 2014 - 01:29 PM
Is there a question here, or are you just re-quoting CometWolf?
CometWolf #7
Posted 28 January 2014 - 02:55 PM
He's responding to the comment here
function hideValues(array,hidden)
for i,v in ipairs(array) do
for i2,v2 in ipairs(hidden) do
if v==v2 then
table.remove(array, i)
return hideValues(array,hidden) –this is called a recursive call, and should be avoided where possible. There is probably a better way to do this, but i haven't really gotten into it.
end
end
end
end
end
The reason it skips one if you don't do it recursively is because you are using table.remove to remove your table indexes. This will remove the index in question, and if the table is numerically indexed it will also move all the following keys down one number. if you remove 4, 5 would now become the new 4. Thus when it checks number 5, it will actually be number 6.
Use this instead

function hideValues(array,hidden)
  for i,v in ipairs(array) do
	for i2,v2 in ipairs(hidden) do
		  if v==v2 then
				 array[i] = nil
		   end
		  end
	end
  end
end
If you wish to learn more, the difference between these 2 methods were recently discussed here:
http://www.computerc...t-from-a-table/
Edited on 28 January 2014 - 01:56 PM
Himself12794 #8
Posted 01 February 2014 - 11:23 AM
He's responding to the comment here
function hideValues(array,hidden)
for i,v in ipairs(array) do
for i2,v2 in ipairs(hidden) do
if v==v2 then
table.remove(array, i)
return hideValues(array,hidden) –this is called a recursive call, and should be avoided where possible. There is probably a better way to do this, but i haven't really gotten into it.
end
end
end
end
end
The reason it skips one if you don't do it recursively is because you are using table.remove to remove your table indexes. This will remove the index in question, and if the table is numerically indexed it will also move all the following keys down one number. if you remove 4, 5 would now become the new 4. Thus when it checks number 5, it will actually be number 6.
Use this instead

function hideValues(array,hidden)
  for i,v in ipairs(array) do
	for i2,v2 in ipairs(hidden) do
		  if v==v2 then
				 array[i] = nil
		   end
		  end
	end
  end
end
If you wish to learn more, the difference between these 2 methods were recently discussed here:
http://www.computerc...t-from-a-table/
Hmmm… when I do that, it seems to behave incorrectly. It refuses to show the Tower Builder option unless there are no files selected to be hidden. I'm not sure if this is due to placement of the file, or an error in my code.
CometWolf #9
Posted 05 February 2014 - 12:13 PM
Had to do some file hiding myself as part of something i made the other day. I found that forgoing the for loop in favor of a while true loop was the best option.

_G.nFs.tHide = {
  "startup",
},

_G.nFs.checkHidden = function(path)
  for k,v in pairs(_G.nFs.tHide) do
	if v == path then
	  return true
	end
  end
  return false
end

_G.nFs.list = function(path)
  local tFiles = _G.nFs.backup.list(path)
  local i = 1
  while true do
	if _G.nFs.checkHidden(tFiles[i]) then
	  table.remove(tFiles,i)
	elseif tFiles[i] == "startup2" then -- this elseif is irrelevant to what you're doing, so just ignore it :P/>/>/>/>
	  tFiles[i] = "startup"
	  i = i+1
	else
	  i = i+1
	end
	if i > #tFiles then
	  break
	end
  end
  return tFiles
end
Edited on 05 February 2014 - 11:15 AM