Allow content overrides via SMAPI

Pathoschild commented 8 years ago

Consider allowing mods to override images and content normally loaded from the game's *.xnb data files.

Current system

Mods which only change images or content currently overwrite the game's *.xnb data files. These mods have significant disadvantages:

They're hard to manage, since there's no way to know which files have been changed, or which mods changed which files.
They're hard to uninstall, since you need to recover the original game files.
They can't be used together if they edit the same file.
They can't patch a file, only fully replace it.
There's no standard install process (e.g. drop into Mods/).
In Stardew Valley 1.2+, many of the XNB files are duplicated for each supported language. That means XNB mods need a separate XNB replacement file for each language even for language-agnostic changes (e.g. prices, crop seasons, etc).

Pathoschild commented 8 years ago

Ideally mods could override content in two ways:

by including data in a Content subdirectory;
programmatically through an API.

SMAPI would need to support mods with no DLL, since some mods would only overwrite content.

Entoarox commented 8 years ago

Content directory is not how it is handled in Farmhand, thus not forward compatible
A easy API is not available in Farmhand, thus the same issue.

Further, this would require overriding the ContentManager, unless you also backport other functionality that the Farmhand content manager provides, mods will still prefer to rely on a implementation that does supply the functionality they need.

Pathoschild commented 7 years ago

This was discussed on the SDV discord. Here's a summary for reference, although it probably won't be added to SMAPI since Farmhand is supposed to be right around the corner.

Possible implementation

Raise events from a custom content manager

SMAPI could override the game's ContentManager with its own instance which intercepts content load requests, and raises events which let mods hook into the content loading process. These events could be used to edit data (e.g. add a new crop), provide new files (e.g. for mod use), or patch custom sprites into a spritesheet.

This part is pretty easy to implement, since ContentManager has a virtual Load<T> method: sample implementation

Resolve ID conflicts

The main complication is ID conflicts. Specifically:

mods must not reuse another mod's IDs (e.g. two mods adding a crop with ID=1000);
mods must always use the same ID for an item (so their content can be read from the save file).

One solution is to let mods request a named ID:

int pineappleID = content.GetID("Pineapple", KeyType.Crop);

The named ID would be permanently reserved for that save file. For example, in a smapi-data.json file stored in the save folder:

{
   "AssignedIDs": {
      "Crop::MoreCropsMod::Pineapple": 1000
   }
}

See also discussion at ClxS/Stardew-Farmhand#59 re ID conflicts, though no solution was reached there.

Example

Here's how a mod might use this to add a new crop:

public void Entry(IModHelper helper)
{
   ContentEvents.AfterRead += this.OnAfterContentRead;
}

public void OnAfterContentRead(object sender, ContentEventArgs content)
{
   // add crop data
   if(content.IsPath("Content\\Data\\Crops"))
   {
      var data = content.GetValue<Dictionary<int, string>>();
      var pineappleID = content.GetID("Pineapple", KeyType.Crop);
      data[pineappleID] = "crop data string";
   }

   // add crop sprites
   else if(content.IsPath("Content\\Tilesheets\\crops"))
   {
      Texture2D texture = ...;
      Rectangle position = ...;
      content.PatchSpriteSheet(texture, position);
   }
}

Kimi-Arthur commented 7 years ago

I think another point (or maybe the main point) of this issue is about overriding original resources, (besides the new crop thing mentioned).

Currently, there are a lot of mods out there only overriding existing portraits, characters etc. And these kinds of mods are hard to manage (copy files, back up files, remember which mod is in use etc). If somehow, the de facto standard modding API (SMAPI or Farmhand?) can define a standard layout for these kinds of mods (with only .xnb resource files overriding original files), and provide a "manager" to control what content is overridden by which mod, that will certainly make all the mod providers and users' life much much easier.

As always, thanks for all the efforts.

Bpendragon commented 7 years ago

My mods "Dynamic Horses" and "Dynamic Animals" actually attempt to do this, at least with Sprites. The problem is that data is loaded relative to the Content folder. which makes Helper.DirectoryPath not work as intended sometimes, instead I have to guarantee users use the correct folder name so I can go Path.Combine("..","Mods","DynamicHorses","Horses") which fails if the DynamicHorses directory name is changed.

For horses the general code is

Horse.sprite = new AnimatedSprite(Game1.content.Load<Texture2D>(Path.Combine("..","Mods","rest of path to horse"), 0, 32, 32);
Horse.sprite.textureUsesFlippedRightForLeft = true;
Horse.sprite.loop = true;

Other creatures are different because they use different sized sprites and I haven't looked a whole lot at anything beyond those.

However, I feel an excellent first step would be implementing a function in helper that retrieves the Mod's location relative to the StardewValley/Content folder. Which would allow mod developers to at least override the sprite directly.

Entoarox commented 7 years ago

You are making a mountain out of a molehill here, your issue can be solved simply be creating a new content manager, with the same display manager, but a different base path (Your mod folder)

This issue is about SMAPI natively supporting a mechanic used by EntoFramework and Farmhand to hook into the ContentManager and override certain behaviours.

Pathoschild commented 7 years ago

Discussed with @ClxS. Overriding the content manager like this shouldn't cause any trouble with Farmhand compatibility. Farmhand doesn't have an equivalent API yet, but creating one like this is a possibility to be determined later.

Pathoschild commented 7 years ago

Planning as a potential SMAPI 2.0 feature.

Use cases

These are the possible use cases for mods which override content:

use case	support in
add new entries to data content (new crops, NPCs, items, etc)	✓ 2.0
replace entries in data content (edit crop seasons, item prices, etc)	✓ 2.0
patch image data (edit existing sprites, add new sprites)	✓ 2.x
edit map data (edit layout, add new logic, create new areas, etc)	✘ edit maps after they're loaded

Events

SMAPI would likely expose two events:

event	effect
`ContentEvents.AfterFirstLoad`	Raised when an XNB file is read from disk. Mods can change the data here before it's cached.
`ContentEvents.AfterEveryLoad`	Raised each time an XNB file is read from the cache (including first read from disk). Mods can change the data here without affecting the cache (useful for dynamic values that should change depending on the current location, time, etc).

Helper

The event arguments would include a content helper that lets mods manipulate the content being read:

/// <summary>Encapsulates access and changes to content being read from a data file.</summary>
public interface IContentHelper
{
    /*********
    ** Accessors
    *********/
    /// <summary>The file path being read, with <c>\</c> delimiters and relative to the game folder (like <c>Content\path\to\file.xnb</c>) regardless of platform.</summary>
    string Path { get; }

    /// <summary>The content data being read.</summary>
    object Data { get; }

    /*********
    ** Public methods
    *********/
    /// <summary>Get the original data being read without any changes applied by mods.</summary>
    object GetOriginalData();

    /// <summary>Get the data as a given type.</summary>
    /// <typeparam name="TData">The expected data type.</typeparam>
    /// <exception cref="InvalidCastException">The data can't be converted to <typeparamref name="TData"/>.</exception>
    TData GetData<TData>();

    /// <summary>Get a globally unique ID for the given key.</summary>
    /// <param name="name">A key identifying the content for which to assign an ID.</param>
    /// <remarks>This method generates a permanent unique ID associated with the current mod. It can be called again with the same key to get the same unique ID.</remarks>
    /// <exception cref="ArgumentException">The specified name is null or empty.</exception>
    int GetID(string name);

    /// <summary>Add or replace an entry in the dictionary data.</summary>
    /// <typeparam name="TKey">The entry key type.</typeparam>
    /// <typeparam name="TValue">The entry value type.</typeparam>
    /// <param name="key">The entry key.</param>
    /// <param name="value">The entry value.</param>
    /// <exception cref="InvalidOperationException">The content being read isn't a dictionary.</exception>
    void SetDictionaryEntry<TKey, TValue>(TKey key, TValue value);

    /// <summary>Add or replace an entry in the dictionary data.</summary>
    /// <typeparam name="TKey">The entry key type.</typeparam>
    /// <typeparam name="TValue">The entry value type.</typeparam>
    /// <param name="key">The entry key.</param>
    /// <param name="value">A callback which accepts the current value and returns the new value.</param>
    /// <exception cref="InvalidOperationException">The content being read isn't a dictionary.</exception>
    void SetDictionaryEntry<TKey, TValue>(TKey key, Func<TValue, TValue> value);

    /// <summary>Overwrite part of the image. Patching an area outside the bounds of the image will expand it.</summary>
    /// <param name="texture">The image to patch into the content.</param>
    /// <param name="area">The part of the content to patch. The original content within this area will be erased.</param>
    /// <param name="patchMode">Indicates how an image should be patched.</param>
    /// <exception cref="InvalidOperationException">The content being read isn't an image.</exception>
    void PatchImage(Texture2D texture, Rectangle area, PatchMode patchMode = PatchMode.Replace);

    /// <summary>Replace the entire content value with the given value. This is generally not recommended, since it may break compatibility with other mods or different versions of the game.</summary>
    /// <param name="value">The new content value.</param>
    void ReplaceWith(object value);
}

/// <summary>Indicates how an image should be patched.</summary>
public enum PatchMode
{
    /// <summary>Erase the original content within the area before drawing the new content.</summary>
    Replace,

    /// <summary>Draw the new content over the original content, so the original content shows through any transparent pixels.</summary>
    Overlay
}

Sample mod code

This code adds a custom crop (both data and sprites):

public void Content_AfterFirstRead(object sender, ContentEventArgs content)
{
   switch(content.Path)
   {
      // add crop data
      case @"Content\Data\Crops":
      {
         int pineappleID = content.GetID("pineapple");
         content.SetDictionaryEntry(pineappleID, "crop data string here");
         break;
      }

      // add crop sprites
      case @"Content\Tilesheets\crops":
      {
         Texture2D texture = …;
         Rectangle position = …;
         content.PatchImage(texture, position);
         break;
      }
   }
}

Mods can also edit an existing data value:

content.SetDictionaryEntry("Mon8", value => value.Replace("*giggle*", "*laugh*"));

...or edit the underlying data directly:

var data = content.GetData<Dictionary<string, string>>();
data["Mon8"] = data["Mon8"].Replace("*giggle*", "*laugh*");

Pros & cons

Pros:

✓ Fairly easy to implement.
✓ Can recreate most XNB mods in code.
✓ Mod frameworks can add extension methods like content.AddCustomCrop(…).
✓ Mods can implement exotic features like seasonal sprites, location/time-aware dialogue, etc.
✓ Reduced conflicts compared to XNB mods, since (a) mods can change specific things instead of the entire XNB file, and (b) mods can generate unique IDs to avoid key conflicts.
✓ Simpler maintenance than XNB mods, since they won't need to be updated for every change to the XNB file.
✓ SMAPI mods can be added/removed easily; XNB mods can be difficult to uninstall unless you kept track of which files were edited and kept backups (or use an XNB mod manager).

Cons:

✘ Letting mods override content using a low-level API increases the risk of conflicts (especially if they don't use content.GetID(…)). While the risk is lower than current XNB mods, it may cause a negative perception among players re SMAPI's safety and stability.
✘ Some custom content will be stored in the save file. We should look into what happens if the mod is later removed.

Pathoschild commented 7 years ago

Prototype in SMAPI 1.9 beta

SMAPI 1.9 beta has a prototype of the content API (marked experimental so using it triggers a warning in the console). This is probably close to its final form if there are no objections.

Implementation

The ContentEvents.AssetLoading event passes an IContentEventHelper argument to event listeners:

/// <summary>Raised when an XNB file is being read into the cache. Mods can change the data here before it's cached.</summary>
/// <param name="sender">The event sender.</param>
/// <param name="e">The helper which encapsulates access and changes to the content being read.</param>
void ReceiveAssetLoading(object sender, IContentEventHelper content);

That IContentEventHelper interface provides the root methods that don't depend on the type. (Note the AsDictionary and AsImage methods.)

/// <summary>Encapsulates access and changes to content being read from a data file.</summary>
public interface IContentEventHelper
{
   /*********
   ** Accessors
   *********/
   /// <summary>The content's locale code, if the content is localised.</summary>
   string Locale { get; }

   /// <summary>The normalised asset name being read. The format may change between platforms; see <see cref="IsAssetName"/> to compare with a known path.</summary>
   string AssetName { get; }

   /// <summary>The content data being read.</summary>
   object Data { get; }

   /*********
   ** Public methods
   *********/
   /// <summary>Get whether the asset name being loaded matches a given name after normalisation.</summary>
   /// <param name="path">The expected asset path, relative to the game's content folder and without the .xnb extension or locale suffix (like 'Data\ObjectInformation').</param>
   bool IsAssetName(string path);

   /// <summary>Get a helper to manipulate the data as a dictionary.</summary>
   /// <typeparam name="TKey">The expected dictionary key.</typeparam>
   /// <typeparam name="TValue">The expected dictionary balue.</typeparam>
   /// <exception cref="InvalidOperationException">The content being read isn't a dictionary.</exception>
   IContentEventHelperForDictionary<TKey, TValue> AsDictionary<TKey, TValue>();

   /// <summary>Get a helper to manipulate the data as an image.</summary>
   /// <exception cref="InvalidOperationException">The content being read isn't an image.</exception>
   IContentEventHelperForImage AsImage();

   /// <summary>Get the data as a given type.</summary>
   /// <typeparam name="TData">The expected data type.</typeparam>
   /// <exception cref="InvalidCastException">The data can't be converted to <typeparamref name="TData"/>.</exception>
   TData GetData<TData>();

   /// <summary>Replace the entire content value with the given value. This is generally not recommended, since it may break compatibility with other mods or different versions of the game.</summary>
   /// <param name="value">The new content value.</param>
   /// <exception cref="ArgumentNullException">The <paramref name="value"/> is null.</exception>
   /// <exception cref="InvalidCastException">The <paramref name="value"/>'s type is not compatible with the loaded asset's type.</exception>
   void ReplaceWith(object value);
}

To simplify manipulating dictionaries and images, the AsDictionary and AsImage methods return a more specific content helper:

/// <summary>Encapsulates access and changes to dictionary content being read from a data file.</summary>
public interface IContentEventHelperForDictionary<TKey, TValue>
{
   /*********
   ** Accessors
   *********/
   /// <summary>The content's locale code, if the content is localised.</summary>
   string Locale { get; }

   /// <summary>The normalised asset name being read. The format may change between platforms; see <see cref="IsAssetName"/> to compare with a known path.</summary>
   string AssetName { get; }

   /// <summary>The content data being read.</summary>
   IDictionary<TKey, TValue> Data { get; }

   /*********
   ** Public methods
   *********/
   /// <summary>Get whether the asset name being loaded matches a given name after normalisation.</summary>
   /// <param name="path">The expected asset path, relative to the game's content folder and without the .xnb extension or locale suffix (like 'Data\ObjectInformation').</param>
   bool IsAssetName(string path);

   /// <summary>Add or replace an entry in the dictionary.</summary>
   /// <param name="key">The entry key.</param>
   /// <param name="value">The entry value.</param>
   void Set(TKey key, TValue value);

   /// <summary>Add or replace an entry in the dictionary.</summary>
   /// <param name="key">The entry key.</param>
   /// <param name="value">A callback which accepts the current value and returns the new value.</param>
   void Set(TKey key, Func<TValue, TValue> value);

   /// <summary>Dynamically replace values in the dictionary.</summary>
   /// <param name="replacer">A lambda which takes the current key and value for an entry, and returns the new value.</param>
   void Set(Func<TKey, TValue, TValue> replacer);

   /// <summary>Replace the entire content value with the given value. This is generally not recommended, since it may break compatibility with other mods or different versions of the game.</summary>
   /// <param name="value">The new content value.</param>
   /// <exception cref="ArgumentNullException">The <paramref name="value"/> is null.</exception>
   void ReplaceWith(IDictionary<TKey, TValue> value);
}

/// <summary>Encapsulates access and changes to dictionary content being read from a data file.</summary>
public interface IContentEventHelperForImage
{
   /*********
   ** Accessors
   *********/
   /// <summary>The content's locale code, if the content is localised.</summary>
   string Locale { get; }

   /// <summary>The normalised asset name being read. The format may change between platforms; see <see cref="IsAssetName"/> to compare with a known path.</summary>
   string AssetName { get; }

   /// <summary>The content data being read.</summary>
   Texture2D Data { get; }

   /*********
   ** Public methods
   *********/
   /// <summary>Get whether the asset name being loaded matches a given name after normalisation.</summary>
   /// <param name="path">The expected asset path, relative to the game's content folder and without the .xnb extension or locale suffix (like 'Data\ObjectInformation').</param>
   bool IsAssetName(string path);

   /// <summary>Overwrite part of the image.</summary>
   /// <param name="source">The image to patch into the content.</param>
   /// <param name="sourceArea">The part of the <paramref name="source"/> to copy (or <c>null</c> to take the whole texture). This must be within the bounds of the <paramref name="source"/> texture.</param>
   /// <param name="targetArea">The part of the content to patch (or <c>null</c> to patch the whole texture). The original content within this area will be erased. This must be within the bounds of the existing spritesheet.</param>
   /// <param name="patchMode">Indicates how an image should be patched.</param>
   /// <exception cref="ArgumentNullException">One of the arguments is null.</exception>
   /// <exception cref="ArgumentOutOfRangeException">The <paramref name="targetArea"/> is outside the bounds of the spritesheet.</exception>
   /// <exception cref="InvalidOperationException">The content being read isn't an image.</exception>
   void PatchImage(Texture2D source, Rectangle? sourceArea = null, Rectangle? targetArea = null, PatchMode patchMode = PatchMode.Replace);

   /// <summary>Replace the entire content value with the given value. This is generally not recommended, since it may break compatibility with other mods or different versions of the game.</summary>
   /// <param name="value">The new content value.</param>
   /// <exception cref="ArgumentNullException">The <paramref name="value"/> is null.</exception>
   void ReplaceWith(Texture2D value);
}

Sample usage

This example makes all crops available in any season (regardless of the locale being loaded):

private void ReceiveAssetLoading(object sender, IContentEventHelper content)
{
    // change crop seasons
    if (content.IsAssetName(@"Data\Crops"))
    {
        content
            .AsDictionary<int, string>()
            .Set((key, value) =>
            {
                string[] fields = value.Split('/');
                var newFields = new[] { fields[0], "spring summer fall winter" }.Concat(fields.Skip(2)).ToArray();
                return string.Join("/", newFields);
            });
    }
}

This example just inverts the colours of all textures:

private void ReceiveAssetLoading(object sender, IContentEventHelper content)
{
    if (content.Data is Texture2D)
    {
        // get texture pixels
        Texture2D texture = content.AsImage().Data;
        Color[] pixels = new Color[texture.Width * texture.Height];
        texture.GetData(pixels);

        // invert pixels
        for (int i = 0; i < pixels.Length; i++)
        {
            Color color = pixels[i];
            if (color.A != 0) // not transparent
                pixels[i] = new Color(byte.MaxValue - color.R, byte.MaxValue - color.G, byte.MaxValue - color.B, color.A);
        }

        // set data
        texture.SetData(pixels);
    }
}

Pathoschild commented 7 years ago

Proposed unique ID implementation

The main thing left is implementing the custom ID logic described in the before-previous comment. I was thinking it could work like this. Let's say there's a PineappleMod mod that just adds a custom pineapple crop:

Mod requests a unique ID for key Pineapple. SMAP internally assigns an ID to a unique internal key like PineappleMod::Pineapple, and saves the mapping in an appdata file. Generated IDs should be high enough to avoid conflicts with future vanilla items.
Mod injects the item data into ObjectInformation.xnb, Crops.xnb, and the texture file with that ID.
Right before data is serialised to the save file, SMAPI changes the item ID to 0 (weed) and sets the name to SMAPI/PineappleMod::Pineapple/1097/Pineapple (containing the internal key, assigned ID, and full item name).
After save, SMAPI switches the item back to normal based on that string.
[...]
When loading an arbitrary save file, SMAPI reads the custom items in the save file and does one of four things for each one:
- If the mod which added the item isn't loaded, leave the item as-is (in weed form) to avoid errors.
- Else if the item has an unknown key, register it with the given ID. (If the former ID conflicts with an assigned ID, change it.) Then unweed the item.
- Else if the item's former ID doesn't match the ID currently assigned to that key, transparently change the item's ID to match the assigned ID before unweeding the item. (This avoids issues with mods having already saved the assigned ID, e.g. in their Entry methods.)
- Else SMAPI simply unweeds the item based on that string.

With this approach...

Custom items don't break your save files. If you have custom items but stop using SMAPI or the mod that added them, your save will work fine; you'll just have some strangely-named weeds.
You can load arbitrary save files, even if they have different custom IDs, and SMAPI will transparently map IDs over.

Note that is a low-level API — the goal is to let mods add custom items without conflicting with each other, not to facilitate custom item behaviour. Mods will still need to handle that themselves. Simplifying that will be for a higher-level API in a future SMAPI version.

Kimi-Arthur commented 7 years ago

This will be very helpful indeed!

However, this does not solve the problem when people want to mix multiple XNB mods (e.g combining a portrait mod with a character mod, or even combining several portrait mods), especially existing XNB mods (without using SMAPI).

Or is this a non-goal actually?

Pathoschild commented 7 years ago

@Kimi-Arthur Supporting traditional XNB mods is a non-goal, because overriding the whole XNB file causes several problems that make them inherently incompatible with each other. That's especially true in Stardew Valley 1.2+, which has multiple translated versions of each XNB file — which means most existing XNB mods will only work if you're playing in English. There are a few ways SMAPI could load traditional XNB mods and try to inject them dynamically, but none of them are safe and robust.

Instead, the goal is to provide an API that lets SMAPI mods do the same things as traditional XNB mods, often more easily and more robustly. Writing a SMAPI mod does require programming, but modders can use the SMAPI content API to create framework mods to simplify overriding content (like @Entoarox's XNB Loader).

Kimi-Arthur commented 7 years ago

@Pathoschild Makes sense. Maybe it's better implemented as a mod based on this API. Thanks!

demiacle commented 7 years ago

@Pathoschild I like this! Hadn't dawned on me the items names would be serialized on the error objects... very nice! You could also assign the ID's dynamically to avoid any collisions or at least do a check against the games hardcoded index list on load and if, for example, SDV adds a new item and there is a collision, you could simply move it.

Also the parentSheetIndex is already saved to the object, actually most ( maybe all? ) of the item data looks to be present in the serialized error object. Additionally if you change all the items to weeds, I'm not sure how the stacking behavior would act especially since all of the items seem to serialize data, I'm guessing you might be able to stack weeds into the modded object and produce a stack of the modded object with non-vanilla properties.

My only thoughts concern custom textures, because injecting them might get messy if it won't be handled here because mods wont know what index smapi has given their object, although admittedly I haven't had time to look over any of the new stuff yet so maybe it is already taken care of.

Anyways I approve so far!

Pathoschild commented 7 years ago

@demiacle Thanks for pointing out the stacking rules. I looked into it, and this approach shouldn't affect item stacking because (a) the game won't stack items with different names, (b) the game doesn't consolidate existing stacks, and (c) the object type and other field values used by the stacking rules won't be changed. Changing the parent sheet index does affect stacking, but the name change accounts for that.

I don't think we need to handle collisions with vanilla IDs if we choose a starting index high enough, but just in case I'll make sure SMAPI tracks the type for each ID.

Texture injection should be fine. The parent sheet index is equivalent to the ID, and mods can get their items' unique IDs anytime with code like int parentSheetIndex = content.GetID(KeyType.Crop, "pineapple"). The value returned by that method is the source of truth — if the save file has a different ID for that item, the save data will be adjusted on load.

demiacle commented 7 years ago

Everything looks in order and sounds great to me!

TehPers commented 7 years ago

Currently, any assets that are loaded through Game1.content.Load(assetName) that don't already exist in the Content directory throw an error before the content events are called. This means that you can't actually "add" any assets to the game, just modify existing ones :(

Edit: #246 I tried to fix it, but this code is untested because I'm too lazy to fork SMAPI and figure out how to set up a dev environment for it.

Pathoschild commented 7 years ago

@TehPers I have a prototype event that lets mods inject the content for asset keys that don't exist, but I'm concerned that's a bad idea because it'll encourage key collisions (e.g. if two mods inject a customitems asset). Instead, what do you think about giving each mod a helper.ContentManager which loads files from their mod folder so they don't need to use content events to load their own content?

Entoarox commented 7 years ago

@Pathoschild Due to how SDV handles certain content types, the ability to inject custom content into the main content manager is a must, or there will simply be some situations where SDV's own build-in content behaviour causes issues.

If multiple mods use the same file, it is their own fault for not using mod-based prefixes....

Pathoschild commented 7 years ago

@Entoarox Could you give an example of when a custom asset key must be loaded through the main content loader? I want to make sure the implementation supports the main use cases.

Entoarox commented 7 years ago

Well, outdoor tilesheets, unless specially formatted are all treated as seasonal as you know, and rather then manually redo the seasonal implementation, using the default one could be useful. Unfortunately, CA's code makes heavy assumptions about seasonal tilesheets, they have to be in Content/Maps/ and be named , where the between the two is the only _ in there.

I could look for more examples, but that is one I happen to know from the top of my head due to having spend so much time figuring it out.

Edit: Just went and looked at the code for where I recalled a similar situation, and found it as expected, Buildings when loaded from save get their texture from Content/Buildings using the buildingType used to construct it, so another case where you need to be able to make the game believe a file is in a certain place even when it is not, simply because CA hardcoded things.

Edit 2: The same with NPC characters. I think you get the point by now? CA hardcoded a ton of cases like this for serializable objects to regain their textures...

Pathoschild commented 7 years ago

Mods can override the outdoor tilesheets fine, since they already exist. What they can't do yet is intercept an asset load for a key that doesn't match a vanilla content file. Or do you mean to support creating new maps altogether?

Entoarox commented 7 years ago

New tilesheets in existing locations, new locations with their own tilesheets, etc.

While mods can re-implement their own seasonal behaviour, with CA already having build it in, I see no reason not to make use of it when possible, not re-inventing the wheel and all that.

The same goes for other situations, especially Buildings and NPC, since serializer extensions should only be used if no alternative is available, being able to hook into those using CA's own mechanics is simply the best answer.

TehPers commented 7 years ago

I agree with Ento on this one. If there's a mod conflict, that's the mod creators' faults, not the API's fault. The conflict could have been avoided by using a prefix, and if a prefix couldn't be added, then the mods are incompatible without solution because they add/modify the same asset.

Pathoschild commented 7 years ago

I see three use cases for the mod's content manager so far:

Load a game asset (equivalent to Game1.content but with mod-specific error-handling).
Load a custom asset for internal use by the mod.
Load a custom asset for use in custom maps.

So ideally the content manager should support both vanilla and custom assets, without key collisions between vanilla/custom assets or between mods. One option is to automatically namespace custom keys:

// load a vanilla texture from the game
var letterSprite = helper.Content.Load<Texture2D>(@"LooseSprites\letterBG");

// load a custom texture from the mod folder
// (loaded into the content cache as "SMAPI\<mod ID>\letterBG" so it's available in maps)
helper.Content.Load<Texture2D>(@"~\letterBG");

(Note that the content manager is only for loading assets. Patching existing assets would be done through the content events.)

Pathoschild commented 7 years ago

This ticket covers too much to discuss and review it effectively, so I split it into three smaller tickets: #255 (content events), #256 (managing unique IDs), and #257 (local content managers). Please continue the discussion in those issues as appropriate.

Pathoschild / SMAPI