Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to R2API's ContentPack System #338

Merged
merged 22 commits into from Feb 17, 2022
Merged

Changes to R2API's ContentPack System #338

merged 22 commits into from Feb 17, 2022

Conversation

Nebby1999
Copy link
Contributor

@Nebby1999 Nebby1999 commented Feb 14, 2022

R2API currently manages any kind of added content by adding the Content piece to its internal content pack. while this works well in theory, it also produces the issue where All of the content is under R2API. So for example, things like Aetherium, Digger Unearthed and Direseeker are all int he same contentpack, instead of being in separate contentpacks. While ideally each mod should implement their own content pack manually, this Pull request attempts to fix this issue by making it so each mod that adds content gets their own ContentPack.

This entire thing is done thru the new Class called "R2APIContentManager". As the name suggest, it manages the content packs that r2api is creating for each mod that adds content. In a very, very simplified explanation:

  • When a content mod calls ArtifactAPI.Add(ArtifactDef), at the end of the method instead of adding the ArtifactDef to a list, which then gets added to the r2api content pack, the API calls R2APIContentManager.HandleContentAddition(Assembly callingAssembly, UnityEngine.Object obj).
  • The HandleContentAddition, as the name suggests, handles adding the content to a ContentPack, the ContentPack is made via a SerializableContentPack at first, and then its identifier is set to the mod's BepInName.

There are a few upsides to this method that arent present with the original r2api content handling system.

  1. Each Mod has its own ContentPack, the content pack itself is stored afterwards in a ReadOnlyCollection that's public, which allows mods to modify the content that's being added by a mod.
  2. Since each mod has its own contentpack, a modder can simply go to RoR2.ContentManagment.ContentManager and do a lookup of all the loaded contentpacks in the property "allLoadedContentPacks". This can allow the usage of LINQ to easily find the contentpack of a mod by simply using the mod's modname.
  3. Prefabs that have multiple components that uses indexes from catalogs will be added to each catalog separately. In pre-DLC ror2, prefabs like "ScavSackProjectile", which has both a CharacterBody and ProjectileController, gets added to only the CharacterBody catalog. which is an issue because the ProjectileController's index will be set to invalid. This however, changes in post DLC ror2, where said prefab would be added to both catalogs. HandleContentAddition() has a special method for GameObjects, where if the GameObject has, for example, both a CharacterBody and a ProjectileController, the GameObject itself would be added to both the Mod's ContentPack's BodyPrefabs and ProjectilePrefabs
  4. Ideally, All assets in RoR2 should have unique namings, this is so the Dictionaries in the catalogs that go from string to the Object can work properly. However, not all mods decide to name certain assets (Particularly stuff like SkillDefs.) When the contentManager receives a new content to handle, it'll try and see if the name isnt used in any of the other content packs that's been created (those being its own and ror2's own content pack.) if it finds a duplicate name, it'll create a custom, generic name using the ContentPack's Identifier, the Type of object, and an int. So for example, a skillDef from Direseeker with name " " would be now named DireseekerSkillDef1.

This Pull Request modifies a LOT of classes, as such, i've left most of the code that's been made reduntant in certain api's commented out. if this PR is about to be approved, i'll remove the comments, add documentation, add Logger messages for errors and run a code cleanup for the classes.

@Nebby1999
Copy link
Contributor Author

something that i didnt mention where the extensinve amount of changes ive done internally, i'll list them below:

  1. Due to the nature of how EffectDefs are created, and the way how the content manager handles the Effects themselves, i've deprecated EffectAPI's "AddEffect(EffectDef effect)". this is because the Content manager creates the effectDef in its handle content addition, and as such, passing an EffectDef becomes redundant.
  2. Most API's that had a method that called other methods inside of it to add content have been modified, a key example is UnlockableAPI, where all unlocks are simply managed via the AddUnlockableInternal, which itself accepts an Assembly so the unlockable can be added to the content pack properly.
  3. PrefabAPI now has a method that checks if a GameObject is going to be networked by it. this is used in the ContentManager for deciding wether or not to network a prefab (Since we handle networking prefabs made by prefabAPI's clone method.
  4. R2APIContentPackProvider no longer inheritsfrom IContentPackProvider, this is because R2API no longer has its own contentpack and instead creates them for each mod, this is important, since the IContentPackProvider interface was made with only it having 1 content pack per class.
  5. I've removed the private HashSets from loadoutAPI except for the skin defs, since they're now redundant.
  6. Removed commented code.

Hopefully i havent missed any important changes.

Copy link
Collaborator

@harbingerofme harbingerofme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me

Next time, try to restrict codemaid to only the files/sections you changed to make the reviewer's life easier.

R2API/ProjectileAPI.cs Outdated Show resolved Hide resolved
R2API/R2API.cs Outdated Show resolved Hide resolved
R2API/ItemAPI.cs Outdated Show resolved Hide resolved
R2API/SoundAPI.cs Outdated Show resolved Hide resolved
@Nebby1999
Copy link
Contributor Author

I will keep in mind both the Versioning thing and the Code cleanup only affecting the classes i've modified. I had to do the versioning stuff because it wouldn't load all the mods i had for testing because they required a version greater than 0.0.1, and i dont know how to change the version via build operations.

The "entires" is compeltely inexcusable and i'm suprised i didnt catch it before commiting.

@Nebby1999
Copy link
Contributor Author

The latest commit is an addition to the Object naming system that ensures there are no objects with Null, Whitespace or Empty names. this in turn fixes an issue where skill selections are not saved, because the game remembers the skills you choose depending on the skill's asset name. which shouldn't be Null, Whitespace or Empty.

@Nebby1999
Copy link
Contributor Author

This should be the final commit i do regarding new features to the system, however, i've made some mildly important changes to 2 classes.

  1. I've made "LoadRoR2ContentEarly" public, while all the other main methods are still private or internal, i decided to make it public so other mods can have access to the ReadOnlyContentPack of RoR2's ContentPack.
  2. I've added some public methods for the ContentManager, these public methods are:
    • ReserveSerializableContentPack(), as the name suggests, it simply reserves a SerializableContentPack by calling GetOrCreateSerializableContentPack() the method returns the created SerializableContentPack
    • AddPreExistingSerialziableContentPack(SerializableContentPack), This method allows a mod to add a PreExisting serializable content pack as their content pack. some of the usage scenarios include a ThunderkitMod adding the ItemDefs to their contentPack via ItemAPI so they can in turn use ItemAPI's IDRS systems. Additionaly, You can set a bool value to opt out from r2api loading the content pack automatically, this is very handy for mods that not only use r2api for only adding specific content, but also for mods that load the content manually with their own IContentPackProvider (Not to mention, adding the mod's content pack allows for a better experience on the avoidance of asset name duplication)
    • Both an "AddEntityState<>()" and an "AddContent()" methods, these methods can be used so an external mod can add any Content pack asset directly to their content pack. In addition, These two methods indirectly make any method that adds content directly to a content pack redundant (Example of these are BuffAPI, ArtifactAPI, EffectAPI, ProjectileAPI, SoundAPI's NetworkedSoundEvent, SurvivorAPI and LoadOutAPI's Skill, EntityState And SkillFamily adding methods).
  3. I've added some more exceptions to the R2APIContentManager, main examples include when someone uses the public methods to add content that's not supported by the content manager.

R2API/UnlockableAPI.cs Outdated Show resolved Hide resolved
@xiaoxiao921
Copy link
Member

I think creating a folder called ContentManagement and putting LoadRoR2ContentEarly, R2APIGenericContentPack and R2APIManagedContentPack under R2API.ContentManagement namespace doesnt sound like a bad idea

@Nebby1999
Copy link
Contributor Author

I think creating a folder called ContentManagement and putting LoadRoR2ContentEarly, R2APIGenericContentPack and R2APIManagedContentPack under R2API.ContentManagement namespace doesnt sound like a bad idea

I agree that this sounds like a good idea, i'd be willing to do that here too.

Also, should i mark BuffAPI, ArtifactAPI, ProjectileAPI, EffectAPI and SurvivorAPI as obsolete? considering anyone can just use the AddNewContent method instead of nameAPI.Add()

@Nebby1999
Copy link
Contributor Author

Lastest commit should finish everything, if we decide to deprecate the api's i've listed i'll do so.

R2API/BuffAPI.cs Outdated Show resolved Hide resolved
@Nebby1999
Copy link
Contributor Author

Nebby1999 commented Feb 16, 2022

Hopefully that'll be the final commit i have to do regarding Typos 💀

Regardless, the commit 55709f7 Marks the following things as obsolete:

Class Affected Thing marked Reason
ArtifactAPI Entire Class Artifacts can just be added via the ContentManager
BuffAPI Entire Class Buffs can just be added via the ContentManager
EffectAPI Entire Class Effect prefabs can just be added via the ContentManager
LoadoutAPI AddSkill, StateTypeOf, AddSkillDef and AddSkillFamily SkillDefs and SkillFamilies can just be added via the ContentManager, EntityStates can be added via it's "AddEntityState<>" method
ProjectileAPI Entire Class Projectile Prefabs can just be added via the Content Manager
SoundAPI AddNetworkedSoundEvent NetworkedSoundEvents can just be added via the ContentManager
SurvivorAPI Entire Class Survivor Defs, Bodies and even Masters can just be added via the ContentManager

Considering that some of these API's had some extra functionality (Mainly SurvivorAPI), i think we could add those into some kind of class called "VanillaFixes" and have that run every time r2api gets initialized? i'm unsure.

Also, i am aware that the ContentManager doesnt attempt to check if a content is "valid" (Like how projectileAPI adds a projectile only if it has the projectileController, or a better example, is that an ArtifactDef should really, REALLY have icons, or it'll throw errors ingame). i could make an extra class on a different PR called "ContentAdditionHelpers" where each content has its own Add method where we check for properly built content pieces.

Also removed AddContent() and AddentityState() from R2APIContentManager
@Nebby1999
Copy link
Contributor Author

ContentAdditionHelpers is a public class that can be used to add content pieces to the mod's ContentPack. Since now users should add content thru this class, i've deleted the two public methods in contentManager that added Content itself.

ContentAdditionHelpers has a method for each piece of content that's valid in a content pack and checks for common mistakes that would cause the Content to function erroneously (Such as an ArtifactDef not having icons).

While this class does contain methods for adding Elites, Items and Equipments, the documentation recommends to add those contents via their respective API's, and R2API will log what mods are adding these 3 content pieces via the ContentAdditionHelper. i've only added those methods for completion sake.

@Nebby1999
Copy link
Contributor Author

Final, FINAL commit for this pr, for real this time.

While the previous version for blocking more content entering the catalog worked allright since it was just one list that got created during R2APIContentPackProvider's methods. this doesnt work as smoothly with the current version of the content management.

I've seen mods that like to load their content and add them to their contentpacks in LoadStaticContentAsync from the IContentPackProvider. The previous system wouldn't allow mods to load their content asynchronously. this commit makes it so they can

There are 2 major changes in this commit:
1.- I've renamed the file "IgnoreAccessModifiers" to "AssemblyInfo". on this file, i've added the attribute [assembly: HG.Reflection.SearchableAttribute.OptIn]. this allows us to use the SystemInitializer attribute to initialize well, systems as ror2 does them (Notice how all the catalogs have this attribute on their init method.) The main reason for this is because i've also added the ContentBlocker class, it has a dictionary of Type to Bool, and can be used to test if a Catalog for a type has already initialized.
2.- Any private bools on existing API's now use the ContentBlocker's .GetAvailability method, Delegates that've made redundant (such as the ones called "avoid aditional entries" have been removed.

@nathanpovo
Copy link

nathanpovo commented Mar 7, 2022

For anyone that stumbles on this PR wondering where the deprecated classes have gone: you will have to use the static methods of the static class ContentAddition.

I could not find any documentation describing this change (the deprecated classes are still listed in the repository's wiki), just started getting compilation errors after updating to the latest version of R2API.

Is there any documentation or guidance on what has to be used with the R2APISubmoduleDependency attribute instead of the deprecated classes?

@Windows10CE
Copy link
Contributor

That change was actually made by me in #344, and using the methods in ContentAddition doesn't require any additional submodule dependencies. The change was documented in a Discord thread that contained all the changes made to both RoR2 and R2API, the wiki just hasn't been updated yet due to time constraints mostly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants