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.

Sign up for GitHub

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

Jump to bottom

refactor: fahrenheit to celsius, part 2 #3926

Merged
merged 10 commits into from
Dec 18, 2023
Merged

refactor: fahrenheit to celsius, part 2 #3926

merged 10 commits into from
Dec 18, 2023

Conversation

scarf005
Copy link
Member

@scarf005 scarf005 commented Dec 16, 2023 •
edited
Loading

Purpose of change

part 2 of #2570

Describe the solution

  • migrated weather class to use Celsius.
  • removed legacy usage of freezing_point.

Describe alternatives you've considered

remove discrete temperatures and manage it via enums (temperature_flag)

Testing

  • (to my surprise) local tests passed.
  • was able to load items with/without freezing_point field.

Additional context

  • after 5 months we're back.
  • TIL adding temperatures with temperatures makes no sense (hence C + C != F + F)

@github-actions github-actions bot added src changes related to source code. tests changes related to tests labels Dec 16, 2023
@scarf005 scarf005 changed the title refactor: fahrenheit to celsius, part 2 refactor: fahrenheit to celsius, part 2 Dec 16, 2023
@github-actions github-actions bot added docs PRs releated to docs page JSON related to game datas in JSON format. mods PR changes related to mods. labels Dec 16, 2023
@scarf005
Copy link
Member Author

oh fun, #3908 is preventing me from running all tests

@scarf005 scarf005 requested a review from joveeater December 16, 2023 15:16
Coolthulhu
Coolthulhu reviewed Dec 16, 2023
View reviewed changes
src/item_factory.cpp Outdated Show resolved Hide resolved
src/weather.cpp Outdated Show resolved Hide resolved
@scarf005 scarf005 requested a review from Coolthulhu December 17, 2023 03:17
scarf005 and others added 10 commits December 18, 2023 07:37
@scarf005
refactor: water_temperature to units::temperature
3dc2627
@scarf005
refactor: weather_manager::temperatrue to units::temperature
c3aea32
@scarf005 @Coolthulhu
refactor: explicitly use to_millidegree_celsius
5be3b16 
discussed in cataclysmbnteam#2570 (comment)

Co-authored-by: Coolthulhu <[email protected]>
@scarf005 @Coolthulhu
refactor: simplify celsisus to fahrenheit calls
9594e8e 
discussed in
- cataclysmbnteam#2570 (comment)
- cataclysmbnteam#2570 (comment)

Co-authored-by: Coolthulhu <[email protected]>
@scarf005 @Coolthulhu
refactor: use milidegree_celsius
20477d2 
discussed in: cataclysmbnteam#2570 (comment)

Co-authored-by: Coolthulhu <[email protected]>
@scarf005 @Coolthulhu
refactor: make arbitrary temperatures look arbitrary
20209b2 
discussed in cataclysmbnteam#2570 (comment)

Co-authored-by: Coolthulhu <[email protected]>
@scarf005
test: adjust lossy conversion from fahrenheit to celsius
39b713e
@scarf005
refactor: remove freezing_point
fcbcf16 
it was never used for years.
@scarf005
fix: round temperature manually
f1bd8ce
@scarf005
refactor: use units::temperature for weather_manager
c4e735d
@scarf005 scarf005 force-pushed the remove-fahrenheit-part-2 branch from 355e36b to c4e735d Compare December 17, 2023 22:38
@scarf005 scarf005 merged commit 4b5f9ba into cataclysmbnteam:main Dec 18, 2023
12 checks passed
@scarf005 scarf005 deleted the remove-fahrenheit-part-2 branch December 18, 2023 02:34
scarf005 added a commit to scarf005/Cata-Rogall-Modpack that referenced this pull request Dec 21, 2023
@scarf005
fix: freezing_point 필드 제거
6e061b4 
해당 필드가 cataclysmbnteam/Cataclysm-BN#3926 에서 완전히 제거되며 발생한 오류 메시지 해결
@scarf005 scarf005 mentioned this pull request Dec 21, 2023
Merged
scarf005 added a commit to cataclysmroguelikegallery/Cata-Rogall-Modpack that referenced this pull request Dec 21, 2023
@scarf005
fix: freezing_point 필드 제거 (#125)
6eb4a5c 
해당 필드가 cataclysmbnteam/Cataclysm-BN#3926 에서 완전히 제거되며 발생한 오류 메시지 해결
NappingOcean pushed a commit to cataclysmroguelikegallery/Cata-Rogall-Modpack that referenced this pull request Feb 1, 2024
@scarf005 @NappingOcean
fix: freezing_point 필드 제거 (#125)
8e7df80 
해당 필드가 cataclysmbnteam/Cataclysm-BN#3926 에서 완전히 제거되며 발생한 오류 메시지 해결
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Coolthulhu Coolthulhu Coolthulhu approved these changes

@olanti-p olanti-p Awaiting requested review from olanti-p

@joveeater joveeater Awaiting requested review from joveeater

Assignees
No one assigned
Labels
docs PRs releated to docs page JSON related to game datas in JSON format. mods PR changes related to mods. src changes related to source code. tests changes related to tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@scarf005 @Coolthulhu