antero111 / plugin-presets

A plugin for handling Runelite plugin configurations as presets.
BSD 2-Clause "Simplified" License
7 stars 4 forks source link

Persistent Presets #30

Closed Jademalo closed 1 year ago

Jademalo commented 2 years ago

I'm not sure how feasible this would be, but I've got a suggestion for a "persistent" preset option. This is similar to the idea for preset recording, but has a slightly different use.

When a preset matches and the toggle switch is active, add something like a small lock icon next to the preset; image When the lock is clicked, set the preset into a "Persistent" state; image When a preset is in a persistent state, make it so that any settings changes made to an included plugin will automatically be updated in the preset. Essentially as if you were always clicking "Modified" to constantly update the preset.

If another preset is loaded, the lock is automatically disabled and that preset goes back to functioning like normal.

There are a few situations where this could be really useful. One example is if you have character specific presets, and want to save a different set of highlighted and ignored items per character. Enabling the lock while you're playing means that items you highlight will automatically be saved to that preset without needing to remember to go in and update modified keys manually. Another example could be if Notes data was added as a custom key, so whenever you modify your notes it will automatically update the preset.

Hopefully this is clear enough!

antero111 commented 2 years ago

This is somewhat similiar to the record mode where this could be called as "auto update" or something. This is something I would like to implement.

I am not sure if its clear enough to call it as "lock" since I was thinking of some other lock feature where if you locked some preset, it would not show modified if you made settings. But I tought that its somewhat useless idea since its better to just allways show the modified icon (because still their configs differ and it might be confusing that sometimes the modified does not show).

How would these locks work if multiple presets were locked or should only one preset be locked?

Should we consider if the record mode and this one were somehow merged and only implemented once or should these stay as different features? I would like to know your toughts about this and record mode.

Jademalo commented 2 years ago

I am not sure if its clear enough to call it as "lock" since I was thinking of some other lock feature where if you locked some preset, it would not show modified if you made settings. But I tought that its somewhat useless idea since its better to just allways show the modified icon (because still their configs differ and it might be confusing that sometimes the modified does not show).

Yeah, the lock icon isn't really ideal, I just couldn't think of anything better. My thinking was that it's "locked on", rather than showing as disabled when a change is made like what happens now. It needs to be something to show that the plugin is Live/Active/Persistent. Maybe it could be something like the circular arrow icon used for modified - have it be greyed out when inactive and yellow when active with a tooltip describing what it does, something like "Auto-update included plugins"? That keeps it consistent with Modified too.

How would these locks work if multiple presets were locked or should only one preset be locked?

I was thinking that it would be fine to lock multiple presets if they could both be enabled together, since if they were both on at the same time then they wouldn't conflict. Then if another preset is enabled, immediately remove the locks to prevent any weird issues such as presets being overwritten with another getting loaded. If only enabled presets could be locked, then there shouldn't be any weird conflicts.

Should we consider if the record mode and this one were somehow merged and only implemented once or should these stay as different features? I would like to know your toughts about this and record mode.

Originally I was going to suggest this as part of record mode, but I think the goals of the two features are a bit different. The goal of record mode is to automatically include plugins in a preset when the settings are modified, making it easier to create a new preset. The goal of this is to automatically update the settings of plugins that have already been included in a preset, for settings that are changed frequently such as highlighted/ignored items.

Their function is similar and they're definitely related, but I think their goals are slightly different. I see record mode being used when you first create a preset, and I see this being used when your preset has been created and you're just playing the game.

antero111 commented 2 years ago

If only enabled presets could be locked, then there shouldn't be any weird conflicts.

This may be the way to do it.

You are right that they are similiar but have different goals. I think that they can be added both pretty well and relatively easy to implement since they both kinda same logic.

Next step is to do some refactoring and fixing to the plugin logic and then we can start thinking about implementing this and the record mode.

I can't promise any timeframe on implementation but I will keep this thread updated.

Jademalo commented 2 years ago

Awesome, no rush! Really incredible work you've been doing on this plugin, it's genuinely incredible!

antero111 commented 1 year ago

Hi @Jademalo, been thinking about this feature and have had some new toughts about and I'd like to hear from you that are these suggestions still relevant or have you ran into something that should be changed/improved?

About the icon I tought that should it be a pin? You pin a preset to keep it updated or alternatively it would just be a text like (auto) on the right side of the switch.

The pin could be saved to the preset itself so presets would remember if they where pinned. This way when you switch to your irons preset, it automatically stays as pinned. It would help bigger presets such as main/iron presets etc. so that they allways keep up to date.

The goal of record mode is to automatically include plugins in a preset when the settings are modified, making it easier to create a new preset.

With the pin feature you could create new preset, pin it and then when you make changes the preset automatically updates. Pretty much sums up the preset creation record mode.

Jademalo commented 1 year ago

I've not played in the last month or so (been busy!), but I'd still definitely like to see this!

A pin is a great idea honestly, much better than the lock. I think I'd go for a graphic over text with a mouseover description, you don't want too much clutter and want things to be clear and readable.

The pin could be saved to the preset itself so presets would remember if they where pinned. This way when you switch to your irons preset, it automatically stays as pinned. It would help bigger presets such as main/iron presets etc. so that they allways keep up to date.

Originally I was going to say this sounded like a good idea, but I don't think it would actually work conceptually - How do you disable a preset without disabling the pin? If I enable my main's preset, theoretically it would overwrite the iron's preset if the iron preset was pinned. Since the way the plugin works is that it indicates a preset as active when all settings match, the only way to "disable" a preset is by changing keys it affects. If a preset is pinned then you can't change keys since they automatically overwrite, so you'd end up overwriting them. And if you disable the pin before switching to make it so that it doesn't get overwritten, then it won't know to re-enable the pin next time you enable that preset.

With the pin feature you could create new preset, pin it and then when you make changes the preset automatically updates. Pretty much sums up the preset creation record mode.

They're similar, but do definitely fill different functions. Pin should only be updating the settings of keys that are enabled, whereas record should automatically enable keys as the settings are updated. Pin as I envision it is effectively just the same as clicking "Update" every time you make a change, but that doesn't select keys you don't already have selected. It will be useful because it only affects the keys you care about, so things that you want to change across both your main and iron won't be managed by the plugin and won't automatically be added to the preset. Record on the other hand isn't something to be used all the time, it's specifically to make the job of going through the list and enabling all of the specific keys you want easier. You start recording, change the settings you want to change, and then you've got a preset with the exact keys you want enabled enabled, without any other keys that you don't care about. The actual settings themselves don't matter - it's all about which settings get enabled in the profile.

antero111 commented 1 year ago

If I enable my main's preset, theoretically it would overwrite the iron's preset if the iron preset was pinned

It can be made so that it ignores config change events when switching presets. That way enabling one preset does not overwrite some preset that is pinned.

So we will have 2 ways of auto updating presets:

Should the record mode type pin be able to be toggled on/off for editing? How should it behave if you enable it to a existing preset? How would you enable the record mode? In the edit panel somewhere?

Or should the record mode be only on preset creation?

Jademalo commented 1 year ago

It can be made so that it ignores config change events when switching presets. That way enabling one preset does not overwrite some preset that is pinned.

I've had a think, and I think the system might be better if it's a bit simpler. You don't want to make the system obtuse or weird for the user, and you want to make sure it works the same way as the rest of the plugin. I also think it's probably just better to call it auto-update, since that keeps it consistent with existing features and language in the plugin already.

Yes, there will be two different ways of auto updating presets, but they both definitely have different uses and functions. I also do think you should be able to use record mode whenever, so long as it's clearly designed to be a temporary "setting up" toggle as opposed to the persistent nature of auto-update.

Taking all of this into account, these are my two concepts for the two functions;


Auto-Update

In the preset configuration just above the search box, have a checkbox for "Auto-update profile". When the profile is enabled directly by clicking the toggle switch (but not if the switch is active by the preset matching), it goes to the top of the list and gets the update icon, like this;

image

When you then change any of the keys it alters in general runelite settings, it updates them automatically as if you keep going into the preset configuration and clicking update.

If you enable any other profile, then the auto-updating is disabled. Not just other profiles that are also persistent, but any other profile. It can then be re-enabled by clicking on the toggle switch.

This means that there can't be any issues with keys being overwritten, or weird edge cases where keys don't update. It means there's absolutely no chance of conflict, since it will only auto-update when everything is "static", so to speak. It's clear and obvious when this is happening, and a lot easier to understand for the user.


Record Mode

In preset configuration, have a record button to the left of the local/cloud icon. Have it work such that when it's clicked, it's enabled until you either click it again, or you exit out of the preset configuration menu back to the main preset list. This means it can't accidentally be left on, but it's easy to enable should you want to modify what keys a preset saves.

Make it so that when it's enabled, any changes to the settings will both automatically update that given key as well as enable the key if it wasn't already enabled. This means that when you want to set up a preset, you make it, go into the configuration, click the button, change what you want to be changed, then either click it again or go back to the menu.


Hopefully I've got what's in my head down clearly enough!

antero111 commented 1 year ago

That sounds very good.

Should the record toggle just be a red dot?

I like the "Auto-update profile" checkbox idea. I'd like to add it also to the preset panels preset toggle switch context menu for faster quick toggle.

Should the preset being auto-updated be remembered if client restarts?

Do you think that there will be need for being able to have multiple presets auto-updated or is just one sufficient?

Jademalo commented 1 year ago

Should the record toggle just be a red dot?

Yeah, I can't think of anything better. I'd just make sure it's clear that it's enabled or disabled, maybe grey when disabled and red when enabled?

Should the preset being auto-updated be remembered if client restarts?

I'd say yes, normally for me at least I'm playing the same character for a while.

Do you think that there will be need for being able to have multiple presets auto-updated or is just one sufficient?

I was thinking about this, and honestly it feels like a really niche case that would require two at a time. Having just the one that's automatically disabled when you enable any other preset gets rid of all weird situations where presets are fighting to overwrite eachother, and should also be a lot less complex for the user.

antero111 commented 1 year ago

qTKAiYhegQ

Heres how the auto update looks like now. It gets disable once you load some other preset.

It works but I think that the usage of this feature could be expanded but I'll leave that for later once you get to test this.

Also you can now just double click to edit the preset.

antero111 commented 1 year ago

Based on my testing I think that the best usage of this is when you have e.g. iron and main presets that contain lots of settings and then you keep the auto update on to keep those presets up to date.

Having one base preset that you want to keep updated and then multiple smaller ones like for skybox color etc. are little troublesome since once you enable some preset the auto update gets removed from the base preset. Yeah its intended behaviour but possibly little annoying in the long run. Should something be made to that?

Jademalo commented 1 year ago

Sorry about taking so long to reply, been extremely busy since it's christmas etc!

Would you be willing to make a test branch with some of the bleeding edge dev stuff in so I can test them a bit more directly? It'll probably make giving feedback a bit easier, and I know just enough to be able to poke around with the code and probably give better suggestions.

Visually that gif looks great, definitely think the update arrow icon is the right choice!

antero111 commented 1 year ago

That auto update is already committed to master. If you like you can clone the repo and build it or alternatively make a shadow jar and run it. Refer to the RuneLite Discord and its wiki for help.

Jademalo commented 1 year ago

Oh, so it is, lol, no idea how I missed that it had been pushed. I had just assumed it hadn't been since it wasn't in the hub, didn't realise that that was done separately. I've built the client a couple of times so that's not a problem, I'll have a poke around when I get the chance and let you know my thoughts.

Jademalo commented 1 year ago

Right, had a bit of a poke around, hopefully this feedback is useful!


After playing around with it for a bit, I agree that having the auto-update be disabled when enabling another profile is annoying if you have lots of single-setting profiles.

I was poking around with the code for a while, and realised it wasn't too hard to make it so that it only disables it if the auto-updating plugin and the to-be-enabled plugin don't match. I don't think I originally suggested it because I assumed it would be an absolute nightmare to implement, but honestly I surprised even myself lol.

After testing that for a while it feels much nicer, so I've submitted a PR (#45) with those changes.

(Side note I'm super not a developer, I poke around here and there but I've got little to no idea what I'm doing lol, especially when it comes to object oriented stuff. Hopefully it's not an absolute mess!)


I think one improvement that could be made would be to have three levels to auto-update rather than the current two - Enabled, Inactive, and Disabled.

When you select the auto-update icon in a preset's config, it sets auto-update to inactive for that preset. This doesn't actually auto-update anything, but marks that specific preset as one that will auto update. If the preset itself is already enabled then it enables auto-updating, however if it's not then it sets it to inactive. This puts a grey arrow icon next to the enable toggle on the main preset menu. Multiple presets can be put into this Inactive state, but only one can be Enabled at a time.

If the you enable the preset with the toggle switch, it will load the keys of that preset and then enable auto-updating, turning the icon orange. If you click the grey arrow icon, it will update all keys in the preset to their current state and then enable auto-updating, turning the icon orange. I would recommend a confirmation box or something similar when doing this.

This means that if you change a preset, re-enabling auto-updating doesn't require going into the config menu to do so. It also means that for the Main/Iron setup, when you flick between the two it will automatically enable auto-updating.

antero111 commented 1 year ago

Inactive auto updater sounds good. I was thinking something like that before going on with the saving to config route. Some changes need to be made to the auto updater since now it only stores the one auto updated id in the config. Possibly have to save the inactivity to the presets but I'll have a go at implementing that.

Thanks for testing and putting the time to write that pr.

implementing this will go somewhere after christmas so in the meantime merry christmas and happy new year!

Jademalo commented 1 year ago

I think saving the inactivity is probably the right way to go, since the goal of that would be to mark a given preset as one that automatically auto-updates when you enable it. As I said with the Main/Iron example, being able to switch those profiles and have whichever is most recently loaded auto-update would be pretty great. I had a look at doing it myself last night, but it was way out of my depth lol.

Hope you have a lovely christmas and new year too!

antero111 commented 1 year ago

Hi, https://github.com/antero111/plugin-presets/commit/a3f27680dff249c8ebf709aeb300b511268559e0 added a way for remembering the auto update on presets. Now you can have multiple presets tagged as "auto updated" and when you load one, the auto updater gets enabled automatically.

It would be nice if you could take it for a spin and see if something still needs to be added. When it's good we could update the plugin.

The record mode won't be included in the next release. Currently don't have the time implement it.

Jademalo commented 1 year ago

Done a little bit of testing (been super ill since christmas), and this feels great! Once everything is set it seems to work exactly like I was envisioning, which is fantastic.

I would, however, maybe slightly change what happens when you first enable auto-update on a preset as I feel like it's a bit unintuitive.

At present, the "Auto updated when loaded" flag isn't really indicated on the profile config menu. If you set a profile to be auto updated and then load another such that it goes into that state, going into the profile config menu shows update all as usual and also has the auto update icon greyed out. I would have expected this to show the preset as auto updating in this idle state. In addition, I think enabling auto update on a non-matching profile should enable the flag without actually activating the auto-updating. This means you can't accidentally overwrite your profile trying to set it up.

I've attempted to implement this in #46 and mostly got it working, however I can't seem to track down a really weird bug where if you click the enable/disable icon in the profile config menu twice in a row, it doesn't work properly. If you back out of the menu to the preset list and go back in every time, then it works as I was intending.

Hopefully that's clear enough, I'm not at 100% so it might be a bit all over the place. The functionality in the PR should hopefully give a better idea of what I'm thinking.

antero111 commented 1 year ago

This means you can't accidentally overwrite your profile trying to set it up

I had this same problem when implementing that auto update all when enabling auto update. It was originally made like that because it "automatically update all" so it would click update all right away and be updated instantly. But yeah it's not really good if you accidentaly click auto update on some other preset and then it will get overwritten by accident.

One simple fix to that would be a confirmation like preset deletion has. If preset has "update all" visible, when clicking auto update, it would ask you something like "this will run update all on this preset and then turn on auto updating".

Your implementation would fix this but it raises some other issues with handling how the "auto updated" text and "Update all" button should be visible. If you enable auto update, the "update all" should still be visible because it's still something you can/should do to that preset but also the auto update text should be visible because it's auto updated after you update all or load that preset.

I think that the original way with a confirmation window would be the best. It's simple and you can't accidentaly overwrite presets. Mostly the auto update should be enabled when all configs match e.g. you loaded that preset or clicked update all.

Jademalo commented 1 year ago

Hmm, I wonder if having the auto updated text only when it's actively auto updating and having update all be visible when it's in the idle state would work? That's assuming the auto update icon being orange is enough of an indicator for it being "enabled" for a specific preset, which I think it is honestly.

I'll give that a try and see how it feels, I should have a bit of time tonight. I agree that a confirmation window would definitely be a good idea if going with the original design.

Jademalo commented 1 year ago

Ok, changed how the label/update all visibility works. I really like how this feels. It's clear which state a given preset is in, and there's no risk of automatically overwriting your preset.

Personally, I'd go with this, or something close to it. I think the auto update icon going grey to orange for it being enabled on a preset is enough, and the auto update text showing when it's active is nice and clear.

EDIT: There's one minor edge case where if you enable auto updating and then click update all while there's no other active auto updating profile it won't activate the auto updater, just noticed that.

EDIT2: Ok, that's been fixed. It will now enable the auto updating if there's no other active auto updating profile and you click update all. This functionally feels great to me, I'd like to know what you think!

antero111 commented 1 year ago

Hi, pushed 8d9eca0ae79bea93ad6dbfe90855cdb2b5c3ac4a to master, it should fix the preset editor not being synced. The auto updater still has some bugs but imo those are rare edge cases and they don't break the plugin.

I'm ready to ship this as of now and then do some fixes later if something needs to be done.

Jademalo commented 1 year ago

Awesome, that sounds great!

antero111 commented 1 year ago

Closing this for plugin update. Let's open new issues for fixing future bugs and improvements