Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Ars Magica 5th] improvements and new 1.6 version #8094

Merged
merged 25 commits into from Feb 10, 2021

Conversation

Riernar
Copy link
Contributor

@Riernar Riernar commented Jan 17, 2021

Changes / Comments

  • Corrected some errors in the CSS file that the IDE was complaining about, removed (seemingly) unused CSS
  • Added a python script generated HTM with python directly inside an HTML template
    • allows generating repeating parts of the sheet with loops and helpers
    • edited readme.md to explain how things work. See also fileval.py help
  • Sorted translation keys by alphabetical order to simplify searches in the translation file
  • Wrapped each reference to the fatigue attribute in floor() (fix [Ars Magica 5] .1 decimal in fatigue rolls #6783 )
  • Added inline labels to rolls provided by the sheet
    • added new translation keys encumbrance, circumstances-m
    • added new translation attributes (all ending in _i18n), added them to the translation sheet worker
    • used deferred attribute lookup for abilities, so that the characteristic name is available in the roll
  • Corrected the name of the ritual roll button from roll_Formulaic to roll_Ritual
  • Corrected the name of the soak attribute from attr_Init to attr_Soak
  • Added a button to roll critical stress die, next to the botch and reroll button
    • added a critical translation key
  • Copied the aura input just above the spells in page 5. Makes it easier to change it before casting, and clearer that it is taken into account
  • fixed [Ars Magica 5] Sheet tabs are rendered as plain radio buttons #8116
    • added new translation keys magic and help
  • Added field for soak bonus not coming from armour (spells, virtues, qualities etc...)
    • added new translation key soak-bonus
  • Added simple and stress rolls distinctions
    • added two roll buttons for every roll that can be simple or stress
    • added config dropdown for toggling between buttons or having both
    • simple rolls are display grey/white (dark/light)
    • stress rolls are displayed brown/Khaki, red when a botch, gold when a critical
  • Added a 6th page for sheet documentation and configuration
  • Added an option to group botch rolls and simply input the number of botch dice, instead of having a drop-down
  • Added support for additional fatigue levels
  • Added checkbox for focus
    • added focus translation key
  • Added dropdown for spellcasting gestures and words bonuses
  • Added button to roll botch/critical directly from the chat in rolltemplate
  • Fixed Ability rolltemplate being too wide sometimes
  • Added Alerts to notify player of data loss or important update informations
  • Slight clarification of damages in the attack rolltemplate

Todo

  • Rename attributes which value have been modified to protect existing sheet from breaking
  • Add button to easily handle focus on magic rolls
  • Support additional fatigue levels (there multiple ways to get those)
  • Fix computation of result on botch on stress die
    • make distinction between simple die (1d10) and stress die (1d10-1)
    • add choice selection: double each relevant roll button, or add roll query
  • Make next level and total xp fields readonly
  • Add ability button in rolltemplates to call botch & critical roll button from the chat (under work)
  • Fix the ability rolltemplate width being completely havoc
  • Test the changes in a new game
  • Test the changes with old characters (imported them)

Roll20 Requests

Comments are very helpful for reviewing the code changes. Please answer the relevant questions below in your comment.

  • Does the pull request title have the sheet name(s)? Include each sheet name.
  • Is this a bug fix?
  • Does this add functional enhancements (new features or extending existing features) ?
  • Does this add or change functional aesthetics (such as layout or color scheme) ?
  • If changing or removing attributes, what steps have you taken, if any, to preserve player data ?
    • experience fields are calculated by sheet-worker and will update accordingly
    • abilities' characteristic changed how they work to make the name available. The attribute name has been changed to avoid incompatibilities, but the data cannot be conserved
    • added alerts to notify players of data losses
  • If this is a new sheet, did you follow Building Character Sheets standards ?
    • Not a new sheet, but I tried

@Riernar
Copy link
Contributor Author

Riernar commented Jan 17, 2021

@Medieve I changed a bunch of <option> tags in abilities, rolls ... Do you know what will happen when loading previous characters with the modified HTML ? Will it fall back to the first option, or fail entirely ?

@Riernar Riernar force-pushed the arm5-improvements branch 4 times, most recently from 46c72c5 to 81b174b Compare January 17, 2021 18:50
@Medieve
Copy link
Contributor

Medieve commented Jan 17, 2021

@Medieve I changed a bunch of <option> tags in abilities, rolls ... Do you know what will happen when loading previous characters with the modified HTML ? Will it fall back to the first option, or fail entirely ?

I am going to make assumptions that it will be selecting the option which shares the value that the character sheet ability has stored. If there is no option that matches it will show none of the options selected but still carry the old value.

@Riernar
Copy link
Contributor Author

Riernar commented Jan 18, 2021

I think I found way to handle stress dies in roll20:
ceil(((((1d10cs1cf10 %9) -1)+9) %9) * 1.1)

  • is coloured red on a 10 (possible botch)
  • is coloured green on a 1 (critical)
  • if the die comes up on 10, the final value is 0 (correct total even if no botch die actually botch)
  • if the die comes up on 1, the final value is 0, making it easier to add the exploding dice rolled afterwards
    See the table below as to why this work

@Medieve do you think it is a good idea to ask which dice to roll with a roll query (when relevant), and remove the CSS you had for colouring the rolls since this can be done with the cs and cf dice specifiers ? Perhaps I should double each roll button instead, with e.g. a blue one for simple die and a red one for stress die, to avoid having two pop-up on each roll ?

I hope this can be hidden away from the user in a nested inline roll and still keeps proper colouring.

Computation table

1d10 %9 -1 +9) %9 *1.1 ceil() Note
1 1 0 0 0 0 Easier to add the exploding dice
2 2 1 1 1.1 2
3 3 2 2 2.2 3
4 4 3 3 3.3 4
5 5 4 4 4.4 5
6 6 5 5 5.5 6
7 7 6 6 6.6 7
8 8 7 7 7.7 8
9 0 -1 8 8.8 9
10 1 0 0 0 0 botched total is correct

Note: The +9) %9 is required because roll20 % operator keeps the same signs as the left operand (i.e. -1 % 9 = -1 instead of the desired 1)

@mperes
Copy link
Contributor

mperes commented Jan 18, 2021

Hello @Riernar you pull requests is failing validation because it contains modifications to a root file (.gitignore). Even though it is a good idea to ignore files like ds_store, the way to handle this while keeping it compatible with Roll20 rules would be to use the second or third method described in this article: https://docs.github.com/en/github/using-git/ignoring-files

@Riernar
Copy link
Contributor Author

Riernar commented Jan 18, 2021

Hi @Medieve, sorry to bother you again. I added a button for rolling the peculiar exploding dices of Ars Magica 5 and changed the colours of the reroll button you added to keep some sort of color scheme between the button (reddish botch, teal reroll and green critical).
What do you think of the new colours ? Are you alright with the modification of the reroll button ?

@roll20deploy
Copy link
Contributor

Character Sheet Info Roll20 Internal Use only.

@Riernar
Copy link
Contributor Author

Riernar commented Jan 18, 2021

Hi @mperes , thank you for the help ! I moved the .gitignore for __pycache__ into the local directory to pass the checks.
I kept a .gitignore so that other contributors who run the generation script do not inadvertently commit cached python binaries.

@Riernar Riernar changed the title [WIP] Ars Magica 5th improvements [WIP - do not merge] Ars Magica 5th improvements Jan 18, 2021
@Medieve
Copy link
Contributor

Medieve commented Jan 18, 2021

Maybe instead of a query for every roll or double-rolling, you might try adding a radio-toggle next to the Roll/Botch buttons whose value is the two different dice-expressions and that is injected into the various rollers. Cutting down on the number of steps to make a roll is high on the priority I think.

The dice expression looks very impressive.

The Critical/Explosion background looks fine. I might make it a yellow-gold but that's just because you asked.

I think I found way to handle stress dies in roll20:
ceil(((((1d10cs1cf10 %9) -1)+9) %9) * 1.1)

  • is coloured red on a 10 (possible botch)
  • is coloured green on a 1 (critical)
  • if the die comes up on 10, the final value is 0 (correct total even if no botch die actually botch)
  • if the die comes up on 1, the final value is 0, making it easier to add the exploding dice rolled afterwards
    See the table below as to why this work

@Medieve do you think it is a good idea to ask which dice to roll with a roll query (when relevant), and remove the CSS you had for colouring the rolls since this can be done with the cs and cf dice specifiers ? Perhaps I should double each roll button instead, with e.g. a blue one for simple die and a red one for stress die, to avoid having two pop-up on each roll ?

I hope this can be hidden away from the user in a nested inline roll and still keeps proper colouring.

Computation table

1d10 %9 -1 +9) %9 *1.1 ceil() Note
1 1 0 0 0 0 Easier to add the exploding dice
2 2 1 1 1.1 2
3 3 2 2 2.2 3
4 4 3 3 3.3 4
5 5 4 4 4.4 5
6 6 5 5 5.5 6
7 7 6 6 6.6 7
8 8 7 7 7.7 8
9 0 -1 8 8.8 9
10 1 0 0 0 0 botched total is correct
Note: The +9) %9 is required because roll20 % operator keeps the same signs as the left operand (i.e. -1 % 9 = -1 instead of the desired 1)

@Medieve
Copy link
Contributor

Medieve commented Jan 19, 2021

Can you add a note to template.html that points to the readme so future authors know how to work with the python integration?

@Riernar
Copy link
Contributor Author

Riernar commented Jan 19, 2021

I like the toggle and reducing the number of steps in a roll. I was thinking of having some rolls always be a stress die (e.g. combat rolls, spontaneous casting...), but it would be inconsistent, so I need to think about it.

I tried to keep the colour of the critical button and background in line with the colour of a critical roll in roll20, but gold is nice too, maybe I can get both to be gold.

I'll add a comment to template.html.

Thanks for your input !

@Anduh
Copy link
Contributor

Anduh commented Jan 19, 2021

Hello @Riernar you pull requests is failing validation because it contains modifications to a root file (.gitignore). Even though it is a good idea to ignore files like ds_store, the way to handle this while keeping it compatible with Roll20 rules would be to use the second or third method described in this article: https://docs.github.com/en/github/using-git/ignoring-files

Thanks for providing a clear solution to this problem! I've update the wiki guide to include your recommendation. https://wiki.roll20.net/Building_Character_Sheets#Submitting_a_character_sheet_for_public_use

gitignore

@GuillaumeDIDIER
Copy link

GuillaumeDIDIER commented Jan 19, 2021

Maybe instead of a query for every roll or double-rolling, you might try adding a radio-toggle next to the Roll/Botch buttons whose value is the two different dice-expressions and that is injected into the various rollers. Cutting down on the number of steps to make a roll is high on the priority I think.
[...]

I think the radio toggle is likely to cause mistakes. People will not pay attention to that setting when making a roll and they will eventually make a stress roll when meaning a simple roll, ou a stress roll when it should have been a simple one.

From a User Experience point of view, you need to make it so the user thinks for each roll whether it is a stress or simple roll.

An ideal (but technically not possible due to r20 apparently), would have been to include in the pop-up roll stress and roll simple button, in addition to the bonus.

Two pop-ups seems very heavy handed, and thus would be a poor experience.

My personal suggestion would clutter a bit more the character sheet, but would include a simple and a stress roll button (duplicate the roll button) for all the dice buttons that can be stress or non stress. Those could be distinguished using their colour (or some specific symbol). People would see two dice buttons and will pick the correct one, making way less mistakes than if they needed to remember the global toggle.

@GuillaumeDIDIER
Copy link

I also think that the stress/simple nature of roll that must be simple, or must be stress should be enforced. (Which results in only one die button in my proposed solution).

@Riernar
Copy link
Contributor Author

Riernar commented Jan 19, 2021

@GuillaumeDIDIER @Medieve
Regarding simple/stress die toggle, I think the toggle and the "the user has to think which dice type he wants" requirement are not incompatible: I can add a colour identity to simple and stress die like that:

  • the button for a simple die is plain
  • results from simple dice are always in black (no criticals). This can be achived by disabling critical success and fumble formatting with 1d10cf0cs0 (1d10 where critical success and failure is 0, which is impossible)
  • button for a stress die is gold (it is just a character from a custom font, so I hope i can colour it as text)
  • results from stress die are:
    • red if a botch (achieved with the cf10 in the roll, marking 10 as a critical failure. The formula designed above makes the final value 0, in line with the rules)
    • light blue if a standard result (the colour currently used to mark criticals. Perhaps I should use something else to not confuse player used to the colouring scheme)
    • gold when the result is a critical (obtained by cs1 to mark 1 as a critical success, and changing the formatting of criticals as is currently done in the CSS file). The value is then 0, and the player can use the critical button to roll the critical next exploding dices
  • when switching the toggle, the colour of roll button is updated (by always hiding the unused one)

This prevents from cluttering the sheet with too many roll button, while still providing quick visual input that the dice is the wrong type

@Riernar
Copy link
Contributor Author

Riernar commented Jan 19, 2021

Hi @Medieve, sorry to bother you again. I added a button for rolling the peculiar exploding dices of Ars Magica 5 and changed the colours of the reroll button you added to keep some sort of color scheme between the button (reddish botch, teal reroll and green critical).
What do you think of the new colours ? Are you alright with the modification of the reroll button ?

Regarding the roll formula for the explosive dice, it currently has several drawbacks: the dice explode on a 10 (instead of a 1), so player using 3D dice might get confused. Moreover, the last roll (the first "not a ten") cannot be retrieved and is rerolled, but since it is a d9 (since we already know it is not a 1), it is not displayed when using 3D dice.

I haven't found a formula that perfectly reproduces the roll in roll20, but I have several options that have different advantages.
The formula is separated into two parts: one part computes the number of reroll, while the other computes the last roll that did not explode.

With the table below, which one do you think is the best option ?

explosion \ last roll 1d9+1 1d10r1
2*2**floor(1d10!!10 / 10) - explodes on 10s instead of 1s
- last roll is rerolled but this is not displayed
- explodes on 10s instead of 1s
- last roll is rerolled, this is displayed but 1s must be further rerolled
2*2**({1d10!1}=1) Explodes on 1s, but weird incompatibility between sum roll and comparison roll, so one has to be hidden in an inline roll. The last reroll is not show (it wasn't anyway) Explodes on 1s, but weird incompatibility between sum roll and comparison roll, so one has to be hidden in an inline roll. Either:
-the exploding dices are not shown
- the last reroll is not show

@Riernar
Copy link
Contributor Author

Riernar commented Jan 19, 2021

Maybe instead of a query for every roll or double-rolling, you might try adding a radio-toggle next to the Roll/Botch buttons whose value is the two different dice-expressions and that is injected into the various rollers. Cutting down on the number of steps to make a roll is high on the priority I think.

The dice expression looks very impressive.

The Critical/Explosion background looks fine. I might make it a yellow-gold but that's just because you asked.

I made the critical button and background golden. I found the botch and critical button were too close, so I made the botch button more red. What do you think @Medieve ?

@Riernar
Copy link
Contributor Author

Riernar commented Feb 5, 2021

This PR is almost feature complete. I still want to add a focus button to the laboratory section, and test how existing characters behave when ported.

I'm having a weird bug with the ability rolltemplate: the width is quite enormous regarding the content, for certain abilities names. For instant, TestTestTest is way to large and overflows out of the chat, while an ability named Test TestTest, with an additional space and thus wider, is normal.
I have no idea why this is the case, I'm afraid I dont understand CSS well enough to debug that. If a kind soul has the time to look into it, that would be incredible !

@Riernar
Copy link
Contributor Author

Riernar commented Feb 8, 2021

The issue with the ability rolltemplate is fixed

@Riernar
Copy link
Contributor Author

Riernar commented Feb 8, 2021

Tested the sheet on both firefox and chrome to make sure I didn't break anything obvious. There might be some loose ends still, but I tried to test as best as I could.

I think this is ready for PR

@Riernar Riernar changed the title [WIP - do not merge] Ars Magica 5th improvements [Ars Magica 5th] improvements and new 1.6 version Feb 8, 2021
@Riernar Riernar marked this pull request as ready for review February 8, 2021 20:13
@mperes mperes merged commit 201593c into Roll20:master Feb 10, 2021
@Riernar Riernar deleted the arm5-improvements branch February 10, 2021 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Ars Magica 5] Sheet tabs are rendered as plain radio buttons [Ars Magica 5] .1 decimal in fatigue rolls
6 participants