search

Alphablackwolf / SkillPrestige

6 stars 17 forks source link

Rework YACS adapter to add most space core based skills #24

Closed noah1510 closed 2 months ago

noah1510 commented 2 months ago

This PR refactors the YACS skill adapter to work with (probably) all space core skills. At the moment all skills have to be added manually due to the way the ModHandler is implemented (only one class of a type is allowed there). The added skills are:

Due to how I implemented this, none of these mods is listed as a dependency any more. All that is required to build this is space core.

Please excuse the possibly bad code quality, I have now clue about C# development and wrote this in a simple text editor. The code compiles and all the skills are listed in the statue (I didn't test SlimingSkill). I haven't tested if this actually works but don't see why it shouldn't.

note: that this pr is based on my other one (#23), so you can either just merge this one or merge the other one first in case you want these changes with separate merge commits.

noah1510 commented 2 months ago

here is the most recent build, if you want to test it yourself: SkillPrestige.SpaceCore 1.4.0.zip

feedback is welcome

Alphablackwolf commented 2 months ago

Looks good, there's some minor name changes I might make still, but I need to push up some vital bugfixes first. This is close to the direction I was planning to go in, but there are some gotchas I have to check for in my code and in those skill mods, if the application of a profession is atypical in any way I'll need to add specific handling for each profession. (e.g., vanilla combat professions sometimes add directly to the players health stat and thus require specific handling when added or removed) YACS didn't have anything like that so it was fine, but I haven't reviewed the others yet. I would also need to test that it loads correctly when the relative skill mod is not present

noah1510 commented 2 months ago

okay makes sense Should I move the adapter classes to individual files or do you want to do that just for the ones that need custom changes?

Alphablackwolf commented 2 months ago

I'll tackle it when I can pull the changes in after the multiplayer fix

Alphablackwolf commented 2 months ago

Structurally rewriting the YACS folder turned out to create merge conflicts that were causing issues, so I've done my implementations locally and will push out with the next version, I used this to scaffold my changes, so thank you!