Merge Jiarong's branch #6

Merged

Splendide-Imaginarius merged 24 commits from master+jia into master

2023-06-09 21:52:12 +00:00

Splendide-Imaginarius commented

2023-06-08 04:53:23 +00:00

(Migrated from github.com)

Fixes https://github.com/IllusionMods/Stiletto/issues/5 . Also fixes a bug that broke the knee bend feature.

I've tested the branch for a few days, and it seems to work OK. Here are the caveats:

While I don't see any obvious bugs in the behavior, I don't have the necessary knowledge to properly review all of the code changes themselves.
I didn't test Studio mode at all, so if there are bugs there, I wouldn't have noticed.
There's a lot of code churn here, which might make https://github.com/IllusionMods/Stiletto/issues/3 more difficult to solve.

Personally I don't think any of these caveats should block a merge, but it's not my repo, so my opinion matters little.

Fixes https://github.com/IllusionMods/Stiletto/issues/5 . Also fixes a bug that broke the knee bend feature. I've tested the branch for a few days, and it seems to work OK. Here are the caveats: * While I don't see any obvious bugs in the behavior, I don't have the necessary knowledge to properly review all of the code changes themselves. * I didn't test Studio mode at all, so if there are bugs there, I wouldn't have noticed. * There's a lot of code churn here, which might make https://github.com/IllusionMods/Stiletto/issues/3 more difficult to solve. Personally I don't think any of these caveats should block a merge, but it's not my repo, so my opinion matters little.

Splendide-Imaginarius commented

2023-06-08 04:55:12 +00:00

(Migrated from github.com)

There's a lot of code churn here, which might make https://github.com/IllusionMods/Stiletto/issues/3 more difficult to solve.

There is a chance that I might try to unify the KK and KKS codebases sometime after this PR is merged, but this may or may not happen soon, and my C# skills may or may not be enough to do it successfully.

> There's a lot of code churn here, which might make https://github.com/IllusionMods/Stiletto/issues/3 more difficult to solve. There is a chance that I might try to unify the KK and KKS codebases sometime after this PR is merged, but this may or may not happen soon, and my C# skills may or may not be enough to do it successfully.

ManlyMarco (Migrated from github.com) reviewed

2023-06-09 21:48:32 +00:00

ManlyMarco (Migrated from github.com) left a comment

To merge this with the KKS version it seems like the best way would be to scrap the current KKS codebase and port this version, adding any KKS specific hooks/tweaks that were added to the current version. You can find out what was changed in the KKS version in #2.

This looks like an almost completely different code base than what's currently in the repo. While it improves in a lot of areas it also regresses in others (e.g. no longer adding user-friendly maker controls, vastly increased code complexity, not using some of the newer features, etc.), so I'm hesitant to just blindly merge it. To merge this with the KKS version it seems like the best way would be to scrap the current KKS codebase and port this version, adding any KKS specific hooks/tweaks that were added to the current version. You can find out what was changed in the KKS version in #2.

Stiletto/Stiletto.cs

        public const string GUID = "com.essu.stiletto";

```suggestion public const string GUID = "com.essu.stiletto"; ```

ManlyMarco commented

2023-06-09 21:51:44 +00:00

(Migrated from github.com)

Since this repository doesn't have active work happening on it and doesn't host releases so far, I guess there's no real harm in merging this and improving it from there. If you could handle the KKS port it would be great.

I added a legacy v1 branch in case the old version needs to be updated.

Splendide-Imaginarius commented

2023-06-10 04:58:36 +00:00

(Migrated from github.com)

While it improves in a lot of areas it also regresses in others (e.g. no longer adding user-friendly maker controls, vastly increased code complexity, not using some of the newer features, etc.)

Could you clarify which maker controls and newer features you noticed are missing? I'd be happy to try my hand at re-adding them, if it's within my skill level.

> While it improves in a lot of areas it also regresses in others (e.g. no longer adding user-friendly maker controls, vastly increased code complexity, not using some of the newer features, etc.) Could you clarify which maker controls and newer features you noticed are missing? I'd be happy to try my hand at re-adding them, if it's within my skill level.

ManlyMarco commented

2023-06-10 07:57:22 +00:00

(Migrated from github.com)

Could you clarify which maker controls and newer features you noticed are missing? I'd be happy to try my hand at re-adding them, if it's within my skill level.

My bad, looks like the maker controls did get kept, just moved to a different file. In that case it's just about not using latest BepInEx and other nuget packages, which results in unnecessary reflection and other minor inefficiencies. Overall it's not a big issue, all it really does is make the codebase harder to understand.

> Could you clarify which maker controls and newer features you noticed are missing? I'd be happy to try my hand at re-adding them, if it's within my skill level. My bad, looks like the maker controls did get kept, just moved to a different file. In that case it's just about not using latest BepInEx and other nuget packages, which results in unnecessary reflection and other minor inefficiencies. Overall it's not a big issue, all it really does is make the codebase harder to understand.