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

KKS port and project code merge #9

Merged
merged 14 commits into from
Aug 13, 2023
Merged

KKS port and project code merge #9

merged 14 commits into from
Aug 13, 2023

Conversation

IDontHaveIdea
Copy link
Contributor

Hi,

Just:

  • added Stiletto.Core project
  • moved code from Stiletto project to Stiletto.Core updated dependencies
  • added configuration for conditional compilation
  • added Stiletto.KKS updated dependencies
  • added conditional compilation steps to compile the plug-in for KKS

Did not look in depth into Stiletto_KKS since the code is so different it was easier to work on the Stiletto code base.

I don't use Studio so no testing there. It seems to work fine in KKS.

For the method OnHeelInfoUpdate had to use try {} block. It gives and error the second time you invoke Maker in the same
game session. Don't have KK installed for testing this change in the code for KK. Though logic dictates that the problem should be present in KK also. The nullchecks fix if active will permit the plug-in to continue to run as is the case in KKS.

Copy link
Contributor

@ManlyMarco ManlyMarco left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! The changes look good overall, just some minor issues to button up.

If you are going with the KK codebase, are you sure the KKS version didn't have anything worth keeping? e.g. fixes or UI changes.

Stiletto.Core/StilettoMakerGUI.cs Outdated Show resolved Hide resolved
Stiletto.KKS/Properties/AssemblyInfo.cs Outdated Show resolved Hide resolved
Stiletto.KKS/Stiletto.KKS.csproj Outdated Show resolved Hide resolved
@IDontHaveIdea
Copy link
Contributor Author

Just a fast check the code for the Stiletto_KKS compiles as is for KK but I can't test if it works in KK.

@ManlyMarco
Copy link
Contributor

Alright, let me know when it's ready for a review.

@Splendide-Imaginarius
Copy link
Contributor

Nice work! I'm happy to give it a try on KK and see if anything is obviously broken.

@Splendide-Imaginarius
Copy link
Contributor

No obvious breakage in about an hour of testing in both KK and KKS (mostly KK). I'll try to do some additional testing in the next few days, but so far LGTM.

@Splendide-Imaginarius
Copy link
Contributor

Tested in KK for another couple hours, no issues connected to this PR. I did find a bug in the shoe warp feature, but I'm 90% sure that's a bug in master branch that I accidentally introduced while optimizing that feature at the last minute, not this PR's fault -- I'll send in a PR to fix that after this PR is merged, so that we don't have to deal with merge conflicts.

@IDontHaveIdea is there anything you still need to do before this gets merged?

@ManlyMarco ManlyMarco merged commit 25a9e03 into IllusionMods:master Aug 13, 2023
@ManlyMarco
Copy link
Contributor

@Splendide-Imaginarius I merged it since the issues appear to have been resolved. Feel free to send PRs now :)

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.

3 participants