CompSciLauren / stardew-valley-daily-screenshot-mod

:honeybee: A Stardew Valley mod that automatically takes a screenshot of your entire farm at the start of each day
https://www.nexusmods.com/stardewvalley/mods/4779
MIT License
10 stars 9 forks source link

Fix for #29, #30 #32

Closed kins-dev closed 4 years ago

kins-dev commented 4 years ago

Fixes bugs #29 and #30 as well as fixes an issue with new games.

How it was fixed

Events are now registered OnGameLaunched instead of OnSaveLoaded. This means they are already registered for a new game as well. SaveCreated and SaveLoaded call to new events that update the saveFileCode with Game1.uniqueIDForThisGame. No need to un-register/re-register events. File.Move is now a File.Copy/File.Delete with using statement to figure out if the file is currently locked, a sleep to let the OS unlock the file but only for the Sharing violation exception. Code is as follows:

            int attemptCount = 0;
            while(File.Exists(sourceFile) && attemptCount < MAX_ATTEMPTS_TO_MOVE)
            {
                try
                {
                    attemptCount++;
                    using (FileStream lockFile = new FileStream(
                        sourceFile,
                        FileMode.Open,
                        FileAccess.ReadWrite,
                        FileShare.Read | FileShare.Delete
                    ))
                    {
                        File.Copy(sourceFile, destinationFile);
                        File.Delete(sourceFile);
                    }
                }
                catch (IOException ex)
                {
                    int HResult = System.Runtime.InteropServices.Marshal.GetHRForException(ex);
                    if (SHARING_VIOLATION == (HResult & 0xFFFF))
                    {
                        Monitor.Log($"File may be in use, retrying in {MILLISECONDS_TIMEOUT} milliseconds, attempt {attemptCount} of {MAX_ATTEMPTS_TO_MOVE}", LogLevel.Info);
                        Thread.Sleep(MILLISECONDS_TIMEOUT);
                    }
                    else
                    {
                        Monitor.Log($"Error moving file '{screenshotNameWithExtension}' into {stardewValleyScreenshotsDirectory} folder. Technical details:\n{ex}", LogLevel.Error);
                        attemptCount = MAX_ATTEMPTS_TO_MOVE;
                    }
                }
                catch(Exception ex)
                {
                    Monitor.Log($"Error moving file '{screenshotNameWithExtension}' into {stardewValleyScreenshotsDirectory} folder. Technical details:\n{ex}", LogLevel.Error);
                    attemptCount = MAX_ATTEMPTS_TO_MOVE;
                }
            }

Additionally, code duplication cleanup, logic simplification, magic numbers to constants.

kins-dev commented 4 years ago

I've done a ton of code cleanup/refactoring, I hope you don't mind. I also added a config for the auto snapshot scale, added an example gif, and I'm thinking on how to better describe the rules for taking a snapshot.

CompSciLauren commented 4 years ago

Hi Scott. Thank you so much for taking the time to do all of this! I don't mind one bit. I know there are plenty of things that need improving but I just haven't made the time to do it recently. I'm also still in college and only a few years into programming, so looking at what you did differently and the things you added was a cool learning experience for me. I think it all looks great and I appreciate that you explained the changes and included code comments. I seriously can't thank you enough. This is all very awesome!

When you mention wanting to describe the rules better for taking a screenshot, I'm not totally sure what you're referring to exactly. Are you talking about making the Config easier to understand/use?

Also, do you have a Nexus profile? I'd like to add a new section to the mod page to give credit to people who have contributed to the mod. So if you have a username, or whatever name/link you would prefer, please let me know! If you would prefer not to, that's okay too. Thanks again!

kins-dev commented 4 years ago

So first let me say it was pleasure to work on your code and help you out. You have a lot of potential and while there could be some structure changes, I suspect those came from developing against a moving target of the SMAPI. (I've been coding C# for over a decade, so I have a little more experience under my belt. Most of that was .Net 2.0 code, so I am less familiar with some of the syntax sugar added in later versions.)

What I was thinking in terms of a rule set is something where the user can describe conditions when to take a screen shot, where to save the file, how to name it, etc. That way they could have different directories/zoom levels/etc. done based on in game conditions.

It would be something like:

[
  {
    "rule": "Everyday After 6 am",
    "conditions": [{
      "time": "600"
    }],
    "zoom": "0.25",
    "directory": "c:\\my_save_location",
    "filename": "{year}-{season}-{day}-6am",
  },
  {
    "rule": "Key press",
    "conditions": [{
      "key":"multiply"
    }],
    "zoom": "1.0",
    "directory": "default",
    "filename": "default",
  },
  {
    "rule": "Yearly",
    "conditions": [{
      "Seasons":["Spring"]}, {
      "Days": ["1"]}, {
      "Time": "600"},
    ],
    "zoom": "1.0",
    "directory": "default",
    "filename": "farm-year-{year}",
  },
]

Doing something like this allows the user more flexibility while allowing your code paths to combine into a single structure.

The most important piece of advice I can give you is, adhere to one version of the truth. Derived forms are OK, but wherever possible go back to a single constant for related items. If you find yourself copying and pasting code, make the copy a parameter on the function or make a new function. Doing so will save you a lot of time in the future.

Also, I might have a nexus account, but I don't know the username and it really isn't important to me. Dropping my GitHub account name into a contributors file is more than enough credit.

kins-dev commented 4 years ago

I've started a basic structure and such on my Rules branch if you want to follow along. I'll be adding more in my free time.