ironmonk88 / monks-tokenbar

GNU General Public License v3.0
48 stars 46 forks source link

[BUG] "Show Actors" option only shows the one character selected in User Configuration, not all actors owned by that Player #453

Open tonyrobots opened 8 months ago

tonyrobots commented 8 months ago

Describe the bug "Show Actors" option only shows the one character selected in User Configuration, not all actors owned by that Player. (I'm using the Dungeon Crawl Classics system, v .040.0)

To Reproduce Steps to reproduce the behavior:

  1. Create two player character actors
  2. Assign ownership of both to the same player
  3. Set TokenBar setting to "Show Actors" option = True
  4. Only the one character selected in User Config appears in the tokenbar.

Expected behavior My expectation would be that all actors owned by the player would appear if that option is checked, assuming their disposition is set to "friendly." For GMs, that means all player-owned actors set to friendly.

Please complete as much of the following information as possible:

Additional context Add any other context about the problem here.

It looks like this code in the getCurrentTokens() function in tokenbar.js is responsible. I'm not very knowledgeable about the Foundry API but it looks like it's just grabbing a character attached to the user object (e.g. the one set in the Config I mentioned above), and not attempting to iterate through all actors to find ones owned by the player, but I could be wrong :

  if (setting("include-actor")) {
    for (let user of game.users) {
      if (
        (user.active || setting("show-offline")) &&
        !user.isGM &&
        user.character &&
        !this.entries.find((t) => t.actor?.id === user.character?.id)
      ) {
        this.entries.push({
          id: user.character.id,
          token: null,
          actor: user.character,
          img: null,
          thumb: null,
          stats: {},
          resource1: {},
          resource2: {},
          cssClass: "only-actor",
        });
      }
    }
  }
tonyrobots commented 8 months ago

I might take a swing at making the change myself, but I suspect there may be a problem with what I'm looking for, so hoping someone who knows better than I do will weigh in... thank you!

tonyrobots commented 8 months ago

Replaced that block with this:

  // add actors without tokens that are owned by users, if setting is enabled
      if (setting("include-actor")) {
        // iterate through all actors and if they belong to a player, add them to contention, pending other checks
        for (let actor of game.actors) {
          let t = actor.prototypeToken;

          if (actor.hasPlayerOwner) {
            // exclude actors that are explicitly excluded
            let include = t.getFlag("monks-tokenbar", "include");
            if (include === "exclude") continue;

            // show offline users, if setting is enabled
            let showOnline =
              setting("show-offline") ||
              game.users.find(
                (u) => u.active && u.character?.id == t.actor?.id
              );

            if (!showOnline) continue;

            // exclude actors that don't pass the ownership/ permissions threshold
            let canView =
              game.user.isGM ||
              t.actor?.isOwner ||
              t.actor?.testUserPermission(
                game.user,
                setting("minimum-ownership") || "LIMITED"
              );

            if (!canView) continue;

            debug("Adding actor", actor.name);
            if (!this.entries.find((a) => a.actor?.id === actor.id))
              this.entries.push({
                id: actor.id,
                token: null,
                actor: actor,
                img: null,
                thumb: null,
                stats: {},
                resource1: {},
                resource2: {},
                cssClass: "only-actor",
              });
          }
        }
      }

I suspect I've made some basic error or neglected something, but this appears to be doing what I was hoping. So far have just been hacking the file in place, but I can fork the repo and submit a pull request if you'd like.

tonyrobots commented 8 months ago

(Note: that it ignores disposition, and just uses the more direct monks-tokenbar include/exclude attribute )

ironmonk88 commented 8 months ago

That's not a bug, that's how it was designed.

If you include all the player's owned characters, that could drop a bunch of characters onto the tokenbar that aren't necessarily intended to be there. Any previous actor the player had before switching, or any actors that had died, or any actor that was created in case the current character died, would show up. And you wouldn't have the option to be able to remove them.

The only reliable way to include actors is to limit it to just the selected actor, that way you know it's supposed to be included, anything else is just begging for trouble.

tonyrobots commented 8 months ago

Okay, I understand. In my setup, you could exclude actors by choosing the "Exclude" option in the "Show on Tokenbar" setting, which seems natural. My use case is a scenario where are multiple characters per player, but we're using theater of the mind, so tokens don't necessarily appear in scene. (This would likely be a common approach for those running lvl 0 "funnel"-style adventures.) I was hoping to have an easy way for players to see all of their characters and access their sheets, and I thought the "Show Actors" option was meant to support that. Maybe there's another module for that, or I can hack together something myself.

Appreciate the consideration!