jinxOAO / DSPmods_BepInEx

mods for DSP
4 stars 5 forks source link

[Compatibility] MoreProtoPages has a conflict with CommonAPI's TabSystem #43

Closed limoka closed 2 years ago

limoka commented 2 years ago

It seems your new mod MoreProtoPages is adding 6 new tabs. This is causing a conflict with a feature of CommonAPI that also adds tabs on request. I ask you to either rewrite your mod to utilize CommonAPI's TabSystem or rewrite all of your mods using it and deprecate it.

I believe such feature should be placed in a common library. Currently I'm trying to make CommonAPI a such library. I would be glad if you would be willing to cooperate. If you believe that my TabSystem doesn't have certain feature I'm willing to improve it.

You can find source code for this feature here. I don't have very deep documentation for it, as so far no mods use it, other than my WIP mod SignalNetworks. Here is short usage:

int tabId = TabSystem.RegisterTab($"{MODID}:{MODID}Tab", new TabData
(
    "SignalNetworksTabName",
    "Assets/SignalNetworks/Icons/signal-networks"
));

Here you can see the issue. With MoreProtoPages: изображение Without it: изображение

jinxOAO commented 2 years ago

Of course, Love your mods and I'd love to cooperate. Sorry I didn't realize this feature is already provided by commonAPI, and sorry to trouble you. Seems the best way is to deprecate the MoreProtoPages and change my other mods. And I might need some time to do that. But I have some questions.

  1. I don't understand {MODID}, how should I fill in the first parameter of TabSystem.RegisterTab and the iconPath of the new TabData? This issue prevents me from modifying my mod as I don't currently know how to use it. I'm very amateur as a mod developer, sorry about that. Hope you can help me understand how to fill in the parameters.

For example I've loaded the icons in the past like this

var ab = AssetBundle.LoadFromStream(Assembly.GetExecutingAssembly().GetManifestResourceStream("MoreProtoPages.addtypeicon"));
Sprite AddPageIcon = ab.LoadAsset<Sprite>("add1");
  1. If two mods want to share one new tab, can they try to register the same tab to minimize waste? It would be great if it could. Otherwise they must have one as a dependency, and the other will not register new tabs but use them directly. I think the latter is not so good.
JamieRCHI commented 2 years ago

Could this be why MoreProtoPages is not compatible with DSPMultiplyStackSize mod? The tabs are overlapping the buttons added by the DSPMultiplyStackSize mod.

jinxOAO commented 2 years ago

Could this be why MoreProtoPages is not compatible with DSPMultiplyStackSize mod? The tabs are overlapping the buttons added by the DSPMultiplyStackSize mod.

Seems right. Can't edit the stack size if MoreProtoPages is installed.

limoka commented 2 years ago
  1. First parameter to this function is a UNIQUE string ID of the tab. I have it to be $"{MODID}:{MODID}Tab", which resolves to SignalNetworks:SignalNetworksTab. It can by any unique string. MODID is just your mod id (name) as a string const. Here I use just my mod name, but I recommend using your full GUID. You can define that like so
    public const string MODID= "SignalNetworks";
  2. Regarding loading assets. I don't use this approach. Instead I prefer to load asset bundles created in Unity Editor. I have information on setting unity up. But for the purpose of creating bundle with images, you can use a simple Unity project with correctly set up paths. You can read about that here
  3. Two mods can use one tab. To do that they need to use common tab ID. In that case you can use a ID common for all your mods. For example GniMaerd.DSP.plugins:JinxOAOTab. Each mod will call RegisterTab(string ID, TabData tab) with same ID, and the first one to do so will make a tab, second and after will revive tab number.
jinxOAO commented 2 years ago

Thanks, I'll try that soon.

jinxOAO commented 2 years ago

I'm sorry I still can't load the icon and it keeps poping up errors. I'd like to deprecate MoreProtoPages directly, but because some other players have used the mod, and I haven't learned how to use the common API correctly to add new tabs yet. Therefore, I rolled back MoreProtoPages to the earliest version, which means that there are no additional tab icons, and all page can be switched by number keys. And I will add the commonAPI as its dependency, since that is the better way. Will that be ok? Now this mod should no longer interfere with the additional tabs or buttons created by the commonAPI. On the other hand, if the index of the new tab provided by the commonAPI is 3-9, the keys provided by moreprotopages can also match their buttons.

limoka commented 2 years ago

If you would show what you are doing, I will be able to help. Also if you have Discord, I can help you there. As for keybinds, they can be added to CommonAPI.

jinxOAO commented 2 years ago

That will be great. I was using

void Awake()
        {
            using (ProtoRegistry.StartModLoad(GUID))
            {

                //Initilize new instance of ResourceData class.
                string pluginfolder = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);
                resources = new ResourceData(GUID, "GMMoreProtoPages", pluginfolder); // Make sure that the keyword you are using is not used by other mod authors.
                resources.LoadAssetBundle("addtypeicon"); // Load asset bundle located near your assembly
                //resources.ResolveVertaFolder(); // Call this to resolver verta folder. You don't need to call this if you are not using .verta files 
                ProtoRegistry.AddResource(resources); // Add your ResourceData to global list of resources
            }

            TabSystem.RegisterTab($"{MODID}:{MODID}Tab", new TabData("mppnamegm", "assets/GMMoreProtoPages/add1"));

        }

and the assetbundle was like that in unity image

and the error image

limoka commented 2 years ago

Please show your plugins folder, especially folder with CommonAPI and with your plugin. Also make sure you don't copy CommonAPI with your plugin dll.

limoka commented 2 years ago

Your code and unity project looks almost right.

  1. Please move TabSystem.RegisterTab call inside the using statement.
  2. In code you have assets/GMMoreProtoPages/add1 as your path to icon. But in unity you can see assets/MoreProtoPages/add1. Please ensure that these are the same.
jinxOAO commented 2 years ago

Sorry my bad, forget to set copy to false. now it's image

image

limoka commented 2 years ago

You need a [CommonAPISubmoduleDependency(nameof(ProtoRegistry))] attribute on your plugin class. Like this. Also this is a recommendation and not required, but please move each mod to it's own folder.

jinxOAO commented 2 years ago

Now it's

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using BepInEx;
using HarmonyLib;
using UnityEngine;
using BepInEx.Configuration;
using System.Reflection;
using System.IO;
using CommonAPI.Systems;
using CommonAPI;

namespace MoreProtoPages
{
    [BepInPlugin("GniMaerd.DSP.plugin.MoreProtoPages", "MoreProtoPages", "1.0")]
    [CommonAPISubmoduleDependency(nameof(ProtoRegistry), nameof(CustomDescSystem))]
    public class MoreProtoPages : BaseUnityPlugin
    {
        public const string MODID = "MoreProtoPages";
        public static ResourceData resources;
        public static string GUID = "GniMaerd.DSP.plugin.MoreProtoPages";

        void Awake()
        {
            using (ProtoRegistry.StartModLoad(GUID))
            {
                //Initilize new instance of ResourceData class.
                string pluginfolder = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);
                resources = new ResourceData(GUID, "MoreProtoPages", pluginfolder); // Make sure that the keyword you are using is not used by other mod authors.
                resources.LoadAssetBundle("addtypeicon"); // Load asset bundle located near your assembly
                //resources.ResolveVertaFolder(); // Call this to resolver verta folder. You don't need to call this if you are not using .verta files 
                ProtoRegistry.AddResource(resources); // Add your ResourceData to global list of resources
                TabSystem.RegisterTab($"{MODID}:{MODID}Tab", new TabData("mppnamegm", "Assets/MoreProtoPages/add1"));
            }

        }

    }
}

and seems still the same error. image

image

limoka commented 2 years ago

Well as error suggests you need to add , nameof(TabSystem) to your statement.

jinxOAO commented 2 years ago

Thanks, That worked! I'll start to change my other mods now. Are you kind of an administrator of dsp.thunderstor.io? Can you please delete the MoreProtoPages after I updated my other related mod? Since I don't know how to do that...... Or what should I do about the MoreProtoPages if I want to deprecate it?

limoka commented 2 years ago

No. I'm not. You will need to ask Mythic on modding discord to do that. Or if you want I can ask him for you.

jinxOAO commented 2 years ago

Sure! I don't have a discord account. Hope you can help me for that. Thanks a lot!

jinxOAO commented 2 years ago

Thanks again. Your mods are useful and I've learnt a lot from you.

limoka commented 2 years ago

Also note that you need to register string with ID mppnamegm for tab text to be proper.

jinxOAO commented 2 years ago

got it!

limoka commented 2 years ago

While this is not exactly related I do need to note that you did not correctly register the tabs to be the same one. изображение You have two now.

limoka commented 2 years ago

And on another unrelated note. In your mods you clearly don't use full potential of CommonAPI's ProtoRegistry. When making your next mod related to new items and stuff, would you be willing to try it out? As for this issue, I believe it's resolved

jinxOAO commented 2 years ago

thanks for reminding. Yes this is intentional. Because i got another error of loading the same assetbundle occurred when I tried to register the same tab, and since the two did not belong to the same category, I just decide to register them separately. I'm happy to try it. Indeed I don't use full potential of CommonAPI's ProtoRegistry, because I first learned how to use LDBTool (And it had a video tutorial, plus, with my own language, so.). But I would love to try more about CommonAPI next time.

I do have a little suggestion. I suppose you've already noticed that the new tab icon is a little bit lower than the vanilla icons. So if you can adjusting the position of the button icon a little bit, that will be perfect. And the font size of Chinese tab name seems a little larger. But it was not a big problem. Thanks again. image

limoka commented 2 years ago

Did you try to just rename the bundle? Also what you can do is try to get the tab index using this method: https://github.com/kremnev8/CommonAPI/blob/c80bd35b925c07db60e913aef231da566621658c/CommonAPI/Systems/TabSystem/TabSystem.cs#L53 if you get a ArgumentException register it, otherwise use it.

jinxOAO commented 2 years ago

Yes, i've tried. Maybe I'll try to repack another bundle next time. Thank you for your patience. Just did not figure out how to use GetTabId(string tabId) since it is not static and I dont know where I can find an instance of TabSystem. Sorry if I'm being stupid, I didn't study C# systematically :(

limoka commented 2 years ago

I dont know where I can find an instance of TabSystem. Sorry if I'm being stupid, I didn't study C# systematically :(

You can't. That's a bug.

limoka commented 2 years ago

Another idea I have about loading two of the same bundle is to put a try catch to exception you get around it. Since the bundle is one, which ever first loads is going to provide the asset.

jinxOAO commented 2 years ago

Seems practicable. Will try that next time when I need to share tabs, thx.

limoka commented 2 years ago

I suggest you do that now. Your 2 tab is nearly empty. And there is no punishment to frequent releases.

jinxOAO commented 2 years ago

That worked.