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

Fix corpse length #40454

Merged
merged 5 commits into from
May 14, 2020
Merged

Conversation

kevingranade
Copy link
Member

@kevingranade kevingranade commented May 11, 2020

Summary

SUMMARY: None

Purpose of change

Corpse length was overlooked in #40186 resulting in #40446

Describe the solution

Fixes #40446
Overrides length in item::length() if the item is a corpse.
While I was at it, clarify the name of the function to determine a fallback item length.

Testing

Spawn various corpses and verify they have sane lengths based on their volumes.
Added a test that asserts a minimal length for all non-liquid, non-soft items, including all corpse variants.

@kevingranade kevingranade force-pushed the fix-corpse-length branch 5 times, most recently from ae6d67f to e04b10b Compare May 11, 2020 21:23
@Erozaxx
Copy link

Erozaxx commented May 11, 2020

Hello Kevin, I did try to fix this issue before you've initiated the PR. I did not know how one can solve such conflict in GIT so I've reverted to master, applied your changes as one commit and the applied my changes as this commit [https://github.com/Erozaxx/Cataclysm-DDA/commit/30de543620cdb54693719904528a25bdfab01fff](Add possibility to define average_length for monsters in JSON)
It's first time I am contributing, so sorry if I don't know the ropes. Any comment to my changes welcome and if you think it is worth committing, I'd need some hint how to solve this branching out of your PR stuff. Another option would be to wait until it is part of the master and replicate the changes as separate PR afterwards.

@curstwist curstwist added [C++] Changes (can be) made in C++. Previously named `Code` Items / Item Actions / Item Qualities Items and how they work and interact labels May 12, 2020
@ZhilkinSerg ZhilkinSerg merged commit e6eb0ff into CleverRaven:master May 14, 2020
@ZhilkinSerg
Copy link
Contributor

Hello Kevin, I did try to fix this issue before you've initiated the PR. I did not know how one can solve such conflict in GIT so I've reverted to master, applied your changes as one commit and the applied my changes as this commit [https://github.com/Erozaxx/Cataclysm-DDA/commit/30de543620cdb54693719904528a25bdfab01fff](Add possibility to define average_length for monsters in JSON)
It's first time I am contributing, so sorry if I don't know the ropes. Any comment to my changes welcome and if you think it is worth committing, I'd need some hint how to solve this branching out of your PR stuff. Another option would be to wait until it is part of the master and replicate the changes as separate PR afterwards.

Yeah, just rebase your branch on top of current master.

@kevingranade kevingranade deleted the fix-corpse-length branch June 28, 2020 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Items / Item Actions / Item Qualities Items and how they work and interact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A 7 liter volume rainbow trout is just 1 cm long? Also other wonky values
4 participants