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

Update Auto-Advance #1867

Merged
merged 31 commits into from
Nov 8, 2023
Merged

Conversation

CakeVR
Copy link
Collaborator

@CakeVR CakeVR commented Nov 3, 2023

This PR aims to change Auto-Advance's API and internals to match Auto-Skip.

Tasks to do:

  • Remove state info dictionary
  • Simplify Auto-Advance enable state handling

Change signals to `toggled`.
Change Auto-Advance method approach to variables.
Rename Auto-Advance enable flags.
@CakeVR CakeVR marked this pull request as ready for review November 7, 2023 07:17
Copy link
Collaborator

@Jowan-Spooner Jowan-Spooner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CakeVR Looks great so far. However I think it can be even better.

Most of my comments are on

  • removing unnecessary autoadvance in method names
  • some comment corrections
  • removing setter methods in favor of direct set() declaration on variable declaration.

Feel free to disagree with me if you think I'm wrong.

addons/dialogic/Modules/Core/subsystem_input.gd Outdated Show resolved Hide resolved
addons/dialogic/Modules/Core/subsystem_input.gd Outdated Show resolved Hide resolved
addons/dialogic/Modules/Text/auto_advance.gd Outdated Show resolved Hide resolved
addons/dialogic/Modules/Text/auto_advance.gd Show resolved Hide resolved
addons/dialogic/Modules/Text/auto_advance.gd Outdated Show resolved Hide resolved
addons/dialogic/Modules/Text/auto_advance.gd Outdated Show resolved Hide resolved
addons/dialogic/Modules/Text/auto_advance.gd Outdated Show resolved Hide resolved
addons/dialogic/Modules/Text/auto_advance.gd Outdated Show resolved Hide resolved
addons/dialogic/Modules/Text/auto_advance.gd Outdated Show resolved Hide resolved
@Jowan-Spooner
Copy link
Collaborator

Lgtm. Thanks!

@Jowan-Spooner Jowan-Spooner merged commit 160bcef into dialogic-godot:main Nov 8, 2023
@CakeVR CakeVR deleted the update-auto-advance branch January 1, 2024 21:47
Invertex pushed a commit to Invertex/dialogic that referenced this pull request Jan 26, 2024
* Fix incorrect description of `DialogicAutoAdvance`.

* Add `per_word_delay` variable.

* Add `per_character_delay` variable.

* Add variables for ignoring characters.

* Add `await_playing_voice` variable.

* Add `fixed_delay` variable.

* Add `override_delay_for_current_event` variable.

* Add `waiting_for_next_event` variable.

* Add `waiting_for_system` variable.

* Add `waiting_for_user_input` variable.

* Add `delay_modifier` variable.

* Remove `get_autoadvance_info` method.

* Rename variables and signals.

* Adjust API of Auto-Advance and Auto-Skip.
Change signals to `toggled`.
Change Auto-Advance method approach to variables.
Rename Auto-Advance enable flags.

* Move `ProjectSettings` configuration to `_init`.

* Add missing word.

* Move `class_name` before documentation.

* Fixed variable doc referencing itself.

* Improve signal receiver name.

* Rename `start_autoadvance` to `start`.

* Simplify method naming.

* Remove setters for stacking enable flags.

* Move `class_name` up.

* Use delay override variable directly.

* Rename `_emit_autoadvance_enabled`.

* Remove invalid access.

* Use correct user-input enable flag.
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.

None yet

2 participants