Refactor Teleporter to a reusable SceneLink#2238
Draft
wjt wants to merge 4 commits into
Draft
Conversation
Transitions are implemented as follows. A fully-black ColorRect covers the whole screen. Its alpha channel is varied by a shader, which samples a monochrome mask texture and compares its red channel to a cutoff threshold to determine whether the alpha channel should be 0 or 1 (or a smoothed value in between). The cutoff is tweened from 1 to 0 when fading to black, and from 0 to 1 when fading from black. For the FADE (whole screen fades at once) and RADIAL effects, this works as expected. For RADIAL, the fade-out ends at a point in the middle of the screen, and the fade-in begins at that point. Fine. When a human reads LEFT_TO_RIGHT_WIPE they have a different expectation in the two directions. When fading out, we expect the blackness to start at the left and wipe across the screen to the right. When fading in, we expect the scene to appear at the left of the screen and wipe across the screen. If you think about it in terms of the mask texture, these are actually opposites of one another! The LEFT_TO_RIGHT_WIPE texture does what you would expect for the fade-in, but the opposite for the fade out. Invert the sense of directional transitions when fading out. This means that using LEFT_TO_RIGHT_WIPE for the fade-out has the expected effect of the black mask wiping across the screen from left to right. Adjust all code references accordingly. I'm not fixing all the scenes that use Teleporter because a future commit will change all those anyway. (Confusingly Teleporter refers to the fade-out as "enter" and the fade-in as "exit", but when thinking in terms of scenes you might imagine the fade-out is the "exit" and the fade-in is the "enter".)
If every call site of a method sets the optional parameters to the same values, then the defaults are wrong.
Before commit f7c6809, we had a mixture of bare teleporter.gd nodes, and instances of teleporter.tscn. In that commit I removed the scene: > teleporter.gd is a `@tool` and configures the correct layers on itself, > so I don't really see the value of having the scene. I now want to refactor the teleporter so the transition properties etc. can be shared with other nodes that can switch scenes. To do so, I find that I need to use a teleporter scene rather than a bare node...! Add such a scene, and update every scene in the game to use it, with the tool included here. It'll be removed in the next commit because I'm removing teleporter.gd but we'll have it for posterity.
|
Play this branch at https://play.threadbare.game/branches/endlessm/wjt/refactor-teleporter-to-a-reusable-scenelink/. (This launches the game from the start, not directly at the change(s) in this pull request.) |
Previously, Teleporter allowed you to specify the target spawn point and configure the scene transitions, but CollectibleItem and Cinematic did not. Annoying – I want the player to enter Fray's End from the south at the end of the tutorial, but the last action is to collect a thread so I couldn't. Rename Teleporter to SceneLink. Give it a fresh UID. Change its base class to Node2D rather than Area2D, and move the Area2D-specific logic to a helper node inside the Teleporter scene. Use my favourite niche feature of the Godot type system, as described in a comment. Use CONNECT_DEFERRED to avoid needing to put a call_deferred inside the teleport logic. Then, make Cinematic and CollectibleItem extend SceneLink; delete their bespoke scene-switching logic. Add validation of spawn_point_path. It's a bit weird for this to be a Node2D but it's necessary because CollectibleItem is a Node2D. (Cinematic has always been a Node2D rather than a Node – but this error has proven useful!) Change the default transition effects to FADE. This matches what Cinematic did by default before. It also doesn't look actively wrong if misconfigured, unlike the directional effects. Nonetheless, I also attempted to fix all teleporters to use the correct directions. I updated the splash screen to use this node directly. It's not really much help but it doesn't hurt either. I did not update the Sokoban rule engine or some other places that switch scenes in code to use this node because it doesn't really help much there. Resolves #2159
b994f1e to
a8c2918
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, Teleporter allowed you to specify the target spawn point and
configure the scene transitions, but CollectibleItem and Cinematic did
not. Annoying – I want the player to enter Fray's End from the south at
the end of the tutorial, but the last action is to collect a thread so I
couldn't.
Reintroduce a teleporter scene. (Before commit f7c6809, we had a mixture of bare teleporter.gd nodes, and instances of teleporter.tscn. In that
commit I removed the scene, and I now regret it.) Rewrite all scenes to use teleporter.tscn.
Then rename Teleporter to SceneLink. Give it a fresh UID. Change its base
class to Node2D rather than Area2D, and move the Area2D-specific logic
to a helper node inside the Teleporter scene. Use my favourite niche
feature of the Godot type system, as described in a comment.
Use CONNECT_DEFERRED to avoid needing to put a call_deferred inside the
teleport logic.
Then, make Cinematic and CollectibleItem extend SceneLink; delete their
bespoke scene-switching logic.
Add validation of spawn_point_path.
It's a bit weird for this to be a Node2D but it's necessary because
CollectibleItem is a Node2D. (Cinematic has always been a Node2D rather
than a Node – but this error has proven useful!)
Change the default transition effects to FADE. This matches what
Cinematic did by default before. It also doesn't look actively wrong if
misconfigured, unlike the directional effects. Nonetheless, I also
attempted to fix all teleporters to use the correct directions.
I updated the splash screen to use this node directly. It's not really
much help but it doesn't hurt either.
I did not update the Sokoban rule engine or some other places that
switch scenes in code to use this node because it doesn't really help
much there.
Resolves #2159
Side-quest: flip directional screen-wipe transitions to black
Transitions are implemented as follows. A fully-black ColorRect covers
the whole screen. Its alpha channel is varied by a shader, which samples
a monochrome mask texture and compares its red channel to a cutoff
threshold to determine whether the alpha channel should be 0 or 1 (or a
smoothed value in between).
The cutoff is tweened from 1 to 0 when fading to black, and from 0 to 1
when fading from black.
For the FADE (whole screen fades at once) and RADIAL effects, this works
as expected. For RADIAL, the fade-out ends at a point in the middle of
the screen, and the fade-in begins at that point. Fine.
When a human reads LEFT_TO_RIGHT_WIPE they have a different expectation
in the two directions. When fading out, we expect the blackness to start
at the left and wipe across the screen to the right. When fading in, we
expect the scene to appear at the left of the screen and wipe across the
screen. If you think about it in terms of the mask texture, these are
actually opposites of one another! The LEFT_TO_RIGHT_WIPE texture does
what you would expect for the fade-in, but the opposite for the fade
out.
Invert the sense of directional transitions when fading out. This means
that using LEFT_TO_RIGHT_WIPE for the fade-out has the expected effect
of the black mask wiping across the screen from left to right.
(Confusingly Teleporter refers to the fade-out as "enter" and the
fade-in as "exit", but when thinking in terms of scenes you might
imagine the fade-out is the "exit" and the fade-in is the "enter".)