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

Don't split P4 as an ending #69

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AlexKnauth
Copy link
Contributor

@AlexKnauth AlexKnauth commented Jun 22, 2023

An attempt to fix issue #54 by adding a constraint on "GG_End_Sequence" endings that the scene name does not start with "GG_Hollow_Knight" removing "GG_End_Sequence" implicit endings. I hope this can prevent the cutscene after Pure Vessel from triggering an ending.

I cannot build or test this for now, I am away from my Windows computer. Tested on normal Godhome Ending, delicate flower ending, and Godseeker mode ending to make sure they still split endings based on "Cinematic_Ending" versions.

@AlexKnauth
Copy link
Contributor Author

I've built the dll, but I won't have time to actually go through P4 with it to test this for a few more days

@ShootMe
Copy link
Owner

ShootMe commented Jul 6, 2023

Reach out to slaurent, cerpin, Flibber, or mayonnaisical on discord. They are the ones that deal with the autosplitter changes these days.

@AlexKnauth
Copy link
Contributor Author

AlexKnauth commented Jul 11, 2023

After testing it in P4, it looks like this did not work. I had a livesplit file that had "Pure Vessel" and then the end labeled "Hollow Knight". When I completed P4, it split "Pure Vessel" then played the cutscene, and then split the end "Hollow Knight", so that's not good.

I might have missed something in the scene constraints: Since it split "Pure Vessel", it was on the scene GG_Hollow_Knight, and then it transitioned from one of GG_End_Sequence or GG_Door_5_Finale. I don't know what GG_Door_5_Finale is, so I had just assumed that it was going to GG_End_Sequence. Then the cutscene, which I had assumed was in GG_End_Sequence, might have actually been GG_Door_5_Finale. So when it transitioned from GG_Door_5_Finale to GG_End_Sequence after the cutscene, that would explain why it thought it was an ending. Since a transition from GG_Door_5_Finale to GG_End_Sequence is not a transition from GG_Hollow_Knight to GG_End_Sequence, my code still thought it was an ending.

So perhaps I should change

(nextScene == "GG_End_Sequence" && !sceneName.StartsWith("GG_Hollow_Knight"))

to

(nextScene == "GG_End_Sequence" && !sceneName.StartsWith("GG_Hollow_Knight") && !sceneName.StartsWith("GG_Door_5_Finale"))

Edit: or alternatively I could try

(nextScene == "GG_End_Sequence" && sceneName.StartsWith("GG_Radiance"))

but to do that I would have to test P5 as well as testing P4

Further Edit: and I would need to test P5 in godseeker mode as well as normal

@AlexKnauth
Copy link
Contributor Author

AlexKnauth commented Jul 12, 2023

I found a problem while testing the code in my comment above that might be relevant for another issue with ending splits: when I tried (nextScene == "GG_End_Sequence" && !sceneName.StartsWith("GG_Hollow_Knight") && !sceneName.StartsWith("GG_Door_5_Finale")) at first it didn't work the way I wanted, but when I added nextScene != sceneName to it as well, then it worked.

That other issue is something I remember coming up in All Achievements runs, where they get both the Sealed Siblings ending (Void Heart THK w/ Hornet) and the Dream No More ending (normal Radiance) back to back, but they needed a "nothing split" in between so that it didn't split both at once on the first ending.

If a nextScene != sceneName constraint is added to all the endings, not just godhome endings, then that problem might go away. (Edit: after more testing, no, that doesn't work)

@AlexKnauth
Copy link
Contributor Author

AlexKnauth commented Jul 12, 2023

So of the 3 4 ways to do Godhome Ending I've tested 2 3 of them:

  • Normal mode, P5, Normal Embrace the Void with no flower (Cinematic_Ending_D)
  • Normal mode, Godseeker Flower Quest, P5, Delicate Flower ending (Cinematic_Ending_E)
  • Godseeker mode, P5 (Cinematic_Ending_D)
  • Godseeker mode, P5 again: "what about 2nd breakfast?" (still Cinematic_Ending_D)

In both of the ones I tested, it split when Cinematic_Ending_D (or Cinematic_Ending_E) started, so the GG_End_Sequence code wasn't even triggered.

I don't think the Delicate Flower ending will be different in that respect. At most it might be a different letter like Cinematic_Ending_E or whatever idk. Confirmed Delicate Flower ending splits on Cinematic_Ending_E.

So what ending situation would trigger the GG_End_Sequence ending that wouldn't be covered by the Cinematic_Ending versions? Can the GG_End_Sequence code be deleted?

Edit: it occurs to me that I didn't test doing P5 a second time in the same godseeker file, so I should try that before I go deleting code

@AlexKnauth
Copy link
Contributor Author

AlexKnauth commented Jul 12, 2023

After testing P5 a second time in the same godseeker file, that still goes to Cinematic_Ending_D, and ends there. I don't see any situation where it should end on GG_End_Sequence, so what's the history behind that being added?

I have only been testing on current patch... is it because it was different on older patches of Hollow Knight? Is there another reason I haven't thought of? Or was it a mistake that can be deleted, subsumed by Cinematic_Ending_D and Cinematic_Ending_E?

Diving into git history, it looks like it was added in this commit: 1fd2f62
Which has a changelog entry Fix ending splits in Pantheon ILs. So it looks like the intention at the time was to make P1-P4 stop the timer so that Pantheon IL split files could work even if they had "endTriggeringAutosplit": false like full-game run split files.

That was 5 years ago. Now, it's recommended that IL split files have "endTriggeringAutosplit": true and have the last split stop the timer instead of having an implicit ending stop the timer. It looks like that's been the recommendation since 2 years ago and these commits: #44 07375ec 0c48703
Which have a changelog entry Removed all non-credits implicit endings (aluba, PoP, p1 zote, etc). Add them as autosplits + enable `End-triggering autosplits` to use. The standard Pantheon IL split files now all have "endTriggeringAutosplit": true for P1-P4.

My opinion based on this is that the GG_End_Sequence should be deleted from the endings. But this would potentially break old pantheon IL split files from before that change 2 years ago, in the same way that it would have broken old split files for Aluba, PoP, etc.

@AlexKnauth AlexKnauth marked this pull request as ready for review July 13, 2023 01:47
@AlexKnauth AlexKnauth changed the title WIP: don't split P4 as an ending Don't split P4 as an ending Jul 13, 2023
exclude GG_End_Sequence from endings

godhome ending uses Cinematic_Ending_D or Cinematic_Ending_E depending on flower, so StartsWith "Cinematic_Ending" covers all of it
@AlexKnauth AlexKnauth marked this pull request as draft May 18, 2024 15:07
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.

2 participants