krbz999 / babonus

Other
18 stars 7 forks source link

[Feature Request] Add functionality to help sync one babonus with a canonical bab stored elsewhere #279

Closed p4535992 closed 1 year ago

p4535992 commented 1 year ago

Describe the bug

I think you forgot to expoe the keepId parameter on the api game.modules.get("babonus").api.embedBabonus

To Reproduce

The method game.modules.get("babonus").api.embedBabonus(object, bonus) do not have the keepId parameter The method BabonusWorkshop._embedBabonus(object, bonus, keepId); has a additonal parameter but is not exposed

Expected behavior

Screenshots

Setup

Additional context

The keeId can be useful on copyBonus and moveBonus too

krbz999 commented 1 year ago

This was a conscious decision since it is only used internally. Is there a use case that would necessitate exposing it to the API?

p4535992 commented 1 year ago

It's a use case on a piece of code where I dynamically transfer bonuses from objects A and B to object C. Basically, there's a step where I add the bonuses from object A to object C and a step where I add the bonuses from effect B to object C.

So at some point C has both A's and B's bonuses, now in case A and B had a Bonus with the same name to distinguish them I would have to do some additional code which I would avoid if I could use ids.

This is a problem in the next step where I would like to remove the effects of B from C and at a later time the effects of A from C.

Start C -> C Add A -> C (A) Add B -> C (A + B) Remove A -> C (B) Remove B -> C

However if it is something you don't want to implement it doesn't matter I will make sure I don't use bonus with the same name in more than one object. simple enough.

krbz999 commented 1 year ago

A better change may simply be to change the return value to be the babs created/deleted/moved.

p4535992 commented 1 year ago

Can be useful , in other use cases, in that I don't think it solves the current problem because I still wouldn't have a "track" of which bonus came to me from object A and which from 'object B,

I would need something like the "origin" field of active effects to have a reference to the object from which copied/moved/recovered the bonus.

krbz999 commented 1 year ago

What exactly is the use case here that cannot be resolved by simply turning a bab on or off?

p4535992 commented 1 year ago

I'm sorry I have to explain myself really crappy, I'll try to give you an example at the code level:

const itemA = ...
const itemB = ...
const itemC = ...

const itemsAB = [itemA, itemB];

// Add bonus from A to C and from B to C
for(const itemWithBonusToAdd of itemsAB) {
      const bonuses = game.modules.get("babonus").api.getCollection(itemC) ?? [];
      const bonusesToAdd = game.modules.get("babonus").api.getCollection(itemWithBonusToAdd) ?? [];
      for (const bonusToAdd of bonusesToAdd) {
        let foundedBonus = false;
        for (const bonus of bonuses) {
          // The check for name is not perfect because i can have multiple bonus 
          // with the same name from the itemB, but with different configuration
          if (bonus.name === bonusToAdd.name) { 
            foundedBonus = true;
            break;
          }
        }
        if (!foundedBonus) {
          await game.modules.get("babonus").api.embedBabonus(itemC, bonusToAdd); 
        }
      }
}
...
// after a while i want to remove from C only the bonus i copied from A
      const bonuses = game.modules.get("babonus").api.getCollection(itemC) ?? [];
      const bonusesToRemove = game.modules.get("babonus").api.getCollection(itemA) ?? [];

      for (const bonusToRemove of bonusesToRemove) {
        for (const bonus of bonuses) {
           // Sadly item A and item B have a bonus with the same name 
          // so i removed the bous i copied form the item B too
          if (bonus.name === bonusToRemove.name) {
            await game.modules.get("babonus").api.deleteBonus(item, bonus.id);
          }
        }
      }

What exactly is the use case here that cannot be resolved by simply turning a bab on or off?

This is an automated solution , because the players "mess up" with babonus, so I decided not to let them play with it.

Fixing them by hand is tiring, so I was trying to automate the process as much as possible.

I hope now I have clarified myself if not close the ticket without any problems.

krbz999 commented 1 year ago

I understood the what the first time around, but not the why, and the latter is what is gonna be the reason to consider a feature request.

p4535992 commented 1 year ago

you must have answered me while, I was finishing reissuing the ticket:

What exactly is the use case here that cannot be resolved by simply turning a bab on or off?

This is an automated solution , because the players "mess up" with babonus, so I decided not to let them play with it.

Fixing them by hand is tiring, so I was trying to automate the process as much as possible.

krbz999 commented 1 year ago

An automated solution to what? The process of what?

p4535992 commented 1 year ago

The preconception is that players don't have access to the babonus ui, not being able as a GM to go and set every single item (all the players items are itemC) every time that for example I mistake the configuration of a bonus or the like I have to go by hand to fix them one by one, I decided therefore to set "basic" items (ItemA and ItemB) on which I set the correct bonuses gradually.

At this point I update the bonuses of the many ItemCs in the world using the new configurations of ItemA and ItemB, if by chance ItemA and ItemB have bonuses with the same name I have a hard time distinguishing them, if it is a use case that does not have to subsist, it is perfectly fine I will use different names (e.g. "+1 damage Item A", "+1 damage item B") and end of story.

krbz999 commented 1 year ago

OK, so if I understand this correctly, the actual feature request is for a way to link a babonus to an 'original' source or a way to keep multiple baboni in sync.

p4535992 commented 1 year ago

yes exactly, but put like that it sounds like you have to develop some new mechanism for sync bonus between items, which I wanted to avoid you to do (I had thought with the keepId mechanism that I could handle this right away), is really a very minor feature, but I'll leave the reasoning to you at this point.