8
\$\begingroup\$

The goal

I really enjoy the game Stardew Valley. One of the things that the game has are geodes. Much like real-world geodes, these geodes can be cracked open, and you can find some fun things inside.

Some relatively rare/valuable items can be found inside of them, so it can be beneficial to try and collect them to get the items inside. You may also want to know when you'll get the things you're looking for to help plan/prioritize; if you're going to get it 5 geodes from now then it might be worth searching for them exclusively, while if you're going to get it 100 geodes from now then you might want to do it on the side.

The way that the contents of a geode are determined is based on a predictable random seed, that consists of a unique game ID (I don't touch this at all right now) and the number of geodes that have already been opened. By changing the value the game has for the number of opened geodes, you can then use the same algorithm the game uses to determine what the Nth geode will return. Throughout this post/the code, I generally use the term index to refer to the Nth geode, and the term distance to mean N geodes from now.

There already exist tools to predict when you'll get certain items from a geode. Click here to see what one of my current games is going to return.

I don't like having to leave the game, however, as I find that it breaks immersion (and having to upload my save file is tedious). Instead, I wanted to make a mod using the Stardew Modding API, or SMAPI.

SMAPI provides a lot of open-source tools for working with and manipulating games, and is the standard for Stardew Valley mods.

Options

When deciding on approach, I basically had two choices:

  1. Duplicate the logic the game uses (this is what the existing tools do)
  2. Directly call the method that the game uses

I don't love option 1; if I mess something up, or the game changes, then my mod won't work either. Option 2 has similar vulnerabilities if the method to use changes, but I'm less vulnerable to changes inside the method itself. Ultimately I decided that I'd rather do option 2. This is the basis for the code below.

Things I want out of this review

Before I throw a bunch of code and decisions out at you, I'm particularly interested in answers to the following:

  1. How is my abstraction? There are a few places things are leaking a bit (I call these out below), but for the most part I feel pretty good about it. Please let me know where I'm wrong.
  2. Does the way I've broken things up into logical units make sense? Are there smarter or cleaner interfaces I could be using?
  3. C# is a language I use less than I'd like, and I know that I'm no expert. How idiomatic is the code?

Of course, please feel free to comment on any aspects of the code.

Abstraction

When implementing this, I made a conscious choice to abstract away the Stardew Valley game itself as much as I could. I did this because I want to be able to write unit tests that do not rely on a game currently running, and because it should make me more flexible if, in the future, I wanted to implement option 1, or if I wanted to be compatible with other mods that change what kinds of geodes exist in the game.

Quick note - for brevity I combined some things into single code blocks; every class/interface is in a distinct, appropriately named file.

Abstracting the game itself

If you read the code of a lot of mods, you'll find that a lot of them refer to Game1. This is the class that holds a metric ton of game logic and properties, most of them exposed via static fields. As such, you would see code that looks like var geodeCount = Game1.stats.GeodesCracked. This, pretty obviously, violates my goal of being able to abstract away an actual running game. To that end, I made this interface, and an implementation of the interface:

using StardewModdingAPI;
using System;

namespace Dannnno.StardewMods.Predictor.Shared
{
    public interface IStardewGame
    {
        #region Properties
        /// <summary>
        /// Get the number of geodes that have already been cracked by the player
        /// </summary>
        [StardewManagedProperty]
        public uint GeodeCount { get; set; }
        #endregion

        #region Methods
        /// <summary>
        /// Create a context in which changes to game state are rolled back at the end of
        /// </summary>
        /// <param name="monitor">The monitor to which we should be logging messages, if that is desired</param>
        /// <returns>Disposable context manager</returns>
        public IDisposable WithTemporaryChanges(IMonitor monitor = null);
        #endregion
    }

    /// <summary>
    /// Encapsulates StardewValley's Game1
    /// </summary>
    public class StardewGameWrapper : IStardewGame
    {
        /// <summary>
        /// Get the number of geodes that have already been cracked by the player
        /// </summary>
        [StardewManagedProperty]
        public uint GeodeCount
        {
            get { return Game1.stats.GeodesCracked; }
            set { Game1.stats.GeodesCracked = value; }
        }

        /// <summary>
        /// Create a context in which changes to game state are rolled back at the end of
        /// </summary>
        /// <param name="monitor">The monitor to which we should be logging messages, if that is desired</param>
        /// <returns>Disposable context manager</returns>
        public IDisposable WithTemporaryChanges(IMonitor monitor = null)
        {
            return new GameStateContextManager(this, monitor);
        }

    }
}

Don't worry about the GameStateContextManagerorStardewManagedProperty` for now; I'll come back to this later. Ultimately, what I want is:

  1. A way to get the current number of geodes that have been cracked in the game
  2. A way to modify that value, temporarily.

Abstracting away general data

Mod compatibility is pretty important in the Stardew Valley modding community. If someone thinks that 4 varieties of geodes isn't enough, or they want to be able to find other things in them, I don't want my mod to preclude that. While I haven't yet reached that goal, I do want to make sure I can extend this in the future to be more compatible. This means that the way I retrieve objects in the game needs to be abstract. When I say object, I mean StardewValley.Object, not the C# object. These objects are generally created based on xnb metadata files, but can also be modified by mods.

To retrieve the list of objects that this game has, I created the following:

using System.Collections.Generic;
using StardewModdingAPI;

namespace Dannnno.StardewMods.Predictor.Shared
{
    /// <summary>
    /// Standard interface by which to get stardew valley objects
    /// </summary>
    public interface IStardewObjectProvider
    {
        /// <summary>
        /// Get the raw stardew objects, from key to delimited string
        /// </summary>
        /// <returns>Mapping from stardew object IDs to raw strings</returns>
        public IDictionary<int, string> GetStardewRawObjects();

        /// <summary>
        /// Get stardew objects in stardew format
        /// </summary>
        /// <returns>Collection of stardew objects</returns>
        public IEnumerable<StardewValley.Object> GetStardewObjects();
    }

    /// <summary>
    /// Standard data provider that pulls form the Data/ObjectInformation file
    /// </summary>
    public class StardewDataObjectInfoProvider : IStardewObjectProvider
    {
        /// <summary>
        /// Get the SMAPI helper that can retrieve the xnb content
        /// </summary>
        public IModHelper Helper { get; set; }

        /// <summary>
        /// Create the data provider
        /// </summary>
        /// <param name="helper">With a little help from our friends at SMAPI</param>
        public StardewDataObjectInfoProvider(IModHelper helper)
        {
            Helper = helper;
        }

        /// <summary>
        /// Get the stardew objects from the Data/ObjectInformation.xnb file
        /// </summary>
        /// <returns>The list of objects</returns>
        public IEnumerable<StardewValley.Object> GetStardewObjects()
        {
            foreach (var rawObject in GetStardewRawObjects())
            {
                yield return new StardewValley.Object(rawObject.Key, initialStack: 0, isRecipe: false, price: -1, quality: 0);
            }
        }

        /// <summary>
        /// Gets raw stardew objects from the Data/ObjectInformation file
        /// </summary>
        /// <returns>Raw stardew objects</returns>
        public IDictionary<int, string> GetStardewRawObjects()
        {
            return Helper.Content.Load<Dictionary<int, string>>("Data/ObjectInformation", ContentSource.GameContent);
        }
    }
}

In general, I want to be able to represent objects in one of two ways:

  1. As an actual StardewValley.Object (that I very inelegantly create in GetStardewObjects
  2. As an object id, and string representation of that object

My interface is intended to let me generically have a way to get the list of objects in the game, and then my implementation of it just dumps all of the objects in the Data/ObjectInformation xnb file.

Question - is my direct usage of StardewValley.Object a sign of poor encapsulation? I could certainly create some new interface and class that represents the parts of an object I care about, and provide methods (probably an implicit conversion) to transform to the forms I care about, but ultimately I need the literal StardewValley.Object for my later functionality. I'm not sure that this is worth changing anything.

Actually getting geodes

Now that I've made my shared code (because I want to leverage it for future mods), I need to actually use it. To that end, I create a service interface for retrieving geodes, and a calculator interface that puts it all together.

Question - I'm using service and provider in different places, and it makes sense to me, but are there better names I could be using? I strongly dislike the name factory, and would like to avoid that.

using Dannnno.StardewMods.Predictor.Shared;
using System.Collections.Generic;

namespace Dannnno.StardewMods.Predictor.Geodes
{
    /// <summary>
    /// Service that returns geode objects
    /// </summary>
    /// <typeparam name="IProviderType">The provider that can yield the data</typeparam>
    public interface IGeodeService<IProviderType> where IProviderType : IStardewObjectProvider
    {
        /// <summary>
        /// Get the geode objects this service is aware of
        /// </summary>
        /// <param name="provider">The provider that will yield stardew objects to select geodes from</param>
        /// <returns>Enumeration of the known geode kinds</returns>
        public IEnumerable<StardewValley.Object> RetrieveGeodes(IProviderType provider);
    }

    /// <summary>
    /// Standard interface for calculating the geode's contents
    /// </summary>
    public interface IGeodeTreasureCalculator
    {
        /// <summary>
        /// Get the treasure from a geode
        /// </summary>
        /// <param name="geode">The geode to get the treasure from</param>
        /// <returns>Buried gold</returns>
        public StardewValley.Object GetTreasureFromGeode(StardewValley.Object geode);
    }
}

These interfaces give me the ways I can figure out the way to retrieve the geodes. Their implementations:

using Dannnno.StardewMods.Predictor.Shared;
using System.Collections.Generic;
using StardewValley;

namespace Dannnno.StardewMods.Predictor.Geodes
{
    /// <summary>
    /// Implementation of a stardew geode service that uses the string "Geode" to identify geodes
    /// </summary>
    public class StardewGeodeService<ProviderType> : IGeodeService<ProviderType> where ProviderType : IStardewObjectProvider
    {
        /// <summary>
        /// Get the set of geode objects from game data
        /// </summary>
        /// <param name="provider">The data provider to get objects from</param>
        /// <returns>Enumerable over the geode objects in game data</returns>
        public IEnumerable<StardewValley.Object> RetrieveGeodes(ProviderType provider)
        {
            foreach (var stardewObject in provider.GetStardewRawObjects())
            {
                if (IsObjectInfoAGeode(stardewObject.Value))
                {
                    yield return new StardewValley.Object(stardewObject.Key, 0); // Stack count doesn't matter
                }
            }
        }

        /// <summary>
        /// Determine whether a given record from Data/ObjectInformation is a geode
        /// </summary>
        /// <param name="key">The key of the record</param>
        /// <param name="objectInformation">The string literal from the object information</param>
        /// <returns>Whether the object is a geode</returns>
        /// <remarks>This is currently really stupid, and just does a check for the strings "Geode" and "geode"</remarks>
        private bool IsObjectInfoAGeode(string objectInformation)
        {
            return objectInformation.Contains("Geode") || objectInformation.Contains("geode");
        }
    }

    /// <summary>
    /// Standard method of calculating treasures
    /// </summary>
    public class StardewGeodeCalculator : IGeodeTreasureCalculator
    {
        /// <summary>
        /// Get the treasure from the geode
        /// </summary>
        /// <param name="geode">The geode</param>
        /// <returns>The treasure</returns>
        public Object GetTreasureFromGeode(Object geode)
        {
            return Utility.getTreasureFromGeode(geode); // Why copy the method when we can just use the game's?
        }
    }
}

A few tidbits:

  • Right now the way I distinguish between a geode and a non-geode is pretty dumb. Open to suggestions, but it works well enough for the current scope
  • Utility.getTreasureFromGeode is the actual function the game uses to get what treasure cracking a given geode will return. This is the implementation of option 2 above

The big reveal

Like I mentioned earlier, we just need the seed:

  • The number of geodes cracked so far
  • The unique Game ID

We have those already (game ID is handled for us), and I also have a way to modify the current number of geodes cracked. To that end, if I tweak the current number of geodes, then call StardewGeodeCalculator.GetTreasureFromGeode I will get what that geode would give me at that count. This lets me look forward (or backward) as far as I would like. My implementation of that:

using Dannnno.StardewMods.Predictor.Shared;
using StardewModdingAPI;
using System;
using System.Collections.Generic;
using System.Diagnostics.Contracts;
using System.Linq;

namespace Dannnno.StardewMods.Predictor.Geodes
{
    /// <summary>
    /// The set of directions we can predict in
    /// </summary>
    public enum PredictionDirectionEnum
    {
        Forwards,
        Backwards
    }

    /// <summary>
    /// Predicts what new geode objects will be found
    /// </summary>
    public class GeodePredictor
    {
        #region fields
        private IGeodeService<IStardewObjectProvider> geodeService;
        private IStardewObjectProvider objectProvider;
        private Lazy<IList<StardewValley.Object>> geodeList;
        #endregion

        #region properties
        /// <summary>
        /// Get or set the service used to retrieve the geodes
        /// </summary>
        public IGeodeService<IStardewObjectProvider> GeodeService
        {
            get => geodeService;
            set
            {
                geodeService = value;
                InitializeCache();
            }
        }

        /// <summary>
        /// Get the list of geodes that this predictor can work on
        /// </summary>
        public IList<StardewValley.Object> GeodeList { get => geodeList.Value; }

        /// <summary>
        /// Get or set the provider of geode objects
        /// </summary>
        public IStardewObjectProvider ObjectProvider
        {
            get => objectProvider;
            set
            {
                objectProvider = value;
                InitializeCache();
            }
        }

        /// <summary>
        /// Get or set how to calculate geode contents
        /// </summary>
        public IGeodeTreasureCalculator GeodeCalculator { get; set; }

        /// <summary>
        /// Get or set the associated game
        /// </summary>
        public IStardewGame Game { get; set; }

        /// <summary>
        /// Get or set the associated monitor
        /// </summary>
        public IMonitor Monitor { get; set; }

        /// <summary>
        /// Get or set the predictions we've made
        /// </summary>
        private IDictionary<uint, IDictionary<StardewValley.Object, StardewValley.Object>> CachedPredictions { get; set; }
        #endregion

        /// <summary>
        /// Create a new predictor
        /// </summary>
        /// <param name="service">The service to use to predict geodes</param>
        /// <param name="provider">The provider to retrieve objects from</param>
        /// <param name="game">The game this predictor is associated with</param>
        /// <param name="calculator">The calculator that will return the geode's treasure</param>
        /// <param name="monitor">The monitor to log to</param>
        public GeodePredictor(IGeodeService<IStardewObjectProvider> service, IStardewObjectProvider provider, IStardewGame game, IGeodeTreasureCalculator calculator, IMonitor monitor = null)
        {
            GeodeService = service;
            ObjectProvider = provider;
            Game = game;
            Monitor = monitor;
            GeodeCalculator = calculator;
            InitializeCache();
        }


        /// <summary>
        /// Empty the cache and start over
        /// </summary>
        private void InitializeCache()
        {
            CachedPredictions = new Dictionary<uint, IDictionary<StardewValley.Object, StardewValley.Object>>();
            geodeList = new Lazy<IList<StardewValley.Object>>(() => GeodeService.RetrieveGeodes(ObjectProvider).ToList());
        }

        /// <summary>
        /// Predict the treasures that a geode `distance` items away will return
        /// </summary>
        /// <param name="distance">How far ahead to look</param>
        /// <param name="direction">The direction to look</param>
        /// <returns>For each kind of geode we can predict, the associated result</returns>
        public IDictionary<StardewValley.Object, StardewValley.Object> PredictTreasureFromGeodeAtDistance(uint distance = 1, PredictionDirectionEnum direction = PredictionDirectionEnum.Forwards)
        {
            uint actualSearchIndex = direction switch
            {
                PredictionDirectionEnum.Backwards => distance > Game.GeodeCount ? Game.GeodeCount : Game.GeodeCount - distance,
                PredictionDirectionEnum.Forwards => Game.GeodeCount + distance,
                _ => Game.GeodeCount
            };

            return PredictTreasureFromGeodeAtIndex(actualSearchIndex);
        }

        /// <summary>
        /// Predict the treasures that will come from geodes in a span ahead and behind of our current count
        /// </summary>
        /// <param name="distanceAhead">How far ahead to peek</param>
        /// <param name="distanceBehind">How far behind to look</param>
        /// <returns>The treasures found</returns>
        public IEnumerable<IDictionary<StardewValley.Object, StardewValley.Object>> PredictTreasureFromGeodeByRangeDistance(uint distanceAhead, uint distanceBehind)
        {
            uint startIndex = Game.GeodeCount < distanceBehind ? Game.GeodeCount : Game.GeodeCount - distanceBehind;
            uint endIndex = Game.GeodeCount + distanceAhead;

            return PredictTreasureFromGeodesInRange(startIndex, endIndex);
        }

        /// <summary>
        /// Predict the treasures that a geode at a given count will return
        /// </summary>
        /// <param name="actualGeodeCount">The geode count to check</param>
        /// <returns>The treasures found</returns>
        private IDictionary<StardewValley.Object, StardewValley.Object> PredictTreasureFromGeodeAtIndex(uint actualGeodeCount)
        {
            return PredictTreasureFromGeodesInRange(actualGeodeCount, actualGeodeCount).First();
        }

        /// <summary>
        /// Predict the treasures that geodes within a range will return
        /// </summary>
        /// <param name="firstGeodeCount">The first to check</param>
        /// <param name="lastGeodeCount">The last to check</param>
        /// <returns>The treasures found</returns>
        private IEnumerable<IDictionary<StardewValley.Object, StardewValley.Object>> PredictTreasureFromGeodesInRange(uint firstGeodeCount, uint lastGeodeCount)
        {
            Contract.Requires(firstGeodeCount <= lastGeodeCount, "The first count must not be greater than the last count");

            var results = new List<IDictionary<StardewValley.Object, StardewValley.Object>>();

            using (Game.WithTemporaryChanges(Monitor))
            {
                for (; firstGeodeCount < lastGeodeCount; ++firstGeodeCount)
                {
                    if (!CachedPredictions.ContainsKey(firstGeodeCount))
                    {
                        // Temporarily modify the current geode count
                        Game.GeodeCount = firstGeodeCount;
                        CachedPredictions[firstGeodeCount] = GeodeList.ToDictionary(geodeKind => geodeKind,
                                                                                    geodeKind => GeodeCalculator.GetTreasureFromGeode(geodeKind));
                    }

                    results.Add(CachedPredictions[firstGeodeCount]);
                }
            }

            // Don't use yield return because we don't want to hold the context manager for too long
            return results;
        }
    }
}

Things to note about this:

  • I want to avoid repeatedly recalculating the value. As long as the service and provider I'm using remain valid, I keep a cache. If those change, then I need to refresh the cache
  • I also try to use lazy-initialized values where possible
  • I use an IMonitor; this is a tool SMAPI gives me to do logging
  • To actually check, I do a look-ahead/-behind for where to search, which under the hood resolves into an absolute value of geodes cracked so far
  • Within a temporary scope, I modify the number of cracked geodes, and then get the values.

At this point, the implementation of GameStateContextManager and StardewManagedProperty matters, so that is below:

using StardewModdingAPI;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;

namespace Dannnno.StardewMods.Predictor.Shared
{
    /// <summary>
    /// Attribute that indicates a property is managed in the StardewValley game
    /// </summary>
    [AttributeUsage(AttributeTargets.Property)]
    public class StardewManagedPropertyAttribute : Attribute { }

    /// <summary>
    /// Class to manage state in Game1 that I might modify
    /// </summary>
    public class GameStateContextManager : IDisposable
    {
        /// <summary>
        /// Get the original values in all managed properties
        /// </summary>
        private IDictionary<string, object> ManagedPropertyOriginalValues { get; set; }

        /// <summary>
        /// Get the monitor to use for logging
        /// </summary>
        private IMonitor Monitor { get; set; }

        /// <summary>
        /// Get the game whose properties we want to rollback
        /// </summary>
        private IStardewGame Wrapper { get; set; }

        /// <summary>
        /// Get the list of properties we need to manage
        /// </summary>
        private IList<PropertyInfo> ManagedProperties { get; set; }

        /// <summary>
        /// Create a new instance of the context manager, with initial Game1 values
        /// </summary>
        /// <param name="wrapper">The game object we are going to be rolling back</param>
        /// <param name="monitor">The monitor to log debug messages to, if desired</param>
        public GameStateContextManager(IStardewGame wrapper, IMonitor monitor = null)
        {
            Wrapper = wrapper;
            Monitor = monitor;
            ManagedPropertyOriginalValues = new Dictionary<string, object>();

            ManagedProperties = wrapper.GetType()
                                       .GetProperties()
                                       .Where(prop => Attribute.IsDefined(prop, typeof(StardewManagedPropertyAttribute)))
                                       .ToList();

            foreach (var prop in ManagedProperties)
            {
                var value = prop.GetValue(wrapper);
                ManagedPropertyOriginalValues[prop.Name] = value;
                Monitor?.Log($"Current[{prop.Name}]: {value}");
            }
        }

        #region IDisposable Support
        private bool disposedValue = false; // To detect redundant calls

        protected virtual void Dispose(bool disposing)
        {
            if (!disposedValue)
            {
                if (disposing)
                {
                    // TODO: dispose managed state (managed objects).
                    // Reset the modified game stats

                    foreach (var prop in ManagedProperties)
                    {
                        var value = prop.GetValue(Wrapper);
                        var original = ManagedPropertyOriginalValues[prop.Name];
                        if (value != original)
                        {
                            prop.SetValue(Wrapper, original);
                            Monitor?.Log($"Current[{prop.Name}]: {value}, resetting to: {original}");
                        }
                    }
                }

                // TODO: free unmanaged resources (unmanaged objects) and override a finalizer below.
                // TODO: set large fields to null.

                disposedValue = true;
            }
        }

        // TODO: override a finalizer only if Dispose(bool disposing) above has code to free unmanaged resources.
        // ~GameStatContextManager()
        // {
        //   // Do not change this code. Put cleanup code in Dispose(bool disposing) above.
        //   Dispose(false);
        // }

        // This code added to correctly implement the disposable pattern.
        public void Dispose()
        {
            // Do not change this code. Put cleanup code in Dispose(bool disposing) above.
            Dispose(true);
            // TODO: uncomment the following line if the finalizer is overridden above.
            // GC.SuppressFinalize(this);
        }
        #endregion

    }
}

With a disposable object, I track certain properties for changes, and on dispose I undo those changes. This is not thread safe, which is a future planned enhancement but is not required in current state.

My implementation of GeodePredictor is the thing I dislike the most. I think I got lazy with encapsulation, but when I try to think of alternatives it gets tricky. For example:

  • GeodePredictor should almost certainly not be converting the look-ahead/-behind to an index, but if I don't do that then the caching behavior is either gone or much harder to write. I thought about pushing it down into StardewGeodeCalculator, but that doesn't feel quite right either
  • It should also not be making the temporary scope changes, but if I push that down then the rollback has to happen for every prediction, instead of just once. That isn't the end of the world, but I'd rather do one large rollback than a lot of small ones. I suppose the interface could expose some transactional concept, but that feels like a worse encapsulation leak

This is the part I'm maybe most interested in feedback on


Summary

  1. I made a mod. Its pretty fun, and once I get the UI working I might post that for review
  2. I tried to implement dependency injection to create a looser coupling between the game and my code, but in some places this slipped
\$\endgroup\$
  • \$\begingroup\$ Could you explain a little bit more about "finding geodes" (in the Goal part). I'm trying to wrap my head around how it works. In a method of the code you talk about "distance", but it's not explained at the beginning and as someone who never heard of that game I'm confused, but I'd like to post a review anyways since it's an interesting post. \$\endgroup\$ – IEatBagels Nov 11 '19 at 16:24
  • \$\begingroup\$ @IEatBagels I moved my section about how the game determines it up there, and added how I use distance and index in reference to the geodes. \$\endgroup\$ – Dannnno Nov 11 '19 at 16:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.