drewg13 / foundryvtt-scum-and-villainy

Scum and Villainy game system for FoundryVTT
Other
12 stars 15 forks source link

Faction status sheet #71

Closed MarkPearce closed 12 months ago

MarkPearce commented 12 months ago

created a new faction status sheet based on the factions section of the universe sheet to show faction status, sorted by type and tier.

MarkPearce commented 12 months ago

fixed isGM check on add-faction button

drewg13 commented 12 months ago

Looks nice, you're clearly better with CSS/HTML than I am.

Your last commit broke the sheet and needs to be {{#if isGM}}, not {{#if /isGM}}. Also, the language file needs a translation string for your Actor type. Also, the typo in the nomalizeFactionValue helper is going to drive me nuts. I'll fix all of those on my end though.

One other thing that it really needs is some sort of clear labels on the Status and Jobs fields. There's no indication at all what those signify.

Also, the Delete/Visible/Post icons are unnecessarily faint and should probably be black to match their usage in other places. Unless you want to make them a darker grey everywhere, which would make sense as well.

I'll start trying to figure out the default zero status.

drewg13 commented 12 months ago

Argh, I even had code to try to fix this issue in sav-item.js prepareData(), but I was testing for status.value to be "0" or 0, but not [0]. I'll push an update for this to the master branch which should fix it once and for all.

MarkPearce commented 12 months ago

Thanks for the fixes and merge!

I'll look at the column labels, I was trying to save space. :p , the black controls ... i see what you mean for consistency of style. I wanted to leave the focus on the data, not the controls. Doesn't matter that much as only the GM sees them i suppose. :)

I'll pull the class off that dims them.... think its ok to leave the arrows faint until hover?

drewg13 commented 12 months ago

Yeah, I like the arrows as-is, they're really nice and clean. Oh, I would also add a "Neutral" status to fill that space, although using a white label probably won't look nice in that particular circumstance. I find having it be blank to be bothersome.

Something I did changed the tier properties on the factions to a string instead of a number and broke your romanNumeralTier helper. Easily fixed by changing your comparators to strings, just FYI. Not sure how that happened?

The localization key you need for the Actor type is "ACTOR.TypeFaction-status": "Faction Status", Took me a awhile to figure that one out, just drop it near the others at the top of your en.json

MarkPearce commented 12 months ago

just pulled the repo now, i'll make the adjustments.

The neutral text I intentionally removed. Its not important to know which ones are neutral. It just becomes a wall of text the +/- is the main information here. By having neutral neutral neutral neutral neutral neutral neutral neutral neutral neutral neutral neutral everywhere the real info is harder to find.

drewg13 commented 12 months ago

I see your point on "Neutral", but I think that would only be a problem when you're first loading the sheet up with factions due to the repetition. Once you've got a bunch of them on there, all in different statuses, the repetition won't be present any more, unless you're using the sheet to track all of the factions simultaneously, which is definitely not a recommended practice, even if a VTT makes it easier. I think in my games, we never actively tracked more than 5 or 6 factions, once the game got going and we figured out what we wanted it to be.

MarkPearce commented 12 months ago

Cool, I’m still totally new to SaV, on session 4 of our 1st game.

We put all the factions up and even added a couple new ones…. Then players can sort of see who is out there. (But only gm can open faction sheet)

Maybe the compromise of showing them faint like the arrows works?

On Wed 15. Nov 2023 at 02:26, drewg13 @.***> wrote:

I see your point on "Neutral", but I think that would only be a problem when you're first loading the sheet up with factions due to the repetition. Once you've got a bunch of them on there, all in different statuses, the repetition won't be present any more, unless you're using the sheet to track all of the factions simultaneously, which is definitely not a recommended practice, even if a VTT makes it easier. I think in my games, we never actively tracked more than 5 or 6 factions, once the game got going and we figured out what we wanted it to be.

— Reply to this email directly, view it on GitHub https://github.com/drewg13/foundryvtt-scum-and-villainy/pull/71#issuecomment-1811653118, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVHG6QOS2P7OPI5F336ZBDYEQK3VAVCNFSM6AAAAAA7LGFJ56VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJRGY2TGMJRHA . You are receiving this because you authored the thread.Message ID: @.***>

MarkPearce commented 12 months ago

tested the localization changes v10.312 works fine

[image: image.png]

On Wed, Nov 15, 2023 at 2:26 AM drewg13 @.***> wrote:

I see your point on "Neutral", but I think that would only be a problem when you're first loading the sheet up with factions due to the repetition. Once you've got a bunch of them on there, all in different statuses, the repetition won't be present any more, unless you're using the sheet to track all of the factions simultaneously, which is definitely not a recommended practice, even if a VTT makes it easier. I think in my games, we never actively tracked more than 5 or 6 factions, once the game got going and we figured out what we wanted it to be.

— Reply to this email directly, view it on GitHub https://github.com/drewg13/foundryvtt-scum-and-villainy/pull/71#issuecomment-1811653118, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVHG6QOS2P7OPI5F336ZBDYEQK3VAVCNFSM6AAAAAA7LGFJ56VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJRGY2TGMJRHA . You are receiving this because you authored the thread.Message ID: @.***>