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

Lower volume of MRE packages #66319

Merged
merged 6 commits into from
Oct 9, 2023
Merged

Conversation

azraneth
Copy link
Contributor

@azraneth azraneth commented Jun 19, 2023

Summary

Bugfixes "Lower volume of MRE packages"

Purpose of change

MRE Packages are ~2.4 L in volume and ~800g in weight. In game, they are instead over 3 L weighing approximately 1500g. This is unrealistic and also inconveniences the player since they don't really fit in pockets they're supposed to fit in.

Describe the solution

The MRE specific container beverage_bag is a non-rigid container with a base volume of 0.26 L and a pocket of 0.25 L, meaning a filled bag is twice the volume it's meant to be. This change migrates beverage_bag into zipper_bag, since every image I could find of an MRE beverage bag also included a zipper, meaning they're functionally identical to zipper_bag,

Describe alternatives you've considered

Keeping beverage_bag as a near-copy of zipper_bag, but figured I might as well reduce bloat while I was at it.

Also reducing weight, but could not find measurements on reasonable weights on the containers or their contents.

Testing

Spawned in zombie soldiers and killed them to drop MRE packages and made sure nothing spilled and MREs still spawn sealed when full.
Teleported to military surplus and observed a spawned full MRE package
Loaded in a save that had beverage bags to make sure they migrated properly.

@Karol1223
Copy link
Contributor

Karol1223 commented Jun 19, 2023

I don't see why you're removing the beverage bag? It is a real item. Just because it's not used in MREs doesn't mean it shouldn't be in the game. It could find use elsewhere. Even if they're basically identical having it migrated into a variant would make more sense, though it'd take more work from you than just leaving it be. We're not running out of JSON anytime soon - if it exists irl no reason not to have it ingame.

@azraneth
Copy link
Contributor Author

azraneth commented Jun 19, 2023

The reason to remove it in my eyes is because as it is, it is a zipper bag, although the current version lacks the zipper. If someone wants to use a non-sealable water tight 0.25L (technically the MRE bag is 0.3 L, but it's easier to round for units) plastic bag it's no effort adding one either way, but having an unused non-spawning item seems like it'd be no good to anyone. Having them be the same item helps for the future in case of recipes wanting to use zipper bags and the like.

@Karol1223
Copy link
Contributor

Karol1223 commented Jun 19, 2023

Regardless of our opinions your PR also doesn't migrate anything. You just removed an item without migrating or obsoleting it. This will cause errors in saves that already have this item.

@azraneth
Copy link
Contributor Author

That's a valid point, I'd love to take a guide on how to migrate items properly.

@github-actions github-actions bot added <Bugfix> This is a fix for a bug (or closes open issue) [JSON] Changes (can be) made in JSON Items: Containers Things that hold other things Spawn Creatures, items, vehicles, locations appearing on map astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Jun 19, 2023
@GuardianDll
Copy link
Member

https://github.com/CleverRaven/Cataclysm-DDA/blob/master/doc/JSON_INFO.md#obsoletion-and-migration

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jun 19, 2023
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions and removed json-styled JSON lint passed, label assigned by github actions labels Jun 19, 2023
@casswedson
Copy link
Contributor

do make sure the full mres still spawn sealed; due to some eh... code stuff only full containers spawn sealed, gotta match the container's capacity and the content's volume to the ml

@bombasticSlacks
Copy link
Contributor

do make sure the full mres still spawn sealed; due to some eh... code stuff only full containers spawn sealed, gotta match the container's capacity and the content's volume to the ml

agreed, changing MRE info can lead to some oddities.

makes sure full MREs drop sealed
@azraneth
Copy link
Contributor Author

azraneth commented Jun 21, 2023

After a shrinking of 1ml (turns out 2*4 is 8 and not 7) things do spawn sealed again.

@Maleclypse
Copy link
Member

I'm confused, none of the bags in an MRE are resealable from what I know. So adding zipper bags seems off.

@azraneth
Copy link
Contributor Author

Specifically beverage bags made for being filled with water, a heater, and a flavoring packet are resealable, since otherwise they wouldn't work very well ^^. The only reason I'd see to not make them just a zipper bag is for them to be more durable in some way since they are military grade, but afaik there's no reason for that either.

@Maleclypse
Copy link
Member

Cool deal. Can you rebase or otherwise resolve the conflicts that have occurred? Then this will be ready for final review/merge.

@kevingranade kevingranade force-pushed the bloated_bags branch 3 times, most recently from 77a238a to 7e795ed Compare October 9, 2023 06:33
@github-actions github-actions bot removed json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Oct 9, 2023
@github-actions github-actions bot added BasicBuildPassed This PR builds correctly, label assigned by github actions json-styled JSON lint passed, label assigned by github actions and removed BasicBuildPassed This PR builds correctly, label assigned by github actions labels Oct 9, 2023
@kevingranade kevingranade merged commit 114ad9f into CleverRaven:master Oct 9, 2023
19 of 23 checks passed
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Oct 9, 2023
detahramet pushed a commit to detahramet/Cataclysm-DDA that referenced this pull request Nov 6, 2023
* migrates MRE beverage bags into zipper bags
* fix mre package volume
makes sure full MREs drop sealed

Co-authored-by: Kevin Granade <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) Items: Containers Things that hold other things [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Spawn Creatures, items, vehicles, locations appearing on map
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants