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

Allow level changing during Valerian PDA conversations (new game required) #168

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

SurDno
Copy link
Contributor

@SurDno SurDno commented Aug 25, 2024

If the goal is to stop the rest of the conversation from playing when the player goes to another location and then comes back, I believe a better way to handle it would be to track game loading (through device().precache_frame) and whether we're loading from a different level (which is already tracked by bind_stalker.script for emission starting purposes, but not exposed to other scripts), and if both conditions are met, stop the sound and skip the conversation entirely.

@SurDno
Copy link
Contributor Author

SurDno commented Aug 26, 2024

This doesn't look like it would work if the player loads an autosave created upon re-entry to the Cordon after exiting and re-launching the game, since bind_stalker.level_changed wouldn't be set then.

Why wouldn’t it be? The information about previous level is in the save file directly, so an autosave would have the previous location in it, and it does not matter if you travelled directly or closed the game and started a new session.

A save file made after an autosave will indeed not have the level_changed set to true, but it doesn’t need to - the first restrictor update will happen during autosave loading, way before you would actually be able to make a save. More than that, if it did, then any save file made during the conversation would interrupt it, as you came from another location at some point.

In the vanilla scripts, there is a function utils.level_changing() that uses a different method to detect level changes. That might work instead, but I'm not sure, because I don't understand why (or even if) it works as advertised.

I looked into it but couldn’t get it to work — can’t say if it’s me or if the function does not work as advertised. The solution I provided has been tested, however (although on vanilla, not SRP).

@Decane
Copy link
Owner

Decane commented Aug 26, 2024

I redacted my original review comment a few minutes after posting it upon realizing I was mistaken. For future reference, there is no need to reply to such comments - the misunderstanding had already been clarified.

@Decane
Copy link
Owner

Decane commented Aug 26, 2024

I've pushed a second commit to address the following minor issues:

  • Optional Features/#2. (Choose From This Folder Last)/Sleeping Bag + No Intro Movies/gamedata/scripts/bind_stalker.script had been missed
  • The name "level_changing" is ambiguous between loading out of a level and loading into one, so to leave no room for doubt, I changed it to "loading_into_level"
  • I dropped use of LTX section inheritance in gamedata/configs/scripts/escape/esc_pda_dialog_*.ltx because using it would necessitate noticing the need to begin from on_info2 when adding new on_info-type conditions into any of the inheriting sections, which would be easily overlooked
  • I deleted the useless %=stop_sound%; all sounds playing when loading out of the Cordon will anyway have been terminated by the xr_sound.stop_sounds_by_id() call in restrictor_binder:net_destroy(), so there would be no sound to resume upon re-entering the level

@SurDno
Copy link
Contributor Author

SurDno commented Aug 26, 2024

I assumed it was a GitHub issue, my apologies.

I prefer inheritance when reusing the same code piece throughout the logic file to make it easier to edit in a single place instead of every section where it is used, but your reasoning is valid — it can introduce new issues with conditions potentially being overwritten if one is unattentive. Inheritance is rarely used in logic in CS (I can only recall alternative Sakharov meet scheme in vanilla inheriting from his regular scheme to enable a different dialogue), but it is common in CoP and imo it makes the logic files easier to read (while agreeably being potentially harder to maintain).

I totally forgot about the existence of optional addons in SRP, and yeah, the other changes seem reasonable as well.

One minor thing if this is going to get implemented into the main branch — the strings previously used for level changer blockage should get removed from the localization files.

@Decane
Copy link
Owner

Decane commented Aug 26, 2024

the strings previously used for level changer blockage should get removed from the localization files.

Unfortunately, tips_wait_conversation_finish is also used in a few other logic scripts, so we can't get rid of it here.

Note to self: This PR is not safe to merge without a new game requirement. That's because if someone has a save where e.g. [sr_idle@4] in esc_pda_dialog_2.ltx is the active section, then after taking this PR, the disabled level changers would never get re-enabled in [sr_idle@7]. A similar consideration applies to esc_pda_dialog_4.ltx.

@Decane Decane changed the title Allow level changing during Valerian PDA conversations Allow level changing during Valerian PDA conversations (new game required) Aug 26, 2024
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