Botspot / pi-apps

Raspberry Pi App Store for Open Source Projects
GNU General Public License v3.0
2.02k stars 205 forks source link

New Pi-Apps GUI #1578

Closed Botspot closed 2 years ago

Botspot commented 2 years ago

Hey everyone, Discord seems to be down at the moment, so we'll chat here.

I think the new Pi-Apps interface is complete, but it's best for several eyes to look at it before I upload changes to nearly every major script. ~~Attached is a zip file of my Pi-Apps directory (I took out the .git and data/status folders) pi-apps.zip Extract the zip anywhere you want, and give it a spin!~~ theofficialgman edit: I suggest cloning from the PR into another folder like the /tmp folder:

cd /tmp
rm -rf /tmp/pi-apps
git clone https://github.com/Botspot/pi-apps.git
cd pi-apps
git checkout new-gui
# run the gui
./gui

The PR can be found here: https://github.com/Botspot/pi-apps/pull/1580

Feedback appreciated. This new script design is substantially more complicated than all previous versions of the gui script, so there may be bugs in the flow of dialog windows. In addition to YAD's behavior being changed to a dual-pane layout with integrated list-refreshing, I've also rewritten the xlunch implementation to allow integrated list-refreshing. (avoiding having to close and re-open xlunch when selecting a category)

Pinging a few contributors who might be interested in testing it: @theofficialgman @cycool29 @Itai-Nelken @ryanfortner @mobilegmYT @CleanMachine1 @Jai-JAP @amirdahan

Edit: a previous version of the zip had DIRECTORY hard-coded to ~/pi-apps. It has since been fixed.

theofficialgman commented 2 years ago

setting the color isn't really a hack... it should be the default not setting it lets GTK/yad do whatever it wants with the color. in this case it adopts whatever the cursor was hovered over.

I'm unaware if this is an X11, GTK, Desktop environment, or yad change, but I'm sure I wouldn't be the only person to experience it

Botspot commented 2 years ago

Alright, the color-detection workaround can be used if yad is detected to be a version older than 0.40.0.

theofficialgman commented 2 years ago

Again, that doesn't occur on yad 0.40.0, so therefore it won't occur in the past three debian versions.

Stretch has yad 0.38, fyi. so not the past three. still don't know where the issue comes from though

Botspot commented 2 years ago

Stretch has yad 0.38, fyi. so not the past three. still don't know where the issue comes from though

Could you try installing yad 0.40.0 on your system to see if the problem goes away? Again, on my system I got no warnings, and rows without a set color were consistently black text on light mode, and white text on dark mode.

theofficialgman commented 2 years ago

I'm leaning on it being a GTK bug (or not a bug, more like a functionality change), I've reviewed the yad commit history and see no mention of this issue. I'll compile yad 0.40.0 once I get home later today and check though

theofficialgman commented 2 years ago

I'm leaning on it being a GTK bug (or not a bug, more like a functionality change), I've reviewed the yad commit history and see no mention of this issue. I'll compile yad 0.40.0 once I get home later today and check though

@Botspot bug was in YAD and fixed in 0.40.0 I can reproduce the bug in 0.39.0 and older when I build and test them. gtk warning spam also goes away in 0.40.0 with no color specified

still looking through the commits to see what actually fixed it... nothing references it. it might have been an accidental fix 😆

I'm ok with this though:

Alright, the color-detection workaround can be used if yad is detected to be a version older than 0.40.0.

theofficialgman commented 2 years ago

yad doesn't have svn repo source code for anything after 0.39.0... they stopped uploading it and just make a release tarball on 0.40.0. the issue was not fixed in the latest source code available in the snv repo... so we have no clue what exactly fixed it unless you diff the tarballs

theofficialgman commented 2 years ago

ok, implemented: https://github.com/Botspot/pi-apps/pull/1580/commits/27a9f67f9d707647534e12de235eaf97b2079573 should be fine for everyone now

theofficialgman commented 2 years ago

@Botspot now thats all out of the way and everything seems stable lets talk plans for this gui.

I added back in the announcements (which now only loads once per session since that can't be re-loaded I think). Do you have plans for anything else regarding this? I personally think it all looks really clean. Maybe the only thing I would think of adding would be some sort of scaling method (increase text fontsize everywhere and windows sizes) for high resolution displays.

Here are a couple of things I really like about this update that haven't been mentioned yet

  1. the settings button being in the main window is very nice (I'm sure people missed it entirely being in a separate .desktop file before)
  2. website and credits being in links in the app window is nice as opposed to a credits button at the bottom

My main concern with this update: The UI stays open when installing/uninstalling a script, allowing the user to select more things and get multiple install instances going at once, potentially causing errors.

Botspot commented 2 years ago

@Botspot now thats all out of the way and everything seems stable lets talk plans for this gui.

I added back in the announcements (which now only loads once per session since that can't be re-loaded I think). Do you have plans for anything else regarding this? I personally think it all looks really clean. Maybe the only thing I would think of adding would be some sort of scaling method (increase text fontsize everywhere and windows sizes) for high resolution displays.

Here are a couple of things I really like about this update that haven't been mentioned yet

  1. the settings button being in the main window is very nice (I'm sure people missed it entirely being in a separate .desktop file before)
  2. website and credits being in links in the app window is nice as opposed to a credits button at the bottom

My main concern with this update: The UI stays open when installing/uninstalling a script, allowing the user to select more things and get multiple install instances going at once, potentially causing errors.

Some people already seem to be doing that and we've mostly mitigated it with apt_lock_wait. I think that allowing the UI to be navigable whilst an app is installing is more important now than ever before - installing one app at a time, waiting for the current one to finish before starting the next one, has got to be frustrating.

To avoid multiple instances of manage running, I could implement a lockfile, but that would have its own issues. Alternatively, the manage script could be changed to use a request-stack architecture: each secondary instance of manage would notify a daemon to add the action to a queue. Then, yad could be used to visualize the queue, roughly similar to how gparted's UI works. What do you think, @theofficialgman?

theofficialgman commented 2 years ago

Some people already seem to be doing that and we've mostly mitigated it with apt_lock_wait. I think that allowing the UI to be navigable whilst an app is installing is more important now than ever before - installing one app at a time, waiting for the current one to finish before starting the next one, has got to be frustrating.

I'm mostly concerned about what the individual app install scripts might be doing at the same time. keeping apt update/install from running at the same time is a must, and as you said thats supposed to be handled by apt_lock_wait. I think the problem there is though you expect to be able to access your dummy deb folder for the whole time install_packages is running, not just when apt itself is actually running.

What could happen is two install_packages get called at similar times. The first one runs the apt update white the other waits in apt_lock_wait. The first one finishes and then goes to create the dummy deb, at this time the second script is now running apt update. First script is done now with creating the dummy deb and goes to install it, second script starts creating its own dummy deb. The first script is trying to install its dummy deb and its Packages file is gone because the second script started making its own dummy deb, overwriting the first scripts one it has already created.

this is what is probably happening when the /tmp/pi-apps-local-packages/Packages disappears as seen in some user logs. In order to mitigate this, a few things need to happen. #_1, rename the /tmp/pi-apps-local-packages folder to use a unique name per script (this will prevent other installs overwriting the folder of another). #_2, well now that I've fleshed out #_1 I can't remember what 2 was supposed to be... maybe it will come back to me

I am also concerned about what the individual scripts might be doing that might conflict with one another.... stuff like scripts that compile code, if the user happens to pick two script that need a good amount of ram, one of them may fail due to out of ram issues caused by the other one. Error logs then might come in for the failed script which only failed because another script was using all the resources... so we would for sure need to add some "current parallel installs" line to the error logging so we can see what all is happening at once.

To avoid multiple instances of manage running, I could implement a lockfile, but that would have its own issues. Alternatively, the manage script could be changed to use a request/reply architecture: each secondary instance of manage would notify a daemon to add the action to a queue. Then, yad could be used to visualize the queue, roughly similar to how gparted's UI works. What do you think, @theofficialgman?

well if you only allow one manage instance, that pretty much means only one install can actually be running at once, its just you let the user queue up installs to happen sequentially. The issue I see there just has to do with how pi-apps is structured. sudo permissions don't pass through between subscripts, so each script always asks for the sudo password the first time its called, no matter if you are within the sudo timeout of another subscript. Users might queue up installs, walk away, then come back to find only the first app installed and the second running script and queue is waiting for a sudo password. (there are solutions for this though if you think you want this sequential route)

Crilum commented 2 years ago

@Crilum ok I so force pushed some changes. delete your testing repo and re-clone it and try again please. if it doesn't work, please send the contents of your data/preload folder here

It works now.

Crilum commented 2 years ago

I clicked the update button in the new GUI, and after updating, this happened:

https://user-images.githubusercontent.com/91354257/158033929-b5629fe3-b01d-4f57-8365-c11f7c61aaf2.mp4

theofficialgman commented 2 years ago

I clicked the update button in the new GUI, and after updating, this happened:

brokenpi-apps-2022-03-12_13.22.24.mp4

@Crilum I already told another user this.... don't do that. its expected to break. read here: https://github.com/Botspot/pi-apps/pull/1580#issuecomment-1065927445 if you restart pi-apps now you will notice you are back on the master branch and none of the new GUI remains

Botspot commented 2 years ago

I clicked the update button in the new GUI, and after updating, this happened:

You just "updated" (downgraded) to the current live version of Pi-Apps. The GUI is different, so the app-list will look weird until you close Pi-Apps and launch it again.

Crilum commented 2 years ago

ohh.. oops. Ok.

Botspot commented 2 years ago

Nice! BTW the app info window should be closed when opening settings.

Could you make a screen recording? In my desktop environment, the app Details windows automatically closes when it is no longer focused.

@cycool29, was this resolved?

cycool29 commented 2 years ago

@cycool29, was this resolved?

Not really. It works when I manually focus and unfocus the window, but it did not automatically close when the settings window is launched. Here is the recording:

https://user-images.githubusercontent.com/88134003/159405303-2de8100e-46ea-46a8-a2f0-e962a98c16a9.mp4

theofficialgman commented 2 years ago

@cycool29, was this resolved?

Not really. It works when I manually focus and unfocus the window, but it did not automatically close when the settings window is launched. Here is the recording:

simplescreenrecorder-2022-03-22_11.57.45.mp4

dang why is everything sooooooo sluggish in your video? is it just because you are recording or do you have some craptastic SD card?

also the issue is likely just your DE, it doesn't unfocus the window when you click off of it

cycool29 commented 2 years ago

dang why is everything sooooooo sluggish in your video? is it just because you are recording or do you have some craptastic SD card?

The video plays well for me. Not sure why it is sluggish at your side.

also the issue is likely just your DE, it doesn't unfocus the window when you click off of it

Then this should be fixed right? I am on a very new Raspberry Pi OS bullseye with VNC enabled (so the window manager changed to openbox).

theofficialgman commented 2 years ago

The video plays well for me. Not sure why it is sluggish at your side.

no not the video itself. your pi-apps is running quite slow to do anything. those lists take a while to generate on your install and it takes amiberry over 7 seconds to open.

on my install it takes lest than 2 second to open the all apps page for the first time (takes you 5) and amiberry loads in 1.5 seconds (takes you 7 seconds). I just think either your cpu was pegged by filming this, or your sd card is really bad

cycool29 commented 2 years ago

I think I found the problem.

The app info window needs to be focused before unfocusing so the automatic-close will be triggered.

But when an app is selected from the app list, the app info window is not automatically focused, and if we don't manually close or focus the app info window before clicking settings, the app info will remain open.

Is there a way to force close the app info window whenever the settings button is clicked? Or maybe automatically focus on the app info window when an app is chosen from the list?

cycool29 commented 2 years ago

no not the video itself. your pi-apps is running quite slow to do anything. those lists take a while to generate on your install and it takes amiberry over 7 seconds to open.

on my install it takes lest than 2 seconds to open the all apps page for the first time (takes you 5) and amiberry loads in 1.5 seconds (takes you 7 seconds). I just think either your cpu was pegged by filming this, or your sd card is really bad

I have a Google Meet tab opened, maybe that's the cause. I will test again when my meeting ends.

Botspot commented 2 years ago

The details window should take focus from the app list. I've never seen it not do this. I'm using Mutter window manager. https://user-images.githubusercontent.com/54716352/159527659-40483ed6-e6e0-494f-99d6-f902b81c37e7.mp4

cycool29 commented 2 years ago

I found a way to make it focus on the app info window in openbox, see https://github.com/Botspot/pi-apps/pull/1580#pullrequestreview-918047824.

cycool29 commented 2 years ago

no not the video itself. your pi-apps is running quite slow to do anything. those lists take a while to generate on your install and it takes amiberry over 7 seconds to open.

on my install it takes lest than 2 second to open the all apps page for the first time (takes you 5) and amiberry loads in 1.5 seconds (takes you 7 seconds). I just think either your cpu was pegged by filming this, or your sd card is really bad

After my meeting ends pi-apps runs smoothly.

Botspot commented 2 years ago

@cycool29, could you try adding the --no-focus flag to line 577 in your /tmp/pi-apps/gui script and see if that solves the window focusing issue?

Edit: it did not help.