Ability to manage fade out and in as independant operations along with more advanced fade_in_place() concept

[mouring]

Is your feature request related to a problem? Please describe.
I need to be able to independently control when to "fade out" and "fade in" occurs. My use case would be calling fade_in_place() to fade out, change the existing scene, and fade in. I don't trust that wait_time will be long enough to trust to do this via signal.

Describe the solution you'd like
I would like a new default_options:

• "fade_direction": enum { BOTH, IN, OUT } with the default of "BOTH". Where if you select IN or OUT it will only call _fade_in() or _fade_out(). (Can be implemented in 3.x and 4.0)

Describe alternatives you've considered
The fade_direction option could be done with direct API call (how I implemented it originally). e.g. func scene_fade_in(options), func scene_fade_out(options). Which could call _get_final_options() and then the correct _fade_in(..)/_fade_out(..).

This is the code I'm currently using in my project. I suspect the enum {} should be in SceneManagerConstants.gd, but for the simplity I threw it all into SceneManager.gd.

--- ../Scene-Manager/addons/scene_manager/SceneManager.gd	2022-04-06 22:18:59.000000000 -0500
+++ addons/scene_manager/SceneManager.gd	2022-04-07 16:49:41.000000000 -0500
@@ -12,6 +12,13 @@
 @onready var _animation_player := $AnimationPlayer
 @onready var _shader_blend_rect := $CanvasLayer/ColorRect
 
+enum fade_dir {
+	BOTH,
+	NONE,
+	OUT,
+	IN
+}
+
 var default_options := {
 	"speed": 2,
 	"color": Color("#000000"),
@@ -22,6 +29,7 @@
 	"no_scene_change": false,
 	"on_tree_enter": func(scene): null,
 	"on_ready": func(scene): null,
+	"fade_direction" : fade_dir.BOTH,
 }
 # extra_options = {
 #   "pattern_enter": DEFAULT_IMAGE,
@@ -96,11 +104,13 @@
 func change_scene(path: Variant, setted_options: Dictionary = {}) -> void:
 	assert(path == null or path is String, 'Path must be a string')
 	var options = _get_final_options(setted_options)
-	await _fade_out(options)
+	if options["fade_direction"] == fade_dir.OUT or options["fade_direction"] == fade_dir.BOTH: 
+		await _fade_out(options)
 	if not options["no_scene_change"]:
 		_replace_scene(path, options)
 	await _tree.create_timer(options["wait_time"]).timeout
-	await _fade_in(options)
+	if options["fade_direction"] == fade_dir.IN or options["fade_direction"] == fade_dir.BOTH: 
+		await _fade_in(options)
 
 func reload_scene(setted_options: Dictionary = {}) -> void:
 	await change_scene(null, setted_options)

[jabsatz]

I think that's possible. TBH I never personally used the fade_in_place() function, I just added it because it was very simple and seemed like a useful thing for users.

On that note, I'm interested in your opinion: would an enum be more comfortable to use than two separate, complete "fade_in()" and "fade_out" functions? Don't worry about how it works internally, that's more of an implementation detail. What I'm asking is do you think this

SceneManager.fade_in_place({ "fade_direction": SceneManager.fade_dir.IN })
SceneManager.fade_in_place({ "fade_direction": SceneManager.fade_dir.OUT })

is more comfortable than this?

SceneManager.fade_in()
SceneManager.fade_out()

Or is there any other reason for the enum option I'm not seeing?

[mouring]

fade_in() and fade_out() is more straight forward, and less likely to make a mistake when programming. It also allows for cleaner document as you can state what options are honored.

The only advantage of allowing it to be an option is:

fade_out(..)
[.. Do something ..]
change_scene(scene, { "fade_direction": SceneManager.fade_dir.IN })

So you don't take the hit of a double fade_out().

But it also makes the API less straight forward:

change_scene(scene, { "fade_direction": SceneManager.fade_dir.OUT })
[.. Try doing something on already queue_free()ed code ..]

The rest of the code would have to live in the new scene's _ready()
function (including the fade_in()). Which may not be as straight forward without very clear documentation.

This of course assumes you allow it in change_scene/reload_scene(). However the first case (change_scene + fade_in only) is a useful case. But one can fix that by adding option { "skip_fade_out" : true }. i.e:

fade_out(..)
[.. Do something ..]
change_scene(scene, { "skip_fade_out": true })

Not sure "NEITHER" as an option is useful, but I tend to be a completionist for code when a feature has no real cost.

[jabsatz]

Perhaps I could add our internal _fade_out and _fade_in methods to the public API as fade_out and fade_in (refactoring them a bit so they parse the options on each one).

Now that I think about it, the fade_in method could be pretty cool to have, for example when you want to have a transition start when you open the game.

I agree that skip_fade_out is necessary there, I can also easily add a skip_fade_in for completion sake. I'm a bit averse to adding enums to the API since I think the syntax ends up being kind of long.

I'd consider making the _replace_scene function public as well, but I don't know if that would introduce too much confusion. Maybe if we change its name and document it well? I'll leave that as a problem for another time anyway, for now we'll proceed with this solution:

• New api methods: fade_out(options) and fade_in(options)
• New options: "skip_fade_out": bool and "skip_fade_in": bool

[jabsatz]

Okay, I've implemented this feature on main, I've also done an... admittedly hasty merge back to the godot-4.x branch.

Sorry it's a bit rushed, but I'm leaving on vacation for a couple of weeks and will most likely not be able to make changes for a while. I'll still try to be responsive here so let me know if there are any issues.

[mouring]

Have fun on vacation. I may consider looking at writing a few more demos using these APIs in a good way.

[jabsatz]

Merged the fixes you sent @mouring, they look good to me.
I won't have a chance to update the asset lib version until I return, but if this is fixed can we close the ticket?