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

[#3958] Improve localization of rest messages #4749

Open
wants to merge 1 commit into
base: rest/dialogs
Choose a base branch
from

Conversation

arbron
Copy link
Collaborator

@arbron arbron commented Nov 18, 2024

Updates the rest message to select the proper plural form of Hit Points and Hit Dice to match the number recovered.

Also adjusts the flavor to automatically display the rest time in the message flavor, rather than relying on hard-coded labels for each rest variant. This ensures the flavor will properly display the rest time even if it doesn't exactly match the default values.

In order to support this as well as improve other time-based localization in the future, this PR adds two new utlity methods:

  • convertTime converts a time from one set of units to another. It will also attempt to pick the best unit automatically if the to units are not defined, selecting the largest unit that can fully represent the value (with some wiggle room so it will display 90 minutes, rather than 1.5 hours).
  • formatTime converts a time value and units into a string for display, relying on the system-provided plural localization if available and if not using Javascript's standard unit formatting for numbers.

Updates the rest message to select the proper plural form of Hit
Points and Hit Dice to match the number recovered.

Also adjusts the flavor to automatically display the rest time in
the message flavor, rather than relying on hard-coded labels for
each rest variant. This ensures the flavor will properly display
the rest time even if it doesn't exactly match the default values.

In order to support this as well as improve other time-based
localization in the future, this PR adds two new utlity methods:
- `convertTime` converts a time from one set of units to another.
  It will also attempt to pick the best unit automatically if the
  `to` units are not defined, selecting the largest unit that can
  fully represent the value (with some wiggle room so it will
  display 90 minutes, rather than 1.5 hours).
- `formatTime` converts a time value and units into a string for
  display, relying on the system-provided plural localization if
  available and if not using Javascript's standard unit formatting
  for numbers.
@@ -426,10 +454,44 @@ export function getSceneTargets() {
/* Conversions */
/* -------------------------------------------- */

/**
* Convert the provided time value to another unit. If not final unit is provided, then will convert it to the largest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Convert the provided time value to another unit. If not final unit is provided, then will convert it to the largest
* Convert the provided time value to another unit. If ot final unit is provided, then will convert it to the largest

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you deleted the wrong letter there :)

* @param {string} from The initial unit as defined in `CONFIG.DND5E.timeUnits`.
* @param {object} [options={}]
* @param {boolean} [options.combat=false] Use combat units when auto-selecting units, rather than normal units.
* @param {string} [options.to] The final units, is explicitly provided.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param {string} [options.to] The final units, is explicitly provided.
* @param {string} [options.to] The final units, if explicitly provided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants