Move Popochiu settings config to ProjectSettings (fix #26)#200
Move Popochiu settings config to ProjectSettings (fix #26)#200
Conversation
mapedorr
force-pushed
the
feature/26-popochiu-settings-in-proyect-settings
branch
from
5464fcb to
ae3e1b5
Compare
| #endregion | ||
|
|
||
| #region Private #################################################################################### | ||
| static func _initialize_project_cfg( |
There was a problem hiding this comment.
It's just nitpicking, but what if this is called _initialize_project_setting() instead? I know it come in the config.gd file so maybe it doesn't make sense, but it would be more readable about what it does in the new context.
There was a problem hiding this comment.
I agree. I'll do the change.
|
|
||
| # Remove the JSON file if config says so | ||
| if _config.should_remove_source_files(): | ||
| if PopochiuEditorConfig.should_remove_source_files(): |
| ) | ||
| return | ||
|
|
||
| if $AnimationPlayer.get_animation_list().is_empty(): |
There was a problem hiding this comment.
This smells like a bugfix sneaked into this PR, right?
There was a problem hiding this comment.
More than a bugfix, I see it as a small UX improvement. Many messages like this appear in the Output:
They can be annoying during the early stages of development if animations are not yet ready, or if one has characters without animations.
This makes sure the logic about triggering the right animation is not done if there are no animations in the character.
| if $AnimationPlayer.has_animation(animation): | ||
| return animation | ||
| # No valid animation is found. | ||
| printerr('Animation not found %s' % [animation_label]) |
There was a problem hiding this comment.
This was probably intended as useful information for the developer, but I agree an error is too much.
Do you think we can log this info somewhere so that the dev has a log of missing animations that the engine expects to be played?
There was a problem hiding this comment.
I agree, but we already have a message that gives the proper context to devs. You can see an example of it in the screenshot that is part of the answer to your previous comment. I think there is no need to print this message if we already have a better one: it uses the Popochiu print style and gives a better feedback.
| @export var transition_layer: PackedScene = null | ||
| ## The time, in seconds, that will take the game to skip a cutscene. | ||
| @export var skip_cutscene_time := 0.2 | ||
| var skip_cutscene_time := 0.0 |
There was a problem hiding this comment.
Why the value change?
There was a problem hiding this comment.
Forget me, it's clear.
| gi.name = 'GraphicInterface' | ||
| else: | ||
| gi = load(PopochiuResources.GUI_ADDON_FOLDER).instantiate() | ||
| gi = load(PopochiuResources.GUI_GAME_SCENE).instantiate() |
There was a problem hiding this comment.
Is this ALWAYS available due to the default? What if a game has a custom GUI selected and no GUI is available to instantiate?
There was a problem hiding this comment.
What do you mean by "a custom GUI selected"?
The GUI scene should be always available because Popochiu creates the corresponding files for the GUI when one closes the Setup popup the first time. But thanks to your comment, I just remembered that I have to include a validation to prompt the user to select a GUI in case they come from a version of the plugin prior to beta 1 (I don't remember in which version we included the GUI templates).
| prints(ES) | ||
| prints(EN) | ||
| print_rich('[wave]%s[/wave]' % SYMBOL) | ||
| print_rich("[wave]%s[/wave]" % SYMBOL) |
There was a problem hiding this comment.
At this point I'm going to ask: should we always prefer double over single quotes for strings in the source code?
There was a problem hiding this comment.
I prefer double because it makes it easier to create messages that have apostrophes. Do you prefer single quotes?
| return _get_project_setting(SCALE_GUI, false) | ||
|
|
||
| #region Public ##################################################################################### | ||
| func initialize_editor_settings(): |
There was a problem hiding this comment.
Not sure where this one ended up. Are we still going to have editor settings for the plugin?
There was a problem hiding this comment.
Yes, but now they are in the res://addons/popochiu/editor/config/editor_config.gd script.
Parting from the progress made by @drbloop , now new sections for PopochiuSettings are added to ProjectSettings when the Editor starts. Next thing will be making sure values are properly saved and loaded, and remove the GUI and TransitionLayer path dependency, which will be only in the `popochiu_data.cfg` file.
When the game starts, Popochiu settings from the ProjectSettings are maped to the properties of the ProjectSettings class.
Enabling the Advanced settings won't be necessary Popochiu loads the GraphicInterface scene is from res://game/graphic_interface by default.
Also remove all calls related to save the settings file. [upd] Changing the game type and adding items to the inventory on start are now stored in the ProjectSettings.
Refactore accessing Popochiu editor and project settings with static classes PopochiuConfig and PopochiuEditorConfig. [upd] Do not show animation not found messages if the character AnimationPlayer node doesn't has animations.
mapedorr
force-pushed
the
feature/26-popochiu-settings-in-proyect-settings
branch
from
ae3e1b5 to
3817583
Compare
Add ignored editor_config.gd file. [upd] Minor code improvements in PopochiuClickable related to the double-click functionality.
Add functions to PopochiuResources to check if the project setup is done and there is a GUI template selected.
2 participants
This moves the settings related to Popochiu to a section in the ProjectSettings.