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

Adjustment of time definitions to account for non-standard system scales. #5

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jmrd98
Copy link

@jmrd98 jmrd98 commented Jan 12, 2019

Hi. When using either a non-stock system or a rescaled system, it is common to use the Kronometer mod in conjunction with Kopernicus. The rescaled systems typically result in a day/year length that differs from either the 24/365 or 6/426.08 scales.

This patch changes the encoded references for a day/year length from being hard coded to being picked up from the KSPUtil.dateTimeFormatter method. This returns either of the standard lengths in a stock game, and this method is overloaded by Kronometer to return the correct values for a rescaled system. Credit due to @Aelfhe1m and @TriggerAu for similar patches to KerbalAlarmClock and TransferWindowPlanner for leading me in the right direction.

The expected results are that both SnacksPerDay and the estimated remaining snacks display reflect the actual length of the day in the system being used.

I have tested the changes on my system and it appears to be working, but I am not very proficient in C#/Unity and would like your opinion on the changes. Thanks much, I really enjoy using your mods!

// return 360;
//else
// return 1440;
return KSPUtil.dateTimeFormatter.Day;

Choose a reason for hiding this comment

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

Suggested change
return KSPUtil.dateTimeFormatter.Day;
return KSPUtil.dateTimeFormatter.Day / 10;

The previous numbers look to be a tenth of a Day, could use a var here

Copy link
Author

Choose a reason for hiding this comment

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

This may need author input; the series of settings are in order of increasing duration in minutes, with the change intended to fill in for a whole day. I'm not sure what you mean about a tenth of a day?

Choose a reason for hiding this comment

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

Sorry mate I got my maths wrong, its not a fraction of a day like I thought - too many thoughts, not enough brain cells

@@ -38,7 +38,7 @@ public static class WindowUtils
{
const double SECONDS_PER_MINUTE = 60.0;
const double MINUTES_PER_HOUR = 60.0;
static double HOURS_PER_DAY = (GameSettings.KERBIN_TIME) ? 6.0 : 24.0;
static double HOURS_PER_DAY = KSPUtil.dateTimeFormatter.Day / 3600; // (GameSettings.KERBIN_TIME) ? 6.0 : 24.0;
public static double SECONDS_PER_DAY = SECONDS_PER_MINUTE*MINUTES_PER_HOUR*HOURS_PER_DAY;

Choose a reason for hiding this comment

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

You could property this one too to return KSPUtil.dateTimeFormatter.Day

@TriggerAu
Copy link

@jmrd98 : Added some suggestions for you, see what you think

@jmrd98
Copy link
Author

jmrd98 commented Jan 25, 2019

@TriggerAu Thanks very much, I very much appreciate the hints. The individual comments covered it for the most part. I broadly agree with your suggestions, I just didn't know enough C# to make them comfortably.

My basic goal was to make minimal adjustments (less things to break ;) ) but these all seem reasonable/minimal as well. I was about to say I wont' have a chance to look at this until the weekend, but apparently I can commit them from github... wonders never cease. It will still be until the weekend until I can give it a proper test, at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants