OctarineSourcerer / cyberpunk2020

A FoundryVTT system for Cyberpunk 2020. It's playable now! There's a lot of hole, because there's a LOT of system to cover, but it should be mostly usable <3
23 stars 21 forks source link

Exotic Weapon Attack Skill List is Empty #123

Open ElPresidentePoole opened 2 years ago

ElPresidentePoole commented 2 years ago

image

As you can see, when making an Exotic weapon (which normally allows all skills to be used), the list is empty. I have tested this on two systems (one Windows and one Linux) and the issue persists on both.

Tested against the current release, 0.3.6.

ElPresidentePoole commented 2 years ago

I'm new to Foundry development so forgive me if I'm wrong, but I think this is caused by line 72 in item-sheet.js.

    sheet.attackSkills = [...attackSkills[this.item.system.weaponType].map(x => localize("Skill"+x)), ...(this.actor?.trainedMartials() || [])];

Since attackSkills['Exotic'] is empty, it returns an empty array.

ElPresidentePoole commented 2 years ago

Alright so I figured this out. This is less so of a "bug" and more so just what happens when you have an RPG without a set list of skills (aside from the default skills, you can invent new ones as per the 2020 rules). Because there is no static set of skills, how do we get them? Well, we can't look up the skills in the compendium - there is (in theory) an infinite number of possible skills! We do the next best thing - get the list of skills of the actor who currently has the item equipped, and that's exactly what the author did!

sheet.attackSkills = [...attackSkills[this.item.system.weaponType].map(x => localize("Skill"+x)), ...(this.actor?.trainedMartials() || [])];

    // TODO: Be not so inefficient for this
    if(!sheet.attackSkills.length && this.actor) {
      if(this.actor) {
        sheet.attackSkills = this.actor.itemTypes.skill.map(skill => skill.name).sort();
      }
    }

The if statement there just checks if there is anything in attackSkills[key] (in our case, attackSkills['Exotic']) and gives us all of the skills of our owning/parent actor if it's empty. I was going to submit a PR for a quick and dirty warning so player's and referee's know what's going on, but I scrapped it because it felt really hacky and annoying. I'd say this issue now isn't "Exotics Can't Be Assigned an Attack Skill" but rather "There Is No Feedback for How To Make an Exotic Weapon Work". I'd suggest putting a red exclamation mark next to the Attack Skill dropdown in the case of an Exotic weapon in the journal (where it wouldn't have an owner to read skills from) so other people know what's going on and how to fix it.

For curiosities sake, here was my proposed fix.

    if(!sheet.attackSkills.length) {
      if(this.actor) {
        sheet.attackSkills = this.actor.itemTypes.skill.map(skill => skill.name).sort();
      } else {
    alert(`This item needs to be in an actor's sheet before we can determine what skills can be used.`);
      }
    }

Again, I really recommend adding a warning or some kind of feedback so people know what's going on, just maybe not mine lol.

So anyways I just spend hours debugging a bug that wasn't really a bug.

By the way, sorry if I'm spamming your inbox with these issues. I'm currently running a game in 2020 and have a lot of people playtesting it. This whole quest I've been on with exotic weapons started from me trying to understand how to implement bows lol.

OctarineSourcerer commented 1 year ago

Okay! Sorry for taking so long to get back to this. In main, Exotic weapons should now show a textbox for the skill to choose, with suggestions based on the actor that owns it. For items not owned by an actor, this will still let you select a skill.

Two things to note:

Try that out, let me know what you think? :)