[solved] Program not breaking loop when needed, a few other questions

EDIT2: All my current issues have been solved. Thanks again!

EDIT: Actual topic changed. Please read below quote regarding topic change. (Not making a new thread unless needed.)

Nhorr, on 04 October 2015 - 03:45 AM said:

I'm really appreciative of all the help, but I guess I'd like to shift this from a "how do I fix this" thread to a "how do I properly and cleanly do what I'm trying to do" thread. I'm gonna re-write the whole code, so how could I properly code something which parts could be dynamic and the entire UI be somewhat modular for simple upkeep (from pastebin code)?

If the use of shell.run and executing of other programs within programs themselves won't do (since it's obviously an improper "glue and popsicle sticks" method, even I realize that), how can I have parts of the UI download the latest updated pastebin code so key parts (like the updater, for example) stay up to date?

——————————————————————————————————————————————————————————————————————————–

I've tried looking at my code for a fix for the newest version of a personal project I'm taking on for a while, and I'm stuck.

The code in question? These are the main culprits:
http://pastebin.com/nH3LzJg1
http://pastebin.com/JUhsDLDd
http://pastebin.com/dM9PFG97

You can see in v0.4u how it's supposed to behave, at least what the user sees.
However, I'm having a few issues with v0.5u under the hood.

Basically, with v0.5u I've tried optimizing the code with global functions (found in 'startup') and local functions, but I'm experiencing 2 major issues (1 reoccurring that I thought I delt with in v0.4u, 1 new).
The reoccurring issue I've had is when the function 'runShell()' gets executed (by function 'rDP1') after having run the 'updater' program and going back to the desktop, the loop does not fully break. I've tried various methods, and the only solid way I've broken the loop is with an error, which obviously isn't clean. I tried changing the 'runDesktop()' function to run on a repeat loop rather than a while loop, stopping when the variable 'desk' is set to false (which happens in 'rDP1()'). This originally worked even though there was a slight delay, but it seems the issue is back, and I can't figure out how to fix it this time.
Things I've noticed about this issue:
-It only happens when another program gets opened, then when the 'desktop' program gets executed again
-When the loop won't break, after any click/button press 'desktop' continues functioning like normal (well, it would, if it weren't for the next issue I've found…)

The second issue I'm having is after any menu is opened and then 'rDE()' gets run to clear the menu and prepare 'runDesktop()' for the next selection from the user, the desktop locks up except for the clock. The only reason why I think this may be happening is the 'parallel.waitForAll' code near the end of the 'desktop' program. Still, no clue as to why this is happening.

I've tried several different things, attempted fixing it with numerous failed attempts, and frankly, I'm stuck.

I hate to ask, but what can I do to fix these two issues, or at least, what's causing them in my v0.5u code? (And would implementing os.pullEventRaw into 'desktop' cause any other issues? Not quite sure since I haven't been able to tell with these issues present.) As a computercraft novice, all help is obviously welcome.

Many thanks in advance.

For your first issue the main thing I notice is the use of parallel.waitForAll, which would cause drawDesktop to continue to run even when runDesktop has finished. I would suggest using waitForAny, which would run only until one of the functions passed to it returns.

I am sorry, but I only had time to read the code for one problem hope you can find the other :D/>

valithor, on 28 September 2015 - 07:43 PM said:

For your first issue the main thing I notice is the use of parallel.waitForAll, which would cause drawDesktop to continue to run even when runDesktop has finished. I would suggest using waitForAny, which would run only until one of the functions passed to it returns.

I am sorry, but I only had time to read the code for one problem hope you can find the other :D/>

Any help is help, nonetheless… I'll rework this into my code tomorrow and see if it fixes that issue… no need to apologize, this may actually fix both issues… :D/>

Changing All to Any didn't fix any issues. In fact, when the updater itself runs it breaks a loop now - I've got to figure that one out later. :/

Valithor anything I should know coming in? I need something to do :)/>

TYKUHN2, on 29 September 2015 - 09:51 PM said:

Valithor anything I should know coming in? I need something to do :)/>

You can take a stab at it. I misread the question the first time I tried to answer, so I fixed a problem he would have had, but not the one he was asking about.

One little thing you could look at would be the difference between the elseif statement on line 78 and 90.

I have a decent understanding of what is going on, but with all of the shell.run calls it would appear there are multiple layers of each program running. This will lead to many headaches eventually, not to mention if someone were to run the program long enough it would crash from having too many functions calling each other in same stack.

edit:

Honestly… I would suggest rewriting the entire thing, without using shell.run in any of it. Everything you are using it for takes either a default CC api call ex: shell.run("delete file") = fs.delete("file") or a slight change in structure of the code. Running your own program from within itself is almost never a good idea.

I'm setting up an enviroment right now. I hope I got file names and such right.

I noticed desktop locking up but I cannot seem to find the updater option so I am not able to test that.

EDIT: I didn't notice forum posts merged.

I agree with Valithor there. One thing I noticed is startup is a small program. You could easily merge it with desktop or another file.

It is kinda hard to read. If I want to fix it I probably have to re-write it in a format I can read, which would take a long period of time. I can only imagine runDesktop is somehow holding runDesktop up.

valithor, on 29 September 2015 - 09:58 PM said:

TYKUHN2, on 29 September 2015 - 09:51 PM said:

Valithor anything I should know coming in? I need something to do :)/>

You can take a stab at it. I misread the question the first time I tried to answer, so I fixed a problem he would have had, but not the one he was asking about.

One little thing you could look at would be the difference between the elseif statement on line 78 and 90.

I have a decent understanding of what is going on, but with all of the shell.run calls it would appear there are multiple layers of each program running. This will lead to many headaches eventually, not to mention if someone were to run the program long enough it would crash from having too many functions calling each other in same stack.

edit:

Honestly… I would suggest rewriting the entire thing, without using shell.run in any of it. Everything you are using it for takes either a default CC api call ex: shell.run("delete file") = fs.delete("file") or a slight change in structure of the code. Running your own program from within itself is almost never a good idea.

What would you recommend I do instead? I tried to make the UI modular so different parts could be updated/etc while the rest remained the same. What might you recommend to fix that issue itself?

TYKUHN2, on 29 September 2015 - 10:10 PM said:

I'm setting up an enviroment right now. I hope I got file names and such right.

I noticed desktop locking up but I cannot seem to find the updater option so I am not able to test that.

EDIT: I didn't notice forum posts merged.

I agree with Valithor there. One thing I noticed is startup is a small program. You could easily merge it with desktop or another file.

(The 'Versions' option under the 'NhUI' tab executes the 'updater' program. Should have clarified that. You can also run the installer for v0.5u [installer for v0.4u, for reference])
Part of the reason I designed there to be a startup file like that one is in case the desktop gets deleted it checks for an issue such as that and fixes it if needed.
It's not fully done, and I see truth in the possibility of merging it, but I have more intentions for that program (think of it being like a system check that executes on startup).

TYKUHN2, on 29 September 2015 - 10:19 PM said:

It is kinda hard to read. If I want to fix it I probably have to re-write it in a format I can read, which would take a long period of time. I can only imagine runDesktop is somehow holding runDesktop up.

So it could be an issue with runDesktop(), then? And I could briefly try explaining the code for you - depending on what you need explained will depend on how easily I can explain it though.

I know I should have used tables for some time - just never fully comprehended how to set them up (again, I'm still new to this, even though I've been working this for about a month).

If I'm looking at a re-write (which will be a pain in the a**, but I'm willing), what should I change from the ground up?
Is there anything I can do to make the UI overall modular and peices of it dynamic without multiple programs, and if so, how could I do that?
And if multiple programs need to be used, is there a way to cleanly work that (say, closing a program in the executed program's code)?

Thanks for all the assistance - just happy to be getting responses about this (and TYKUHN2, your signature is too true).

EDIT: I guess I should clarify the order of operations. Obviously, startup runs first, and checks if a password protection program is detected ('login', downloaded and handled by NhUI by user selection), and if it is, it runs it (once 'login' finishes its job it runs 'desktop'). If 'login' is not detected (no password protection program is downloaded by default), 'desktop' is run. Two additional programs, 'updater' and 'uninstaller' keep to themselves until run by desktop, and .info stores the users password when a password is set, and is non-existent if there is no 'login' program.

EDIT 2: Still can't figure out what's breaking the loop in 'updater'. No luck on figuring out what's preventing 'desktop' from functioning properly, either. Guessing a re-write is imminent either way.

Nhorr, on 30 September 2015 - 05:22 PM said:

I know I should have used tables for some time - just never fully comprehended how to set them up (again, I'm still new to this, even though I've been working this for about a month).

Same way. I took me a while to comprehend tables. They are incredibly useful and I would recommend you learn them.

Nhorr, on 30 September 2015 - 05:22 PM said:

Part of the reason I designed there to be a startup file like that one is in case the desktop gets deleted it checks for an issue such as that and fixes it if needed.

So like my BIOS? I use my BIOS to load my files synchronously to some degree. It sucks though so don't ask for it as an example, it may make you a worse programmer :D/>

Instead of running runDesktop() again have the rDP() (or whatever it is) return and the runDesktop on a loop.

TYKUHN2, on 30 September 2015 - 08:41 PM said:

Same way. I took me a while to comprehend tables. They are incredibly useful and I would recommend you learn them.

I suppose now's as good a time as any since it looks like I may be re-writing the code. Might as well do it right this time around and implement tables.

TYKUHN2, on 30 September 2015 - 08:41 PM said:

So like my BIOS? I use my BIOS to load my files synchronously to some degree. It sucks though so don't ask for it as an example, it may make you a worse programmer :D/>

Hah, can't say if it is or isn't since I haven't ever seen it, but it sounds like it to a very simple degree - I'd give a definite yes or no if I'd have seen it, but apparently that could be dangerous… :P/>

TYKUHN2, on 30 September 2015 - 08:41 PM said:

Instead of running runDesktop() again have the rDP() (or whatever it is) return and the runDesktop on a loop.

So, basically, have runDesktop() as a loop (like it is), but implement another loop for the function that goes to shell that breaks?
Whether or not that's what you meant I may try that… still don't know what's breaking the loop in 'updater' and/or how NhUI could be modular and dynamic without using shell.run/etc. :(/>
Either way I've already faced the fate of having to re-write… just curious as a work around to make things operate roughly as they do but in a much cleaner, better designed fashion.

Thanks again for the help… I realize this really isn't important, but it does mean quite a lot for me - NhUI's been a really fun personal project.

I am also making an OS as a personal project, though my main goal is security, which leads to interesting ideas sometimes.

runDesktop() should be a loop like it is but instead of doing runDesktop() you just end rDP or whatever the function is. That way the original runDesktop and drawDesktop which are still running are used. Prevent clutter. If we solve you problem then after approximately 256(?) times clicking update (which is somewhat unreasonable but can still cause lag at low numbers) the program will crash.

Edit: And if your running an updater don't you want to close the program and reopen it anyways?

TYKUHN2, on 01 October 2015 - 01:02 AM said:

I am also making an OS as a personal project, though my main goal is security, which leads to interesting ideas sometimes.

Neat - I wish you luck! :D/>

TYKUHN2, on 01 October 2015 - 01:02 AM said:

runDesktop() should be a loop like it is but instead of doing runDesktop() you just end rDP or whatever the function is. That way the original runDesktop and drawDesktop which are still running are used. Prevent clutter. If we solve you problem then after approximately 256(?) times clicking update (which is somewhat unreasonable but can still cause lag at low numbers) the program will crash.

Think I got it (won't make any bigger changes till tomorrow since on mobile). I'm guessing you're referring to rDP3() (the one that starts up the updater). I've gone back and added comments to what each rDP#() function does - I should rename the actual functions to be more specific. They originally were appearing twice in the code (once for key press detected, then mouse click, so I just boiled them down to just one function but never gave them a proper name… untidy and a bad idea, I know…).

TYKUHN2, on 01 October 2015 - 01:02 AM said:

Edit: And if your running an updater don't you want to close the program and reopen it anyways?

Like, close 'desktop' once 'updater' is run? Yeah, is there any way to close a program in code? A return loop with a variable set beforehand, and then a canceling variable placed at the beginning of 'updater'?

EDIT: Things like this bug me. Went ahead and changed function names to make more sense. Also going to try closing the program by using variables like I mentioned.

EDIT 2: Tried implementing a variable to stop an overarching 'desktop' return loop (I thought this would be a poor fix if it even worked, reading over the entire 'desktop' program constantly). Thing is, even with two variables that should stop runDesktop() from continuing, the updater briefly works, then it goes back to 'desktop'. Nothing was fixed in doing this, just more weird behavior.

Or restart the computer. That way startup runs and makes sure everything is correct.

TYKUHN2, on 01 October 2015 - 09:18 PM said:

Or restart the computer. That way startup runs and makes sure everything is correct.

Not quite sure what you're aiming at, sorry…

I'm really appreciative of all the help, but I guess I'd like to shift this from a "how do I fix this" thread to a "how do I properly and cleanly do what I'm trying to do" thread. I'm gonna re-write the whole code, so how could I properly code something which parts could be dynamic and the entire UI be somewhat modular for simple upkeep (from pastebin code)?

If the use of shell.run and executing of other programs within programs themselves won't do (since it's obviously an improper "glue and popsicle sticks" method, even I realize that), how can I have parts of the UI download the latest updated pastebin code so key parts (like the updater, for example) stay up to date?

Generally, auto-updating is frowned upon. Users tend not to like having the code on their drives changed unless they ask for it to be changed. Having your scripts ping pastebin every time you run them is furthermore a waste of time.

Were I to re-write your script, I'd start out by removing as much redundant code as possible. You'll find it much easier to manage once you don't have to constantly scroll through reams of text that don't have to be there.

First off: Redundant functions. If you have a bunch of the things that do very nearly the same thing, then odds are they can be re-written as one.

For example;

Spoiler

local function v05()
  term.clear()
  shell.run("delete","installer")
  shell.run("pastebin","get","qExuNAUg","installer")
  shell.run("installer")
end

local function v04()
  term.clear()
  shell.run("delete","installer")
  shell.run("pastebin","get","WCdpCaEm","installer")
  shell.run("installer")
end

local function v03()
  term.clear()
  shell.run("delete","installer")
  shell.run("pastebin","get","ntSVBj0v","installer")
  shell.run("installer")
end

local function v02()
  term.clear()
  shell.run("delete","installer")
  shell.run("pastebin","get","cEgrT1Qs","installer")
  shell.run("installer")
end

.
.
.

    if b==2 then
      v05()
    elseif b==3 then
      v04()
    elseif b==4 then
      v03()
    elseif b==5 then
      v02()

Here we have the same chunk of code repeated four times. Let's create a function which accepts a parameter, making it more dynamic:

Spoiler

local function vWhatever(pasteID)
  term.clear()
  shell.run("delete","installer")
  shell.run("pastebin","get",pasteID,"installer")
  shell.run("installer")
end

.
.
.

    if b==2 then
      vWhatever("qExuNAUg")
    elseif b==3 then
      vWhatever("WCdpCaEm")
    elseif b==4 then
      vWhatever("ntSVBj0v")
    elseif b==5 then
      vWhatever("cEgrT1Qs")

Or, we could use a table to make things a little more readable:

Spoiler

local vIDs = {v05 = "qExuNAUg", v04 = "WCdpCaEm", v03 = "ntSVBj0v", v02 = "cEgrT1Qs"}

local function vWhatever(vID)
  term.clear()
  fs.delete(shell.resolve("installer"))  -- No need to load the "delete" script separately to do a simple job.
  shell.run("pastebin","get",vIDS[vID],"installer")
  shell.run("installer")
end

.
.
.

    if b==2 then
      vWhatever("v05")
    elseif b==3 then
      vWhatever("v04")
    elseif b==4 then
      vWhatever("v03")
    elseif b==5 then
      vWhatever("v02")

You've done the opposite to this with your setTextLGrey()/setTextWhite() functions, which, for whatever reason, are performing an existence check on the term.isColor() function (it exists on every computer, and has done since CC 1.45). Did you mean to execute it, and act according to the result? In that case, we could override term.setTextColour directly:

Spoiler

local monochromeCols

if _CC_VERSION then  --  If we're running at least CC 1.74,
	monochromeCols = {[colours.white] = true, [colours.grey] = true, [colours.lightGrey] = true, [colours.black] = true} -- then these colours work on normal computers
else
	monochromeCols = {[colours.white] = true, [colours.black] = true} -- otherwise only these ones do
end

local oldSetTextCol = term.setTextColor  -- Keep a copy of the old function pointer.

term.setTextColour = function(textCol)  -- Now let's point to a new function:
	if monochromeCols[textCol] or term.isColour() then  -- If the colour's valid on a normal computer, or if this computer's advanced, 
		oldSetTextCol(textCol)  -- then just switch to the requested colour.
	else  -- otherwise, 
		oldSetTextCol(colours.white)  -- default to white instead.
	end
end

term.setTextColor = term.setTextColour

Now you can simply call term.setTextColour() normally, and colours will be filtered automatically.

This code can be further reduced via ternary-style syntax - see this article as to how it works:

Spoiler

local monochromeCols = _CC_VERSION and {[colours.white] = true, [colours.grey] = true, [colours.lightGrey] = true, [colours.black] = true} or {[colours.white] = true, [colours.black] = true}

local oldSetTextCol = term.setTextColor

term.setTextColour = function(textCol)
	oldSetTextCol((monochromeCols[textCol] or term.isColour()) and textCol or colours.white)
end

term.setTextColor = term.setTextColour  -- American support.

This technique lets us boil big chunks of code like this:

Spoiler

  if wip==1 then
    if not term.isColor then
      term.write("NOTE: WIP BUILD - CURRENTLY STABLE")
    elseif term.isColor() then
      term.setTextColor(8192)
      term.write("NOTE: WIP BUILD - CURRENTLY STABLE")
      term.setTextColor(1)
    else
      term.setTextColor(1)
      term.write("NOTE: WIP BUILD - CURRENTLY STABLE")
    end
  elseif wip==2 then
    if not term.isColor then
      term.write("NOTE: WIP BUILD - CURRENTLY UNSTABLE")
    elseif term.isColor() then
      term.setTextColor(16384)
      term.write("NOTE: WIP BUILD - CURRENTLY UNSTABLE")
    else
      term.setTextColor(1)
      term.write("NOTE: WIP BUILD - CURRENTLY UNSTABLE")
    end
  end

… down to something like this:

Spoiler

  if wip==1 then
    term.setTextColour(colours.green)
    term.write("NOTE: WIP BUILD - CURRENTLY STABLE")
  elseif wip==2 then
    term.setTextColour(colours.red)
    term.write("NOTE: WIP BUILD - CURRENTLY UNSTABLE")
  end

There are other areas where you're doing things the long way, but this may be enough to chew on for now. By spotting similar structures in your script you should be able to reduce things down quite dramatically.

Bomb Bloke, on 04 October 2015 - 05:56 AM said:

Generally, auto-updating is frowned upon. Users tend not to like having the code on their drives changed unless they ask for it to be changed. Having your scripts ping pastebin every time you run them is furthermore a waste of time.

I only was trying to auto-update the updater so the newest NhUI versions would be listed, and it only pinged Pastebin when clicked, but I see where you're coming from. Same for the uninstaller, which would remove every single NhUI program installed. Again, I see what you're coming from.

Bomb Bloke, on 04 October 2015 - 05:56 AM said:

You've done the opposite to this with your setTextLGrey()/setTextWhite() functions, which, for whatever reason, are performing an existence check on the term.isColor() function (it exists on every computer, and has done since CC 1.45). Did you mean to execute it, and act according to the result? In that case, we could override term.setTextColour directly:

Well, I was trying to take into account versions before 1.45, but I can see how this would be redundant. I'll definitely work into implementing this into my code soon. Thanks so much for letting me know about this!

Bomb Bloke, on 04 October 2015 - 05:56 AM said:

There are other areas where you're doing things the long way, but this may be enough to chew on for now. By spotting similar structures in your script you should be able to reduce things down quite dramatically.

I'll start there with the re-write. Makes sense that that's the best place to start. Still wondering, is there a way to work around for the updater to update itself, as much as a waste of time as it is? Or do you think it's completely pointless? I figure that way if it updates every time you wouldn't have to update version by version, but instead have an updated list available. It's only the updater that auto-updates and only when it is called for, not automatically.

Again, thanks so much for the help!

Found a much better alternative to shell.run (something along the lines of os.loadAPI), so that solves my last inquiry. Thanks again for all of the help!