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

Solving issue of size #74160

Closed

Conversation

Vgoloshivskiy
Copy link
Contributor

@Vgoloshivskiy Vgoloshivskiy commented May 28, 2024

Summary

Bugfixes "Added check for non crammed, removed flag"

Purpose of change

Issue existed for over 2 days decided to fix myself to make experimental playable for tall charcters
(#74068)
#74068

Describe the solution

added condition so character not always be crammed
changed volume to slighlty more reasonable
removed label Large from Tall (let be honest it was needed to be done anyway tall people are not the size of the cow / bear)

Describe alternatives you've considered

pinging @RenechCDDA until he fixes this issue
/
completly removing this detail

Testing

done 2 test with regular and very tall character before and after changes
before regular charcter was cramed and tall character could not enter after both could enter and noone was cramed

Additional context

i m pretty sure that is all

Vgoloshivskiy and others added 4 commits May 28, 2024 23:33
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added [JSON] Changes (can be) made in JSON Mutations / Traits / Professions/ Hobbies Mutations / Traits / Professions/ Hobbies [C++] Changes (can be) made in C++. Previously named `Code` astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels May 28, 2024
@@ -203,16 +203,19 @@ static units::volume size_to_volume( creature_size size_class )
// e.g. max tiny size is 7500, max small size is 46250, we return
// 46250+7500 / 2 - 1_ml = 26875_ml - 1ml
// This is still stupid and both of these functions should be merged into one single source of truth.

//Updated Calculation Because this one seems off
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how you've determined it "seems off", but your math is just plain incorrect.

Max tiny size is 7500ml, max small size is 46250, the difference is (alternatively) represented as 38750, therefore the midpoint between the two is 7500 + (38750 / 2) = 7500 + 19375 = 26875, as calculated. 26875 is 19375 away from 7500, and 19375 away from 46250.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I incorrectly expressed myself my point was more of taking half of max volume as an average volume for size as it looked a bit better to me (and for Huge I took it min value as it is as far as i can see the only option for mutated characters to still be able to use vehicles

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you belive that this is inappropriate solution i will change it to previous values.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label May 29, 2024
@worm-girl
Copy link
Contributor

worm-girl commented May 29, 2024

This PR seems to fundamentally misunderstand the size system and misattributes a bug with the cramped vehicle effect ( caused by #74004 ) to a trait that has nothing to do with the issue. The bug is currently affecting all characters and afaik a fix is being worked on by RenechCDDA.

As the person who added the Very Tall mutation, its entire purpose was to add the LARGE flag to chargen. Large characters are not "cow and horse sized", they are just humans with mundane gigantism, 6'6" and up. Removing LARGE from the mutation would remove its sole reason for existing. Huge characters are cow and horse sized, but you can't select huge traits in chargen. But that's all irrelevant because the trait has nothing to do with the issue, it's affecting all characters.

Creature sizes are not something that should be arbitrarily adjusted as they interact with a huge number of mechanics and would break many things, especially not because someone thinks the math "feels off". Changing them creates a ton of work downstream and the burden of proof for bad math should be quite high.

As an aside, Kevin has suggested removing the Very Tall trait entirely and I'm inclined to agree, mostly because it leads to a lot of misunderstandings like this one. Very Short causes less confusion so I don't see as much of an issue there.

@Vgoloshivskiy
Copy link
Contributor Author

Apologies, i have no idea who Kevin is. We already have trait Large as mutation, description of Large
image
Bear / Tiger (no cow that one on me)
6'6 is 15 cm above me and i doupt that can be called Large as a Bear and in no way that character can't use average car seat.
I'm closing this PR as i see @RenechCDDA added a fix but if my Very Tall character can't get into car seat I ll reopen it.
Sorry couldn't answer sooner i was working.

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 [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Mutations / Traits / Professions/ Hobbies Mutations / Traits / Professions/ Hobbies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants