feat: apply edit mode to sheet #667

Moonpile · 2023-11-25T21:28:36Z

No description provided.

Moonpile · 2023-11-28T04:31:57Z

src/actors/rqgActorSheet.ts

@@ -1403,6 +1403,39 @@ export class RqgActorSheet extends ActorSheet<
        (createdItems[0] as RqgItem)?.sheet?.render(true);
      });
    });
+


@wake42 these sections are the same or similar and could be generic if runes, skills, and passions all had the same interface or just the same properties. I think this could be a good time to make a change for the future to support occupations and cultures: (we could also finish this issue and do what I'm suggesting in another issue or as a part of the occupations and cultures changes)

.baseChance (for passions this would be 60, for skills it would vary)
.modifiers (maybe this is an array of key-value pairs? Occupations and cultures and such will add permanent bonuses here)
.gainedChance (or maybe we could call it .experience or .experienceGain)
.categoryMod (only for skills, maybe even refactor this to just be another entry in modifiers)
.chance (the sum of the previous three)

Not sure it makes sense to have a baseChance on runes & passions, especially not power runes that are matched against another rune.
The data-rune-grid-edit &. data-passion-grid-edit is the same thing though, I agree. For occupation / culture using this you mean there should be a modifier field for all these items that can be modified by those items, either by active effect or raw changing when you embed an occupation for example?
I guess we could implement a key-value structure and have a modifiers array, but arrays are not that easy to handle in Foundry dataModels I think. So if you wanted to link one of those to an input field it would take some extra code to accomplish.
Talking about datamodels that is also work that needs to be done - migrating the current definitions to use the datamodels Foundry uses: #596
All in all I'm not sure it's good to squeeze all these items into the same mold - they are different I think (even if they are similar)

wake42

In play mode I think we should hide the characteristics formula for everyone.

Maybe we should show the "Edit Rune Magic Spell" context menu for trusted players.

Should we only have one input field for runes & passions? Is it confusing with double percentages? Not sure... For skills it's good I think.

wake42 · 2023-11-28T06:24:44Z

src/actors/_actorsheets.scss

@@ -124,7 +124,7 @@
        .characteristics {
          grid-row: 2;
          grid-column: name-rune-start / name-rune-end;
-
+          width: 100%;


This doesn't seem to do anything. Remove?

wake42 · 2023-11-28T06:36:52Z

src/actors/_actorsheets.scss

@@ -678,81 +678,94 @@
    }
  }

-  input.characteristic-rank-high {
+  *.characteristic-rank-high {


Remove the wildcard (here and on the following rank css rules)

Suggested change

*.characteristic-rank-high {

.characteristic-rank-high {

wake42 · 2023-11-28T18:07:18Z

src/actors/_actorsheets.scss

    }

-    label {
+    .value {
      font-size: min(28px, 3cqw);
      font-weight: bold;


Part of avoiding content switching positions

margin-top: -5px;

I put this in and don't know if it did anything. I thought you were talking about how the characteristics row is slightly taller in one view than the other.

Moonpile · 2023-11-28T19:11:36Z

In play mode I think we should hide the characteristics formula for everyone.

I can hide the dice expression formulas in play mode. The green-red indicators are on that dice expression box. As a gm I still want to see those in play mode so I know if the NPC is, say, large for its type, or weak for its type.

Maybe we should show the "Edit Rune Magic Spell" context menu for trusted players.

I can do this, but should I also do it for Spirit Magic? I'm not sure when even a trusted player is really going to need to edit either though, except for the spirit spell focus field which is always available.

Should we only have one input field for runes & passions? Is it confusing with double percentages? Not sure... For skills it's good I think.

See my comment above about making runes, skills, and passions have similar fields so that we can reduce some handlers, but more importantly because I think it will be helpful to the later steps of the character creation process when you drag an occupation or tribe or culture onto your actor. I think it would be best if it added the bonuses to the .modifiers property I'm proposing. I also think that even for passions it make sense to have a .baseChance of 60, a .gainedChance so you know how much a player has gained if for no other reason. On runes I want the .gainedChance because it will make it easier to determine if a player has correctly added their starting rune amounts (if we want to do validation of character creation).

wake42 · 2023-11-28T20:09:02Z

I can hide the dice expression formulas in play mode. The green-red indicators are on that dice expression box. As a gm I still want to see those in play mode so I know if the NPC is, say, large for its type, or weak for its type.

I was thinking that maybe we could have an individual setting for if you want to see the green-red indicator. That way each player / GM could decide for themselves if they want it.

Maybe we should show the "Edit Rune Magic Spell" context menu for trusted players.

I can do this, but should I also do it for Spirit Magic? I'm not sure when even a trusted player is really going to need to edit either though, except for the spirit spell focus field which is always available.

You're right - lets leave it as it is now.

Should we only have one input field for runes & passions? Is it confusing with double percentages? Not sure... For skills it's good I think.

See my comment above about making runes, skills, and passions have similar fields so that we can reduce some handlers, but more importantly because I think it will be helpful to the later steps of the character creation process when you drag an occupation or tribe or culture onto your actor. I think it would be best if it added the bonuses to the .modifiers property I'm proposing. I also think that even for passions it make sense to have a .baseChance of 60, a .gainedChance so you know how much a player has gained if for no other reason. On runes I want the .gainedChance because it will make it easier to determine if a player has correctly added their starting rune amounts (if we want to do validation of character creation).

Let's keep this as it is, and see where we land on this. It has a lot to do with character building as you say.

Moonpile · 2023-11-28T20:30:22Z

I can hide the dice expression formulas in play mode. The green-red indicators are on that dice expression box. As a gm I still want to see those in play mode so I know if the NPC is, say, large for its type, or weak for its type.

I was thinking that maybe we could have an individual setting for if you want to see the green-red indicator. That way each player / GM could decide for themselves if they want it.

Ok, I'll do that tonight. So would this be a new property on the actor like .showCharacteristicRatings?

Moonpile · 2023-11-28T20:31:05Z

And generally I think we should see how this all works and we'll probably get feedback from users on "this is visually confusing" or "normal players should have access to X because of Y"

Moonpile · 2023-11-29T04:32:00Z

@wake42 made a context menu for Show/Hide Characteristic Ratings and store the preference in actor.system.showCharacteristicRatings

So there was the thing with margin-top that you commented. If that was intended to fix the difference in line height between displaying inputs for the characteristics and just showing the value in a div, it didn't seem to fix it. I think we need to force the div that contains the input or div to always have a height of, say, 110px which is a bit taller than either, then center the contents (div or input).

wake42 · 2023-11-29T06:34:41Z

So there was the thing with margin-top that you commented. If that was intended to fix the difference in line height between displaying inputs for the characteristics and just showing the value in a div, it didn't seem to fix it. I think we need to force the div that contains the input or div to always have a height of, say, 110px which is a bit taller than either, then center the contents (div or input).

Maybe the easiest way is to always show the input and add pointer-events: none; if it's not editable. Matching the height of a text to an input is hard, and the input will shrink if you make the sheet more narrow. The characteristics inputs doesn't look like an input anyway.

wake42

I think the characteristics formula shouldn't be shown in play mode (for anyone, not even GM).

wake42 · 2023-12-02T19:32:04Z

src/i18n/en/uiContent.json

@@ -1176,6 +1174,10 @@
      "WorldMigrationVersion": {
        "settingName": "World Migration Version",
        "settingHint": "The RQG system version this world has been migrated to."
+      },
+      "SchowCharacteristicRatings": {


Misspelling

wake42 · 2023-12-02T20:30:00Z

src/actors/rqgActorSheet.ts

@@ -223,6 +223,8 @@ export class RqgActorSheet extends ActorSheet<
      isEditable: this.isEditable,
      isGM: getGameUser().isGM,
      isPC: this.actor.hasPlayerOwner,
+      showCharacteristicRatings:
+        (getGame().settings.get(systemId, "showCharacteristicRatings") as boolean) || false,


You don't need to cast the type if you register the type of the setting in

fvtt-system-rqg/src/global.d.ts

Line 47 in eabfc02

namespace ClientSettings {

wake42 · 2023-12-02T20:43:16Z

src/i18n/en/uiContent.json

+      },
+      "SchowCharacteristicRatings": {
+        "settingName": "Show Characteristic Ratings",
+        "settingHint": "Show a colored rating bar below characteristics which rates the value as compared to the dice expression formula for the characteristic.  Red is lower than average and green is higher than average.  This can help GMs see an NPC's strengths and weaknesses at a glance."


The last sentence doesn't make sense when a player see it. Maybe could be reformulated to something along the lines of This can help judging an actors strengths and weaknesses at a glance.

Fixed and fixed the spelling of the setting

…fields-in-Character-Creation-Mode-#635

…haracter-Creation-Mode-#635' of https://github.com/sun-dragon-cult/fvtt-system-rqg into Moonpile/Ensure-Players-can-edit-the-needed-fields-in-Character-Creation-Mode-#635

wake42

Lets merge this and any details can be looked at later.

Moonpile added 11 commits November 25, 2023 16:27

feat: apply edit mode to sheet

1b1c915

feat: apply edit mode to sheet

0e36604

feat: apply edit mode to sheet

2561b24

feat: apply edit mode to sheet

cd52e09

feat: apply edit mode to sheet

29655ee

feat: apply edit mode to sheet

3b39108

feat: apply edit mode to sheet

346f6e4

feat: apply edit mode to sheet

1baf0c6

feat: apply edit mode to sheet

9e7af42

feat: apply edit mode to sheet

c69d798

feat: apply edit mode to sheet

91e73d7

Moonpile commented Nov 28, 2023

View reviewed changes

Moonpile marked this pull request as ready for review November 28, 2023 04:38

wake42 requested changes Nov 28, 2023

View reviewed changes

feat: apply edit mode to sheet

54cafa4

feat: apply edit mode to sheet

6dac3df

feat: apply edit mode to sheet

574dea7

wake42 reviewed Dec 2, 2023

View reviewed changes

Moonpile added 5 commits December 3, 2023 14:22

feat: apply edit mode to sheet

facca1b

Merge branch 'main' into Moonpile/Ensure-Players-can-edit-the-needed-…

660f490

…fields-in-Character-Creation-Mode-#635

feat: apply edit mode to sheet

07d0df0

Merge branch 'Moonpile/Ensure-Players-can-edit-the-needed-fields-in-C…

0448b7e

…haracter-Creation-Mode-#635' of https://github.com/sun-dragon-cult/fvtt-system-rqg into Moonpile/Ensure-Players-can-edit-the-needed-fields-in-Character-Creation-Mode-#635

feat: apply edit mode to sheet

c35d4b1

Moonpile requested a review from wake42 December 3, 2023 22:43

feat: apply edit mode to sheet

42660db

wake42 approved these changes Dec 3, 2023

View reviewed changes

Moonpile linked an issue Dec 3, 2023 that may be closed by this pull request

GM UI for Character Creation Mode #633

Closed

Moonpile merged commit d6361c9 into main Dec 3, 2023
1 check passed

Moonpile deleted the Moonpile/Ensure-Players-can-edit-the-needed-fields-in-Character-Creation-Mode-#635 branch December 3, 2023 23:20

github-actions bot mentioned this pull request Dec 3, 2023

chore(main): release 3.4.0 #662

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: apply edit mode to sheet #667

feat: apply edit mode to sheet #667

Moonpile commented Nov 25, 2023

Moonpile Nov 28, 2023

wake42 Nov 28, 2023

wake42 left a comment

wake42 Nov 28, 2023

Moonpile Nov 28, 2023

wake42 Nov 28, 2023

Moonpile Nov 28, 2023

wake42 Nov 28, 2023

Moonpile Nov 28, 2023

Moonpile commented Nov 28, 2023

wake42 commented Nov 28, 2023

Moonpile commented Nov 28, 2023

Moonpile commented Nov 28, 2023

Moonpile commented Nov 29, 2023

wake42 commented Nov 29, 2023

wake42 left a comment

wake42 Dec 2, 2023

Moonpile Dec 3, 2023

wake42 Dec 2, 2023

Moonpile Dec 3, 2023

wake42 Dec 2, 2023

Moonpile Dec 3, 2023

wake42 left a comment

feat: apply edit mode to sheet #667

feat: apply edit mode to sheet #667

Conversation

Moonpile commented Nov 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wake42 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Moonpile commented Nov 28, 2023

wake42 commented Nov 28, 2023

Moonpile commented Nov 28, 2023

Moonpile commented Nov 28, 2023

Moonpile commented Nov 29, 2023

wake42 commented Nov 29, 2023

wake42 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wake42 left a comment

Choose a reason for hiding this comment