nikkisaurus / FarmingBar

4 stars 1 forks source link

[REQUEST][alpha] Link instances of objectives to templates #32

Closed niketa-wow closed 3 years ago

niketa-wow commented 3 years ago

I have a feeling it might not be an actual logic bug but more so non-intuitive user experience.

What I mean is that currently (in contrast with how older versions worked) you said when you make or change an objective you are essentially making changes to the template (class) used to make copies for buttons (instances of that class).

So what I think is happening is that after you create the objective, if you make changes to it (change the condition logic, update the custom condition or anything really) but you have already dragged the previous version of the template on a bar, that instance of the objective does not update.

There's probably some confusing "bug like" things happening with objective renames (at the template level) if they match an already dragged previous version of that or another instance of an objective that's on a bar.

TL;DR: User might expect that when they edit an objective in the objective builder, instances of that objective that are on bars are also updated, which doesn't appear to be the case at the moment.

Re-dragging the updated objective to the bar (or bars it was on) setting bank/alts again etc, ie manually updating instances of the edited objective would be the "fix" but it would have to be pointed out to the user, I don't think it's obvious.

Originally posted by @Road-block in https://github.com/niketa-wow/farmingbar/issues/30#issuecomment-883463828

niketa-wow commented 3 years ago

Btw, here was what I started with as far as instances of the objective:

  1. Add a value in the char db of the button's item that has a reference to the template it was made from
  2. Add the character->bar->button IDs to the objective template itself
  3. On login, as setting buttons up, check if it's linked to a template and if so check if template has changed & update
  4. On template change, update all instances you can (aka the current toon only; other toons are updated on the login)

In theory this sounds like it'd work fine, but I don't remember why I gave up. Maybe it was just getting messy to keep it all clean at the time.

Edit: I'm also thinking the link in the template itself for the diff instances is pointless. Can just scan all the buttons and check for the template anyway. Unless the user has like hundreds of hundreds of populated buttons it shouldn't be bad on performance.

To be honest, after some thought, I think the reason I gave up on it was because I wasn't sure how I'd handle changing includeAllChars and includeBank. But now that those are button only, it might be irrelevant.

No! I just remembered why I decided it was too complicated, because I couldn't make up my mind how to handle this issue:

  1. Say you have an objective, and you put it on the bar. But then later you delete the objective. Do you leave all the instances on all bars? (Maybe if the linked reference doesn't exist, remove the reference and treat it like a normal objective?) Or do you remove all instances of it?
  2. What if you rename it? This might be why I had the list of all buttons using the objective, because otherwise if you then created a new objective with the old name, it would then link to the wrong thing. The login process would then be more complicated, as you'd then have to check for the template ref, check the template itself to see if the button is included, then do whatever.
  3. Creates the extra step of when you move a button instance, you then have to go update the template.

I think basically all of these things combined was just too much for me to think about while still deciding how it SHOULD behave. I can probably still do it, but it'd be more ideal if I can think of a cleaner way to implement it than all this.

niketa-wow commented 3 years ago

https://github.com/niketa-wow/farmingbar/releases/tag/v3.0-alpha11.1

Road-block commented 3 years ago

I read through it and I think you are correct.
There's too many edge cases, it gets complicated real fast.

I think the best idea is to leave it as is but add hints/help what user should do to update objectives on bars.

Ideally in-game but at least on the addon page / wiki.

The only thing that is not very intuitive at the moment is that you create an objective > you drag it to a bar.
If you want to edit that instance properties you cannot do that directly, you have to edit the template and re-drag.
If that info is shown to the user (in the help tab in-game, or a tooltip hint ideally) it is no problem at all.

niketa-wow commented 3 years ago

I sucked it up and implemented it. I took some time to clean up the code first so it didn't seem as overwhelming.

It works like this now: Anything you change on the template updates the instances on all characters. But if you delete the template, instead of removing it from all bars like before, it creates a copy that becomes its own individual objective. Basically a ghost.

Which gives me an idea. I can implement a way to create a template from a button. So that would allow someone to recreate a deleted template if it's still on their bar. Or could be another quick way to make certain things. (like dragging an item on the bar but then going in and editing maybe the objective).