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

NSV Materials Fix #2558

Merged
merged 2 commits into from
Nov 10, 2023
Merged

Conversation

Ikalpo
Copy link
Contributor

@Ikalpo Ikalpo commented Oct 3, 2023

About The Pull Request

makes the NSV material amounts look nicer in code
fixes the durasteel sheet material amount (it was missing a bit of titanium)
fixes the amount NSV materials cost in ORMs and lathes
adds material amounts to tile/carpet/ship (they are made of durasteel)

Why It's Good For The Game

fix man good
and also I want to be able to recycle those floor tiles when I accidently make them

Testing Photographs and Procedure

Screenshots&Videos

image

Changelog

🆑
fix: You can now recycle nanoweave carpet tiles
fix: Fixed the amount that NSV materials cost in ORMs and lathes
/:cl:

@Karmic-Skink
Copy link
Contributor

image

The implication that the green there looks nicer in code is 100% subjective and also 1000% the wrong choice.

@Bokkiewokkie
Copy link
Contributor

image

The implication that the green there looks nicer in code is 100% subjective and also 1000% the wrong choice.

Yeah @Ikalpo you should just make it one decimal or fraction

@Ikalpo
Copy link
Contributor Author

Ikalpo commented Oct 6, 2023

image
The implication that the green there looks nicer in code is 100% subjective and also 1000% the wrong choice.

Yeah @Ikalpo you should just make it one decimal or fraction

is list(/datum/material/iron = MINERAL_MATERIAL_AMOUNT*0.2/4, /datum/material/silver = MINERAL_MATERIAL_AMOUNT*0.15/4, /datum/material/titanium = MINERAL_MATERIAL_AMOUNT*0.65/4) better?

@Karmic-Skink
Copy link
Contributor

image
The implication that the green there looks nicer in code is 100% subjective and also 1000% the wrong choice.

Yeah @Ikalpo you should just make it one decimal or fraction

is list(/datum/material/iron = MINERAL_MATERIAL_AMOUNT*0.2/4, /datum/material/silver = MINERAL_MATERIAL_AMOUNT*0.15/4, /datum/material/titanium = MINERAL_MATERIAL_AMOUNT*0.65/4) better?

No

@Karmic-Skink
Copy link
Contributor

Karmic-Skink commented Oct 7, 2023

Okay so firstly, you are making this far more complicated than it needs to be. It really does not need to contain any operations to determine what the exact number needs to be and just makes it harder to read.

image
Consider this image.
When we are taught mathematics at school, we are first taught fractions as these are the easiest numbers for our human brains to make sense of.
As we progress, we move forward into decimals and scientific nomenclature.
Which most adults have somewhat an understanding of.

By adding basically a horrible inverse of a fraction using decimals and including operators, you are effectively making it extremely hard to read for no real reason.

tl;dr there is absolutely nothing wrong with using fractions or straight decimals, please stop trying to reinvent the wheel.

Copy link
Contributor

@Bokkiewokkie Bokkiewokkie left a comment

Choose a reason for hiding this comment

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

Agree with Karmic on the readability of multiplications here (btw karmic just request changes for it next time)

@Bokkiewokkie Bokkiewokkie merged commit a3f9e81 into BeeStation:master Nov 10, 2023
9 checks passed
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.

3 participants