Skip to content

Add self-update mechanism (fixes #3)#11

Merged
niezbop merged 21 commits intoDragonBox:masterfrom
niezbop:feature/self_update
Nov 13, 2017
Merged

Add self-update mechanism (fixes #3)#11
niezbop merged 21 commits intoDragonBox:masterfrom
niezbop:feature/self_update

Conversation

@niezbop
Copy link
Copy Markdown
Member

@niezbop niezbop commented Nov 10, 2017

Enable Uplift to check for updates and act consequently.

  • Automated daily check to see if an update is available

  • MenuItem to force this check

Fixes #3

@niezbop niezbop self-assigned this Nov 10, 2017
@niezbop niezbop requested a review from lacostej November 10, 2017 10:07

private static bool ShouldCheck()
{
string lastCheck = EditorPrefs.GetString(lastUpdateCheckKey);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would that prevent update in a different project?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure. It could actually. I'll check this.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It looks like the preferences are project specific!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Cool.

{
Debug.Log("No update for Uplift available");
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

some indentation issues.

List<System.Object> releases = (List<System.Object>)obj;
foreach (Dictionary<string, object> release in releases)
{
if(VersionParser.GreaterThan((string)release ["tag_name"], About.Version))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

isn't tag_name formatted like "v1.0.0beta1".

Do we need to strip the v?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nope! The Version parser ignore the v by default. I have tested this and it works.

EditorApplication.update -= EditorUpdate;
EditorPrefs.SetString(
lastUpdateCheckKey,
DateTime.UtcNow.ToString(dateFormat, provider)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why not store time in seconds?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Because I'm not sure you can in C#. Or we would have to write our own method, something like

public static int convertDateTimeToSeconds(DateTime dateTimeToConvert)
    {
        int secsInAMin = 60;
        int secsInAnHour = 60 * secsInAMin;
        int secsInADay = 24 * secsInAnHour;
        double secsInAYear = (int)365.25 * secsInADay;

        int totalSeconds = (int)(dateTimeToConvert.Year * secsInAYear) + 
                       (dateTimeToConvert.DayOfYear * secsInADay) +
                       (dateTimeToConvert.Hour * secsInAnHour) +
                       (dateTimeToConvert.Minute * secsInAMin) + 
                       dateTimeToConvert.Second;

        return totalSeconds;
    }

which would me more cumbersome than anything imo. DateTime.Second, .Minute etc... only returns the current seconds, minutes... (whithin the [0-60] range for instance)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The issue is that EditorPrefs does not accept long. Anyways if your first comment is true, then we should not use the Preferences, so we will not face the issue. I'm not sure I see why storing it as an integer is better than a string though.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was just not a fond of date parsing. I see that you use CultureInfo.InvariantCulture so it should be fine.

@niezbop niezbop force-pushed the feature/self_update branch from a94fadf to 11ca1b2 Compare November 10, 2017 11:26
DirectoryInfo assetsPath = new DirectoryInfo(Application.dataPath);
string destination = Path.Combine(assetsPath.Parent.FullName, unitypackageName);

ServicePointManager.ServerCertificateValidationCallback = delegate { return true; };
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should be careful here. I am not fond of bypassing SSL.

See http://www.khalidabuhakmeh.com/validate-ssl-certificate-with-servicepointmanager for I think a better solution.

@niezbop niezbop force-pushed the feature/self_update branch from 5cb5ab7 to e9977f1 Compare November 10, 2017 14:16
@niezbop niezbop force-pushed the feature/self_update branch from 6a429a1 to 371fbf5 Compare November 10, 2017 15:25
Copy link
Copy Markdown
Member

@lacostej lacostej left a comment

Choose a reason for hiding this comment

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

There are a few small things we can do to improve, but it's already great as is. Let's have this in!

}
EditorGUILayout.LabelField("SSL Certificates:", EditorStyles.boldLabel);
EditorGUILayout.HelpBox(
"Unknown certificates could be the result of our registered certificates being outdated or the result of a potential attack. Trusting unknown certificates could lead to security breaches. Use at your own risk!",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We could explain where we use SSL certificates. Only in the updater.

using Uplift.Windows;
using Uplift.Updating;
using System;
using System.Collections.Generic;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we will have to clean this up.

@lacostej lacostej changed the title Add self-update mechanism Add self-update mechanism (fixes #3) Nov 11, 2017
if(VersionParser.GreaterThan((string)release ["tag_name"], About.Version))
{
Debug.Log("There is a new version of Uplift available!");
UpdatePopup popup = EditorWindow.GetWindow(typeof(UpdatePopup), true) as UpdatePopup;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not sure how that works if we have had multiple updates, e.g. upgrade from 1.0.0beta1 to 1.0.0beta4.

- Extract GitHub release fetching to new separate namespace,
  GitHubModule

- Display all possible updates in the update popup
@niezbop niezbop merged commit e26871d into DragonBox:master Nov 13, 2017
@niezbop niezbop deleted the feature/self_update branch November 13, 2017 10:48
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.

2 participants