drewg13 / foundryvtt-scum-and-villainy

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

Faction sheet tweaks #73

Closed MarkPearce closed 11 months ago

MarkPearce commented 12 months ago
  1. Fixed deprecated warnings in en.json for all effected types. eg "TYPES.Actor.faction-status", "TYPES.Item.planet": "Planet", etc
  2. fixed helper function spelling normalizeFactionValue
  3. use string comparison for tier helper
  4. added faint Neutral labels
  5. fix isGM that broke sheet

**I don't think we need a status | Jobs header label. hover tells you what it is, as does clicking button... I mean it would be better, but theres not really room. Trying to make sure it fits 1280 screens (foundry min)

drewg13 commented 12 months ago

Be careful with the deprecation warnings. I was leaving those in on purpose until V12 to maintain V10 compatibility. No idea how many people are still on V10, but I try to maintain at least 2 versions, when it's feasible.

Thanks for the flexibility and the quick response. I'll mess with it some more tomorrow evening.

MarkPearce commented 12 months ago

tested on v10.312 with changes. no warnings/errors.

[image: image.png]

On Wed, Nov 15, 2023 at 4:32 AM drewg13 @.***> wrote:

Be careful with the deprecation warnings. I was leaving those in on purpose until V12 to maintain V10 compatibility. No idea how many people are still on V10, but I try to maintain at least 2 versions, when it's feasible.

Thanks for the flexibility and the quick response. I'll mess with it some more tomorrow evening.

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

drewg13 commented 11 months ago

Oops, didn't mean that to happen.

drewg13 commented 11 months ago

Gotta say, I don't love the opinionated reformatting of unrelated code in sav.js. I'm sure it's something your IDE does and I'm sure the way I do it is dumb, but it's a bit grating on the nerves to have your code arbitrarily rewritten.

I'll revert the V10 deprecation changes you made to en.json and the reformatting and it should be good to merge.

MarkPearce commented 11 months ago

Sorry it’s automatic on save with prettier. If you want to share your config I can resubmit

On Thu 16. Nov 2023 at 00:26, drewg13 @.***> wrote:

Gotta say, I don't love the opinionated reformatting of unrelated code in sav.js. I'm sure it's something your IDE does and I'm sure the way I do it is dumb, but it's a bit grating on the nerves to have your code arbitrarily rewritten.

I'll revert the V10 deprecation changes you made to en.json and the reformatting and it should be good to merge.

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

MarkPearce commented 11 months ago

The v10 changes are fines, tested it on 10.312

On Thu 16. Nov 2023 at 00:27, Mark Pearce @.***> wrote:

Sorry it’s automatic on save with prettier. If you want to share your config I can resubmit

On Thu 16. Nov 2023 at 00:26, drewg13 @.***> wrote:

Gotta say, I don't love the opinionated reformatting of unrelated code in sav.js. I'm sure it's something your IDE does and I'm sure the way I do it is dumb, but it's a bit grating on the nerves to have your code arbitrarily rewritten.

I'll revert the V10 deprecation changes you made to en.json and the reformatting and it should be good to merge.

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

drewg13 commented 11 months ago

No, I took care of it all, no worries. I saw you made the V10 comment, but didn't realize what it was referring to. My testing shows the same, good to know. I'll revert that then.

drewg13 commented 11 months ago

Nope, my mistake, I hadn't reverted before testing, that change will have to wait until my first V12 release. It doesn't break anything in terms of actual errors, but it no longer translates the entries in the Create New Item or Create New Actor type dropdowns. It's not a huge issue for English speakers, as you just get the type keys in lowercase with underscores, but I do have foreign users for whom it would break their translations in V10.

image

MarkPearce commented 11 months ago

Oh.... it just uses the type name instead of the actual string value, tricked me on my test. Good catch!

I normally leave programming to developers. :p

On Thu, Nov 16, 2023 at 12:50 AM drewg13 @.***> wrote:

Nope, my mistake, I hadn't reverted before testing, that change will have to wait until my first V12 release. It doesn't break anything in terms of actual errors, but it no longer translates the entries in the Create New Item or Create New Actor type dropdowns. It's not a huge issue for English speakers, as you just get the type keys in lowercase with underscores, but I do have foreign users for whom it would break their translations in V10.

[image: image] https://user-images.githubusercontent.com/1455026/283279331-29ea31e1-cc05-4e81-830d-d3e32777e2af.png

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

drewg13 commented 11 months ago

I get it, I hate deprecation warnings in my console too. I just have (or had at one point) a significant userbase in Brazil who worked really hard with me to get all of the translation stuff cleaned up and functional.

You're better at programming than I am, I just happen to be a subject matter expert on my own system, having already made these mistakes once or twice (or thrice) before. I do really appreciate the contribution! Feel free to fix my f-ed up CSS and/or pretty up the sheets any time you like.

MarkPearce commented 11 months ago

localization of strings may not be great for the faction sheet. My CSS is terrible, so there's fixed width columns in there....

Is there a portuguese language file i could test with?

Why don't I add in text-overflow: ellipsis To prevent overlapping?

On Thu, Nov 16, 2023 at 1:04 AM drewg13 @.***> wrote:

I get it, I hate deprecation warnings in my console too. I just have (or had at one point) a significant userbase in Brazil who worked really hard with me to get all of the translation stuff cleaned up and functional.

You're better at programming than I am, I just happen to be a subject matter expert on my own system, having already made these mistakes once or twice (or thrice) before. I do really appreciate the contribution! Feel free to fix my f-ed up CSS and/or pretty up the sheets any time you like.

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

drewg13 commented 11 months ago

There are modules for Portuguese and Spanish that add in Babele support for translation of the compendium content as well. You can find the links in the README. They're both probably out of date though.