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

Draft of Stage Module #492

Closed JaceDaDorito closed 1 year ago

JaceDaDorito commented 1 year ago

-Base of the Plugin -Stage Registration -Public Readonly Dictionary for people to quickly grab variants of of a locale

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

JaceDaDorito commented 1 year ago

I may be a bit blind, i do think that the StageRegistration.cs class is ok, however, it should follow the pattern of the partial classes of the module's main static classes. Like how StagesAPI class has the [AutoVersion] attribbute alongside being partial

that itself should be an easy fix, just move the constant strings to StageRegistration, and mark StageRegistration with the [AutoVersion] attribute and mark it as partial.

How come you want the plugin strings in the stage registration class instead?

JaceDaDorito commented 1 year ago

image image Just bug fixed the Stage Registration class. I registered Waffle House as a golemplains and ancientloft alt respectively and the weights are correct, making them proper alts.

Nebby1999 commented 1 year ago

I was thinking that this should probably have a separate class with static properties that give you the bacescenenameoverrides so it's easier to register variants

xiaoxiao921 commented 1 year ago

No, refer to them through the SceneDef, they are addressable, remember.

JaceDaDorito commented 1 year ago

I was thinking that this should probably have a separate class with static properties that give you the bacescenenameoverrides so it's easier to register variants

Probably not a good idea because the same system can be used for modded variants as well. For example, it should be able to register "FBLScene" as a valid fogbound variant. The list of baseSceneNames that can be used as a key for variants isn't fixed because of this. I can probably refer people to this page anyway.

JaceDaDorito commented 1 year ago

Wait do you mean a util that can input say "Aphelian Sanctuary" and it returns "ancientloft", or a list of valid keys that can be registered?

xiaoxiao921 commented 1 year ago

Like I said before no strings should be used for actual control flow, only indirectly, through addressables keys (the only valid use) -> SceneDef

JaceDaDorito commented 1 year ago

Like I said before no strings should be used for actual control flow, only indirectly, through addressables keys (the only valid use) -> SceneDef

yeah i was just wondering, i dont know if i misunderstood or not

JaceDaDorito commented 1 year ago

I already mentioned it in Discord but I approached a dilemma with implementation with the Addressable Scripts. The addressable scripts were designed to be used in source; in comparison R2API is compiled before use. For me to implement the Addressable scripts I would need to convert all instances of #if UNITY_EDITOR to if(Application.isEditor). Currently this doesn't work at the moment and I will investigate later.

We have some options though:

Option 1 (if it works): Continue trying to implement the Addressable scripts into R2API +Would keep the Stage Registration and Addressable scripts in one place. Developers would only need to download 2 packages: R2API.Stages and RoR2EditorKit. Users would only need to download 1 mod: R2API.Stages. -Has worse performance because if(Application.isEditor) is an active check while running the game. The #if UNITY_EDITOR clause and its code gets taken out when compiling, meaning the check wouldn't need to happen. Probably wouldn't drop performance by too much but its still a factor. -Would be harder to develop and maintain editor functionality because it would be in a separate package (RoR2EditorKit).

Option 2: Make the Addressable scripts (and possibly monster pool scriptable object) into its own package that would be paired with R2API.Stages. This package would be present in source instead similarly to Moonstorm Shared Utils. +Better Performance +Editor functionality for the Addressable Scripts would be bundled with the package so it would be in one place -Developers would have to download 3 different packages: R2API.Stages, Hypothetical Source Dependency, and RoR2EditorKit. Users would have to download 2 mods: R2API.Stages and the Hypothetical Source Dependency.

I would like to know opinions before I continue. As I said however, I will continue investigating why switching to Runtime checks don't work yet.

JaceDaDorito commented 1 year ago

Replying again because my edit didn't sync for some reason.

Option 3: Nuclear Option: Get rid of R2API.Stages entirely and just make it a source dependency that isn't directly connected. +All the positives above. Developers only need to download this dependency and RoR2EditorKit. Users only need to download this dependency. -Disconnected from R2API so it may have issues with maintenance . -May look less authentic and may not reach as many people.

(Also sorry for accidently closing the PR again, why is the close PR button right next to comment)

JaceDaDorito commented 1 year ago

Blacklister works as intended. I was able to replace golemplains2 with wafflehouse image

JaceDaDorito commented 1 year ago

Let me know if there can be optimizations to any of the code here. I'm planning on releasing this soon because I feel like I tested it thoroughly enough.

JaceDaDorito commented 1 year ago

This should be ready to publish on thunderstore. I didn't implement the alt vields, moon, etc. like I wanted to but that is ok. I will add that another time.

JaceDaDorito commented 1 year ago

Let me know when you guys plan to publish so I can publish my own dependency along side it.