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

Use system time for holiday menu screens #39897

Merged
merged 22 commits into from
Apr 29, 2020

Conversation

Drewscriver
Copy link
Contributor

@Drewscriver Drewscriver commented Apr 25, 2020

Summary

SUMMARY: Features "Fixes #36628, implementing use of system time for holidays"

Purpose of change

Fixes #36628, automatically setting holiday screens based on the local system date, instead of by manually updating the source code.

Reduces code churn for holiday builds, and makes holiday easter egg screens viewable for users of future stable release versions.

Describe the solution

Change current_holiday from a static const to a member variable
Implement is_easter() as a member function
Implement get_holiday_from_time() as a member function, calling is_easter
note that get_holiday_from_time() had to use preprocessor directives due to differing versions of localtime
Call get_holiday_from_time() when opening the screen

Describe alternatives you've considered

Letting someone else do it.

Testing

Set system date and checked that all holiday screens loaded All did, except that in the process, I found that the ASCII art for independence day is messed up.

Additional context

std::localtime is non-intuitive, and Easter is now least favorite holiday.

The two recipes for fruit juice took the same amount of time to craft, even though one was 100% juice, and the other 50% juice and 50% water.  This change cuts the 50% water recipe crafting time from 5 minutes to 3 minutes to reflect the reduced amount of effort.
@anothersimulacrum
Copy link
Member

Put fixes in front of the issue it closes, or else github won't automatically close it when this PR is merged.

Fix error calculating thanksgiving.  Also make Halloween a week, instead of all month, and add a couple of comments.
Added commentary for formula.
@lgtm-com
Copy link

lgtm-com bot commented Apr 25, 2020

This pull request introduces 1 alert when merging 2018990 into 889f657 - view on LGTM.com

new alerts:

  • 1 for Use of potentially dangerous function

src/main_menu.cpp Outdated Show resolved Hide resolved
@anothersimulacrum
Copy link
Member

You need to remove the issue after the fixes too.
It should be something like Fixes #12345.

@ifreund ifreund added <Enhancement / Feature> New features, or enhancements on existing [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) labels Apr 25, 2020
@kevingranade
Copy link
Member

So... there's a reason I said to have a table of dates you import, because I don't want anyone messing with gross calendar code.
Just grab a table of dates like this and embed it in the code: https://www.theholidayspot.com/easter/easter_dates.htm

src/main_menu.h Outdated Show resolved Hide resolved
Drewscriver and others added 2 commits April 28, 2020 22:41
as suggested

Co-Authored-By: anothersimulacrum <anothersimulacrum@gmail.com>
as suggested

Co-Authored-By: anothersimulacrum <anothersimulacrum@gmail.com>
src/main_menu.cpp Outdated Show resolved Hide resolved
src/main_menu.cpp Outdated Show resolved Hide resolved
src/main_menu.cpp Outdated Show resolved Hide resolved
src/main_menu.cpp Outdated Show resolved Hide resolved
src/main_menu.cpp Outdated Show resolved Hide resolved
@nexusmrsep
Copy link
Contributor

Fragment of this PR results in compilation errors under Code::Blocks project, as follows:

image

image

@ZhilkinSerg
Copy link
Contributor

Which gcc version?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` <Enhancement / Feature> New features, or enhancements on existing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use system time for holidays
9 participants