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

When using Styles, loading a save doesn't complete #1959

Closed
sudisk opened this issue Dec 16, 2023 · 0 comments · Fixed by #1960
Closed

When using Styles, loading a save doesn't complete #1959

sudisk opened this issue Dec 16, 2023 · 0 comments · Fixed by #1960
Labels
Bug 🐞 Something isn't working Critical 💩

Comments

@sudisk
Copy link
Contributor

sudisk commented Dec 16, 2023

The problem

Describe the bug
When loading a save, the UI refresh is incomplete, and the dialogue is stuck and not all resources are correctly updated.

What is expected when
image

To Reproduce
Steps to reproduce the behavior:

  1. Create a style
  2. Create a Timeline
  3. Create a few control buttons in a topmost Canvas Layer for ease of use (Start/Stop/Save/Load)
  4. Start the timeline and save at any point
  5. Click the Load button
  6. See error

Expected behavior
The display should be identical to the standard timeline run.

Screenshots
Expected (this is an actual timeline screenshot):
image

After load:
image

The background is not shown, the text isn't displayed and the "next" indicator isn't visible.

System (please complete the following information):

  • OS: Windows 10
  • Godot Version: 4.2stable
  • Dialogic Version: [e.g. 1.0]

Solutions

Code changes in the DialogicGameHandler.gd, subsystem_background.gd files (see possible fixes)
Additional code changes in subsystem_styles.bg that applies for any style change with background may be needed too.

Workaround
None.

What you tried to get the feature working

Possible fixes

DialogicGameHandler.gd:

In method load_full_state:

  1. The 'Styles' subsystem is referred to as 'Style'. This may be from an older version of Dialogic.
  2. The call to "start_timeline" comes too early. If we compare to the "start" method, the timeline must be loaded after the layout is fully loaded and ready.

These two changes restore most of the functionality of the load function.

subsystem_background.gd

The previous fix does not fully solve the issue. The background still won't show until there is a background change in the timeline.
The root cause of the issue is that a new Layout scene is loaded and until the previous layout is fully unloaded, which may happen later, there are multiple backgrounds registered as "dialogic_background_holders".

There are multiple solutions to this issue. The most trivial one would be to prevent switching layout, but that may not be what is expected depending on the save. The second solution implies making certain that we target the right layout scene when loading resources.

In method update_background:

  1. Make sure that the DialogicNode_BackgroundHolder control that we target is the right one, that is the one belonging to the newly created style.
  2. There is an optimization that doesn't work when loading a save. At line 50, there is a check that the argument parameter is different from the current state of the background. However the current state is used as the argument, which makes the method exit without having loaded the background. Removing this check makes the background being able to be loaded again.

Implementing the first change requires filtering the get_tree().get_first_node_in_group() result with only result(s) that belong to the new layout instance. The filtering could be done directly in the update_background method, but I suggest putting this filtering in the Style subsystem as it is may be a global style related need. However some users may want to remove the Style subsystem if they are certain to use only one dialogue style in their app. Therefore it is mandatory to check if the Styles subsystem is used and use the global method in this case. The code could even be made reusable by putting it in a new method in DialogicGameHandler.gd, but I did not see any need for it elsewhere.

Implementing the second change is quite straightforward at first glance. Just remove lines 50-51 in subsystem_background.gd solves the issue. This may cause however an optimization issue. A better solution may be to add an optional parameter to the update_background method to force updating of the background, in this case the background would be updated no matter what.

I have already done most of this changes locally, and I will submit a pull request with my own implementation of the changes.

sudisk added a commit to sudisk/dialogic that referenced this issue Dec 17, 2023
@Jowan-Spooner Jowan-Spooner added Bug 🐞 Something isn't working Critical 💩 labels Dec 18, 2023
@Jowan-Spooner Jowan-Spooner added this to the Version 2.0 (beta) milestone Dec 18, 2023
Jowan-Spooner added a commit that referenced this issue Dec 19, 2023
* Restore loading a save when using Styles (#1959)

Restore load functionality.

* Small adjustments

---------

Co-authored-by: Jowan-Spooner <raban-loeffler@posteo.de>
Invertex pushed a commit to Invertex/dialogic that referenced this issue Jan 26, 2024
…gic-godot#1960)

* Restore loading a save when using Styles (dialogic-godot#1959)

Restore load functionality.

* Small adjustments

---------

Co-authored-by: Jowan-Spooner <raban-loeffler@posteo.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐞 Something isn't working Critical 💩
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants