Jump to content
  • Sky
  • Blueberry
  • Slate
  • Blackcurrant
  • Watermelon
  • Strawberry
  • Orange
  • Banana
  • Apple
  • Emerald
  • Chocolate
  • Charcoal
  • 0
Brodur

What can I do better?

Question

Asking honestly, what can I do better in this program? I am not overly familiar with lua and would love some pointers on how I could make this program better.

Also, pointers on getting event listening working would be great!

https://pastebin.com/rChVHKss     https://github.com/Brodur/Turret-Control

 
--- Turret Control
-- Helps manage OpenModularTurrets turret blocks that have serial IO
-- @Author: Brodur
-- @Version: 2.2
-- @Requires:
-- https://pastebin.com/WvF00A71 : JSON Utility functions
-- https://pastebin.com/pLpe4zPb : Menu functions
-- https://pastebin.com/SKwM2UZH : Lua JSON Library
 
local term = require("term")
local component = require("component")
local event = require("event")
local json = require("json")
local menu = require("menu")
local jsonutil = require("jsonutil")
local fs = require("filesystem")
 
local m = {}
local tb = "turret_base"
local lb = "-------------------------------"
 
local mmopts = {
"Summary",
"Targeting",
"User Admin",
"Push Config",
"Exit"
}
 
local db = {}
local toRemove = {}
local jsondir = "/home/settings.json"
 
--- Trims strings
-- Gets rid of trailing white space or special characters.
-- @param s The string to trim.
-- @return The trimmed string.
function trim(s)
return (s:gsub("^%s*(.-)%s*$", "%1"))
end
 
--- Summary output
-- Outputs the summary of a selected turret.
-- @param turrets A table containing the hardware addresses of turrets.
function m.summary()
term.setCursor(1,1)
print(mmopts[1] .. "\n")
print("Trusted players")
print(lb)
 
for k,v in pairs(db.users) do
print(" - " .. k, v and "[Admin]" or "[Trusted]")
end
print("\nAttacks")
print(lb)
print("Mobs:\t\t" .. tostring(db.targets.setAttacksMobs))
print("Neutrals:\t" .. tostring(db.targets.setAttacksNeutrals))
print("Players:\t" .. tostring(db.targets.setAttacksPlayers))
 
print("\n\nPress enter to continue...")
term.read()
end
 
--- Set Attack
-- Sets the attack setting for all turrets.
-- @param turrets A table containing the hardware addresses of turrets.
-- @param flag  1 or 2, corresponds to true or false, toggles the attack mode.
-- @param option  Which option was chosen in the target function
function m.setAttack(flag, option)
local methods = {"setAttacksMobs", "setAttacksNeutrals", "setAttacksPlayers"}
local arguments = {true, false}
db.targets[methods[option]] = arguments[flag]
db.hasChanges = true
end
 
---Target
-- Selects the targeting parameter
-- @param turrets A table containing the hardware addresses of turrets.
function m.target()
local options = {"Attack Mobs", "Attack Neutrals", "Attack Players", "Exit"}
local opt = -1
while opt ~= #options do
opt = menu.list(options, mmopts[2])
if opt ~= #options then
local flag = menu.dialog(options[opt], "True", "False")
m.setAttack(flag, opt)
else break end
end
end
 
--- Add Trusted Player
-- Adds a trusted player to all turrets, either with or without admin privileges.
-- @param player  The player to add.
-- @param usrType 1 for trusted, 2 for admin.
function m.addTrustedPlayer(player, usrType)
local args = {false, true}
for k,_ in pairs(db.users) do
if player == k then error("Cannot add a user that already exists!") end
end
db.users[player] = args[usrType]
db.hasChanges = true
end
 
--- Remove Trusted Player
-- Removes a user from the trusted players list.
-- @param player The user to remove.
function m.removeTrustedPlayer(player)
toRemove[#toRemove+1] = player
db.users[player] = nil
db.hasChanges = true
end
 
--- Users
-- Launches the add or remove user dialog
function m.users()
local options = {"Add a trusted user", "Remove a trused user","Exit"}
local opt = -1
while opt ~= #options do
opt = menu.list(options, mmopts[3])
if opt == 1 then
local userTypes = {"Trusted", "Admin"}
term.write("Add a trusted player: ")
local player = trim(term.read())
local usrType = menu.dialog("User or Admin?", "User", "Admin")
local opt = menu.dialog("Add \'" .. player .. "\' as " .. userTypes[usrType] .." type user?", "Confirm", "Cancel")
if opt == 1 then
m.addTrustedPlayer(player, usrType)
end
end
if opt == 2 then
local users = {"Cancel"}
for k,v in pairs(db.users) do users[#users+1] = k end
local user = -1
while user ~= 1 do
user = menu.list(users, "Select a user")
if user ~= 1 then
local player = users[user]
local confirm = menu.dialog("Remove \'" .. player .. "\' from trusted users?", "Confirm", "Cancel")
if confirm == 1 then
m.removeTrustedPlayer(player)
table.remove(users, user)
end
end
end
end
end
end
 
--- Distribute Json
-- Disseminates the settings from the database to all turrets.
function m.distribJson()
for _,v in pairs(db.turrets) do
for _,player in pairs(toRemove) do
component.invoke(v, "removeTrustedPlayer", player)
end
toRemove= {}
 
  for user,priv in pairs(db.users) do
component.invoke(v, "addTrustedPlayer", user, priv)
  end
for meth,bool in pairs(db.targets) do
component.invoke(v, meth, bool)
end
end
end
 
--- Sub menu
-- Determines which menu function to call.
-- @param index The selected index in the main menu options table.
function m.subMenu(index)
local turrets = {}
for k in pairs(component.list(tb)) do turrets[#turrets+1] = k end
db.turrets = turrets
 
if index == mmopts[1] then m.summary() end
if index == mmopts[2] then m.target() end
if index == mmopts[3] then m.users() end
if index == mmopts[4] then m.distribJson() end
end
 
--- On Load
-- Initializes the database and grabs the initial turrets.
function m.onLoad()
if not fs.exists(jsondir) then
db = {
    hasChanges=false,
    targets={setAttacksMobs=false,
         setAttacksNeutrals=false,
         setAttacksPlayers=false
        },
    turrets = {},
    users = {}
  }
  jsonutil.save(db, jsondir)
end
          
db = jsonutil.load(jsondir)
for k in pairs(component.list(tb)) do db.turrets[#db.turrets+1] = k end
m.main()
end
 
--- Main
-- Launches the main logic and displays the top level menu.
function m.main()
local mmoptTitle = "Main Menu"
local mmopt = -1
 
while mmopt ~= #mmopts do
if db.hasChanges then
jsonutil.save(db, jsondir)
db.hasChanges = false
m.distribJson()
end
if mmopt ~= #mmopts then
mmopt = menu.list(mmopts, mmoptTitle)
m.subMenu(mmopts[mmopt])
end
end
end
 
--- On Component Added
-- Push the stored config when a new turret is detected.
-- @param eventType The type of event, passed by event listener.
-- @param address The hardware address of the new component.
-- @param componentType   The type of the new component.
-- @todo figure out why the event listener is not firing all of the time, until that is figured disregard this function
-- function onComponentAdded(eventType, address, componentType)
-- if componentType == "turret_base" then
-- distribJson()
-- end
-- end
 
-- event.listen("component_added", onComponentAdded)
m.onLoad()
return m
 

Share this post


Link to post
Share on other sites

4 answers to this question

Recommended Posts

  • 1

Overall the program looks pretty solid. Personally I like to write things with more descriptive names for when I have to revisit programs after a while. One place where things become unclear is using 'k' or 'v' in your pairs loops. If the keys and values are really arbitrary then this makes sense however several places in your code could be clarified by using better variable names. Here's an example..

--- Distribute Json
-- Disseminates the settings from the database to all turrets.
function m.distribJson()
  -- # also db.turrets is an array so maybe ipairs would help a reader infer that. _ lets me see the index/key is irrelevant in this loop but perhaps some loop abstraction would be nice?
  for _,v in pairs(db.turrets) do -- # what exactly is v? maybe this chould be compId or turret_id?
    for _,player in pairs(toRemove) do
      component.invoke(v, "removeTrustedPlayer", player) -- # seems v is a uuid(component address)?
    end
 
    for user,priv in pairs(db.users) do -- # like this! this is more clear
      component.invoke(v, "addTrustedPlayer", user, priv)
    end
    for meth,bool in pairs(db.targets) do -- # bool could be more clear. perhaps should_engage or something that describes the variable purpose
      component.invoke(v, meth, bool)
    end
  end
  toRemove= {}
end
local privs = {
  [1] = true, -- # equal to admin
  [2] = false, -- # generic trusted priv
  admin = true, -- # named privs are nice to have
  trusted = false --# maybe 'user' is too generic? perhaps 'basic' or 'trusted'?
}

function m.addTrustedUser(user, level)
  if db.users[user] then error "Cannot add an existing user!" end
  db.users[user] = level
  db.hasChanges = true
end

m.addTrustedUser('molinko', privs[2]) -- # or privs.trusted this is nice and readable.
m.addTrustedUser('Brodur', privs.admin) -- #  I dont have to look at the privs table to see whats happening...

Overall your program looks fine. Personally I would just clarify(imply) whats happening a bit more with your variable names and change some to do the same.

Nice program though :)

Share this post


Link to post
Share on other sites
  • 1

Yeah it would.. I missed that. Perhaps a nil check would be better.

function m.addTrustedUser(user, level)
  if db.users[user] ~= nil then error "Cannot add an existing user!" end
  db.users[user] = level
  db.hasChanges = true
end

 

Share this post


Link to post
Share on other sites
  • 0
1 hour ago, Molinko said:

if db.users[user] then error "Cannot add an existing user!" end

I agree with all of your points, thanks! 

One question, in the above quote would this not be undesirable? As I store my users in a table where they are the key and a boolean is the value. So if the user was false, aka trusted but not admin, would the above code not function properly?

 
{
"hasChanges": false,
"users": {
"Brodur": true,
"crassclown": true,
"fatso12321": true,
"Ouhai_Ruby": true,
"Kyle8910": false,
"Sacreddeathflame": false,
"Emfitty": false
},
"targets": {
  "setAttacksMobs": false,
"setAttacksNeutrals": false,
  "setAttacksPlayers": false
},
"turrets": [
]
}

Share this post


Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Answer this question...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Loading...

×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use and Privacy Policy.