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

KK and KKS codebases need to be merged #3

Closed
ManlyMarco opened this issue May 29, 2022 · 14 comments · Fixed by #18
Closed

KK and KKS codebases need to be merged #3

ManlyMarco opened this issue May 29, 2022 · 14 comments · Fixed by #18

Comments

@ManlyMarco
Copy link
Contributor

No description provided.

@ManlyMarco
Copy link
Contributor Author

@IDontHaveIdea Stiletto_KKS folder be removed now?
The Stiletto folder should be renamed to Stiletto.KK to match Stiletto.KKS I think.

@Splendide-Imaginarius
Copy link
Contributor

There may still be a few small improvements in Stiletto_KKS that aren't ported over yet, I haven't yet finished auditing for those. However, those are still discoverable via Git history anyway, so removing it from master doesn't seem like a bad idea. Agreed on renaming as well.

@ManlyMarco
Copy link
Contributor Author

@Splendide-Imaginarius Do you want to do the folder rename or should I do it? Also at what point would this be ready for a release?

@Splendide-Imaginarius
Copy link
Contributor

@ManlyMarco Feel free to do the folder rename if you like. I've skimmed the Stiletto_KKS code and I think the only remaining change that's worth porting over is the filtering feature. I'm going to try to port that sometime in the next week or two (haven't looked at the code very carefully yet, so not sure how hard it'll be to port); once that's done, I think tagging a release would make sense.

@Splendide-Imaginarius
Copy link
Contributor

I've skimmed the Stiletto_KKS code and I think the only remaining change that's worth porting over is the filtering feature.

It looks like Stiletto.Core actually does have an analogue of the filtering feature, but with a completely different implementation. I'm not sure which implementation is more robust; I'll need to test that. (In particular, this seems like the kind of functionality that could easily get very confused by whatever weird things kPlug does to add extra characters to H-scenes.)

@ManlyMarco
Copy link
Contributor Author

I don't use kPlug so I'll rely on your testing.

@Splendide-Imaginarius
Copy link
Contributor

So, at least on KK (didn't try KKS), I can't reproduce the behavior that motivated adding the filter to begin with. No extra characters are showing up in the Stiletto game GUI while in an H-scene in story mode. The filter in Stiletto.Core does exclude kPlug-invited characters, but choosing not to enable the filter results in exactly the behavior I'd want (the H-scene participants and the kPlug-invited characters show up, and no other characters do). So, unless there's some bug that's KKS-specific, I think the existing behavior in Stiletto.Core is fine.

I'd like to add the shoe warp controls to the game GUI, since it's kind of lame that I only added them to the maker GUI already; once that's done, I think we'll be good for tagging a release.

@ManlyMarco
Copy link
Contributor Author

ManlyMarco commented Sep 14, 2023

So, unless there's some bug that's KKS-specific

It's that, test it in KKS as well. Story mode is handled differently as far as character loading and unloading goes.

@Splendide-Imaginarius
Copy link
Contributor

OK, I'll give KKS a try. May take a little while for me to do so.

@Splendide-Imaginarius
Copy link
Contributor

Splendide-Imaginarius commented Nov 3, 2023

Stiletto.Core works fine regarding filtering in KKS with ksPlug: choosing "All Characters" in an H-scene in story mode yields only the main girl plus any girls invited via ksPlug (the other girls who were present on the map at the time the H-scene began do not show up). Choosing "Current Characters" shows only the main girl; ksPlug-invited characters are excluded. I see no problem with this behavior, I don't think there's any kPlug-related reason to try to import the legacy Stiletto_KKS filtering implementation.

@ManlyMarco are there any other edge cases (besides kPlug/ksPlug) I should test with the filtering before I conclude that Stiletto.Core's filtering is adequate?

@ManlyMarco
Copy link
Contributor Author

@ManlyMarco are there any other edge cases (besides kPlug/ksPlug) I should test with the filtering before I conclude that Stiletto.Core's filtering is adequate?

None that I can think of, really. I guess we release and people will yell if something doesn't work.

@Splendide-Imaginarius
Copy link
Contributor

Stiletto_KKS folder be removed now?

Covered by #17

The Stiletto folder should be renamed to Stiletto.KK to match Stiletto.KKS I think.

Still need to do this, I'll see if I can get to it soon.

ManlyMarco pushed a commit that referenced this issue Dec 12, 2023
As best I can tell, there is no remaining functionality in the legacy implementation that's not included in Core. So we should be able to safely remove the legacy implementation.
Refs #3
Splendide-Imaginarius added a commit to Splendide-Imaginarius/Stiletto that referenced this issue Dec 13, 2023
For consistency with Stiletto.KKS.

Fixes IllusionMods#3
@Splendide-Imaginarius
Copy link
Contributor

Once #18 is merged, I think we're ready to tag a release. I do have some other enhancements I want to attempt implementing, but there's no reason for them to block a release.

@ManlyMarco I do request that the various contributors who worked on Stiletto since the last release however-many-years-ago be credited in whatever release notes accompany the tag.

@ManlyMarco
Copy link
Contributor Author

Sure, it just needs a readme at this point.

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 a pull request may close this issue.

2 participants