Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Random wallpaper? #6

Closed
ghost opened this issue Aug 30, 2023 · 22 comments
Closed

Random wallpaper? #6

ghost opened this issue Aug 30, 2023 · 22 comments

Comments

@ghost
Copy link

ghost commented Aug 30, 2023

Pretty self explainatory and was already implemented in wallutils.
Maybe you can implement it on waypaper itself?

@anufrievroman
Copy link
Owner

Hi, thanks for the suggestion. I started implementing it. There will be --random argument for cli usage, as well as a button to set a random image from GUI. This will work on all backends.

@anufrievroman
Copy link
Owner

So, I've implemented it. There is --random flag, or Random button, or R hotkey to set a random wallpaper. If you are using AUR package, you can already test it.

@ghost
Copy link
Author

ghost commented Aug 31, 2023

Alright, tysm. I'll close this issue.
(What the heck this is fast as hell updates wha-)

@ghost ghost closed this as completed Aug 31, 2023
@anufrievroman
Copy link
Owner

Haha, yes, I figured I better work on the application while I have motivation, because in a few months time it's becoming increasingly difficult to find it :D

@ghost
Copy link
Author

ghost commented Sep 1, 2023

true lmao

@badloop
Copy link

badloop commented Feb 12, 2024

Looking to help with this project... appears that --random just opens the app (at least with swaybg as backend, havent tested others. My assumption would be that it would set a random wallpaper and exit. I can work on implementing this if no one objects.

@anufrievroman
Copy link
Owner

Hi, the --random option works only together with --restore option, then it works like you describe. Otherwise, with GUI it sort of doesn't make sense. Perhaps it could be improved so that when --random is passed the --restore is assumed.

@badloop
Copy link

badloop commented Feb 12, 2024

ah i see... yeah I think with it being a top level option like that it would flow more naturally. I can work on that. I was also going to add an option to output the current wallpaper path. that way it could be chained together with other utilities like gtkgreet for a seamless user experience

@anufrievroman
Copy link
Owner

Sounds good, feel free to propose pull requests on this :)

@badloop
Copy link

badloop commented Feb 14, 2024

Mind re-opening this for my tracking? Also how open to change are you? Right now there seems to be a disconnect between how the random wallpaper is set via the cli in __main__.py and how the app itself sets a random wallpaper. I would assume that upon setting the wallpaper we would want to update the config, but since change_wallpaper() is being called directly, this isn't happening. I'm more than happy to make these updates, but dont want to step on any toes.

@anufrievroman
Copy link
Owner

anufrievroman commented Feb 14, 2024

Hmm, yes, you're right, it's strange that we don't update the config when the wallpaper is changed via cli --random. It should save the config indeed. Please update, I am quite open to changes, thank you for asking.

@anufrievroman anufrievroman reopened this Feb 14, 2024
@badloop
Copy link

badloop commented Feb 15, 2024

I dont have multiple monitors. Can someone explain the functionality that is expected/currently takes place when --random is used in a multi-monitor situation? I would assume we assign a random image to each monitor, but how is this reflected in the config file for restoring later?

@anufrievroman
Copy link
Owner

anufrievroman commented Feb 15, 2024

Sure, for example, under normal circumstances, config stores two lists: cf.wallpaper and cf.monitors so that each monitor has a corresponding wallpaper, unless user sets a wallpaper for "All" monitors, then there is just one element in these lists, i.e. cf.monitors = ["All"] and the same wallpaper is applied to all monitors.

Now, when you use --random via cli, it should take the cf.wallpaper list and replace every item in the list with a random wallpaper. This is what happens in:

        for wallpaper, monitor in zip(cf.wallpaper, cf.monitors):
            if args.random:
                wallpaper = get_random_file(cf.backend, cf.image_folder, cf.include_subfolders)
            change_wallpaper(wallpaper, cf, monitor, txt)

However, if we use Radnom button in the GUI, it just sets the random wallpaper to whichever monitor is currently selected in GUI.

p.s. note that despite the name cf.wallpaper it is actually a list of several wallpapers (or at least one), for historical reasons....

@anufrievroman
Copy link
Owner

But what is missing, is something like this at the end to save the changed wallpapers:

if args.random:
    cf.save()

@badloop
Copy link

badloop commented Feb 15, 2024

yeah thats my main difficulty in reading through this. It seems the Config object is a conflation of both the external configuration file and the running app, and things are intermixed in ways that make it unclear which thing needs to be changed. I'm assuming based on what you're saying that in the configuration file itself the values of wallpaper and monitors are going to be comma separated strings that could be split into a list when read.

@badloop
Copy link

badloop commented Feb 15, 2024

I'm almost ready to submit the PR, just wanted to make sure I wasn't breaking multiple monitors

@badloop
Copy link

badloop commented Feb 15, 2024

It does appear though that --random breaks down when monitors is 'All' because of this logic:

        if self.selected_monitor == "All":
            self.monitors = [self.selected_monitor]
            self.wallpaper = [self.selected_wallpaper]
        elif self.selected_monitor in self.monitors:
            index = self.monitors.index(self.selected_monitor)
            self.wallpaper[index] = self.selected_wallpaper
        else:
            self.monitors.append(self.selected_monitor)
            self.wallpaper.append(self.selected_wallpaper)

So in the case of monitors being 'All' and --random being passed, do we want the same wallpaper to be applied to all monitors, or a random wallpaper to be applied to each monitor?

@anufrievroman
Copy link
Owner

The short answer I think is the same wallpaper on all monitors.
The way changer.py then changes wallpapers is basically such that "All" is treated as a monitor. If we want separate random wallpapers for different monitors, that would require a larger rewrite. Considering that this is a small convenience function, I think it's not worth complicating things here too much. I mean, if somebody wants to script these things, it would perhaps be better to use swww directly, without waypaper as a proxi.

@badloop
Copy link

badloop commented Feb 15, 2024

... If we want separate random wallpapers for different monitors, that would require a larger rewrite. ...

lol, this is where I had been headed. I had done some significant reworking of the entire app before I just stopped and said it would probably be better to just create a new app instead. I've updated __main__ with what I think it should need to be for this. Its inefficient because its saving the output file on every loop due to that logic above, but it not heavyweight, so its not a huge deal. Submitting PR now.

Edit: not sure of your preferences, but I use black and isort, so there's some additional changes due to the opinionated formatting it does.

@anufrievroman
Copy link
Owner

About the config, you can thing of it as an object that contains all the parameters of the app when it runs. It's just that initially these parameters are loaded from the file, and then saved back into the file when changed. It was supposed to be so that users don't manually edit config.ini.

Yes, in config.ini wallpaper is comma separated, which later converts into the list.

@badloop
Copy link

badloop commented Feb 15, 2024

PR #29 submitted

@anufrievroman
Copy link
Owner

I tested it, seems to work fine. I'll close it then. Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants