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

[#3629, #4142] Add units to container capacity, add volume capacity #4912

Merged
merged 2 commits into from
Jan 7, 2025

Conversation

arbron
Copy link
Collaborator

@arbron arbron commented Dec 20, 2024

Reworks container capacity to allow for defining more than one capacity type (so a container can define a maximum number of items and a maximum weight). Weight capacity now accepts actual units so the weight can be measured in kilograms or tonnes if desired.

Container Units

Adds a volume capacity type to containers for measuring in cubic feet or liters. This includes the addition of volumeUnits to the configuration and a "Use Metric Volume Units" setting to match the other units settings.

Closes #3629
Closes #4142

Reworks container capacity to allow for defining more than one
capacity type (so a container can define a maximum number of items
and a maximum weight). Weight capacity now accepts actual units
so the weight can be measured in kilograms or tonnes if desired.

Adds a volume capacity type to containers for measuring in cubic
feet or liters. This includes the addition of `volumeUnits` to
the configuration and a "Use Metric Volume Units" setting to match
the other units settings.

Closes #3629
Closes #4142
@arbron arbron added this to the D&D5E 4.2.0 milestone Dec 20, 2024
@arbron arbron self-assigned this Dec 20, 2024
module/config.mjs Outdated Show resolved Hide resolved
@arbron arbron requested a review from Fyorl January 7, 2025 17:47
Copy link
Contributor

@Fyorl Fyorl left a comment

Choose a reason for hiding this comment

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

See my comment about multiple container capacities, but we can move ahead with this for now.

lang/en.json Outdated Show resolved Hide resolved
module/config.mjs Outdated Show resolved Hide resolved
module/data/item/container.mjs Outdated Show resolved Hide resolved
@@ -275,14 +305,15 @@ export default class ContainerData extends ItemDataModel.mixin(
* @returns {Promise<Item5eCapacityDescriptor>}
Copy link
Contributor

Choose a reason for hiding this comment

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

This will need some future work I think. If we're changing container capacity from being exclusively items or weight, to inclusively items, weight, or volume, that implies to me that a container with item and weight capacities set would be considered exceeding its capacity when either of those two thresholds are met. So we no longer have a single capacity value, but instead multiple.

We could probably render multiple bars on the container sheet. That should look ok, I think, and this behaviour is something you opt-into by having both of these values filled-in. But either way, some of these internal capacity method signatures will need to change.

@arbron arbron merged commit a69bf73 into 4.2.x Jan 7, 2025
@arbron arbron deleted the container-capacity branch January 7, 2025 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants