risk-of-thunder / R2API

A modding API for Risk of Rain 2
https://thunderstore.io/package/tristanmcpherson/R2API/
MIT License
136 stars 55 forks source link

R2API.ContentManagement is the biggest codesmell I've contribued and needs rectification. #511

Open Nebby1999 opened 10 months ago

Nebby1999 commented 10 months ago

(I hate my code)

In all seriousness, ContentManagement, while it was a good idea, needs to be refactored to be modified to use BepInPlugins instead of obtaining the assembly via the calling method.

Using the assembly causes issues as it can sometimes create unecesary content packs. Or overall unintended behaviour

This rectification should be done in a clean way ideally to avoid mods breaking, which could be difficult.

The other option of course, is to do nothing and completely axe ContentPack frameworks from R2API once Seekers of The Storm releases, and let modders properly use IContentPackProvider.

harbingerofme commented 10 months ago

How do you account for multiple bepinplugins in a single assembly?

Nebby1999 commented 10 months ago

Is there even a mod that does this?

xiaoxiao921 commented 10 months ago

There are some mods that do this though this is a non issue because of how PluginInfo work, it's just a member field tied to the BaseUnityPlugin instance so not a problem.

The problem with the current methods is that removing them would be breaking, so the best we can do is marking them as obsolete and provide overload that take a PluginInfo instead

yekoc commented 10 months ago

The existing methods can be re-routed to the plugininfo by reflecting through the plugin attribute (finding the specific instance of BaseUnityPlugin isn't necessary because BepIn itself maintains a mapping between plugin id and info). The issue regarding GetCallingAssembly pointing back to R2API during internal dispatch (which ContentManagement does have a bit more layers of then it needs) can be mitigated by climbing the stack frame,which isn't ideal,but would work fine for our purposes. I can confirm that there are mods that bundle multiple plugins but it has been long enough since that was discussed that I don't remember any specific examples.

Priscillalala commented 9 months ago

Is all the hassle to split content added by contentmanagement into separate content packs worthwhile?

yekoc commented 9 months ago

There are some mods that count on it (primarily WAILA,though it's possible that there are other consumers that aren't advertising it)

Priscillalala commented 9 months ago

Alright, here are my thoughts on content management then

That is what I was thinking about regarding the external API. Internally, content management has its own problems:

I have some interest in working on this content management rewrite; I am not sure how busy Nebby is or how much they wish to work on it though.

Nebby1999 commented 9 months ago

We can work it together if you'd like, currently i have my final defense on my thesis on the 11th, after that i'm officially on vacation.