LiveSplit.HollowKnight icon indicating copy to clipboard operation
LiveSplit.HollowKnight copied to clipboard

Don't split P4 as an ending

Open AlexKnauth opened this issue 2 years ago • 6 comments

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 avatar Jun 22 '23 20:06 AlexKnauth

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

AlexKnauth avatar Jul 06 '23 17:07 AlexKnauth

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

ShootMe avatar Jul 06 '23 17:07 ShootMe

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 avatar Jul 11 '23 23:07 AlexKnauth

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 avatar Jul 12 '23 03:07 AlexKnauth

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

  • [x] Normal mode, P5, Normal Embrace the Void with no flower (Cinematic_Ending_D)
  • [x] Normal mode, Godseeker Flower Quest, P5, Delicate Flower ending (Cinematic_Ending_E)
  • [x] Godseeker mode, P5 (Cinematic_Ending_D)
  • [X] 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 avatar Jul 12 '23 17:07 AlexKnauth

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: https://github.com/ShootMe/LiveSplit.HollowKnight/commit/1fd2f621e6d95bb3cd0ffb516c8bb89ee81275c0 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: https://github.com/ShootMe/LiveSplit.HollowKnight/pull/44 https://github.com/ShootMe/LiveSplit.HollowKnight/commit/07375ec2da8468f83a72ed10731eceaf9f3cc466 https://github.com/ShootMe/LiveSplit.HollowKnight/commit/0c487031bfa3fb9169d545d31b2ed0bff32f7451 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 avatar Jul 12 '23 22:07 AlexKnauth