godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.49k stars 21.26k forks source link

Project Settings will return null falsely #92450

Open betalars opened 6 months ago

betalars commented 6 months ago

Tested versions

It happend on 4.2.2.stable but I think calling this bug "reprodcible" is kind of a stretch

System information

nixos unstable

Issue description

I use Project Settings to customize a Closed Captions Plugin I am currently working on.

So I define a setting:

var _text_scaling:String = "accessibility/closed_captions/text_scaling"

func _enable_plugin():
    ProjectSettings.set_setting(_text_scaling, 1)
    ProjectSettings.add_property_info({
        "name": _text_scaling,
        "type": TYPE_FLOAT,
        "hint": PROPERTY_HINT_RANGE,
        "hint_string": "0.5,5,0.1",
        "doc": "Define custom scaling."
    })
    ProjectSettings.set_initial_value(_text_scaling, 1)

    var error: int = ProjectSettings.save()
    if error: push_error("Encountered error %d when saving project settings." % error)

and try to access the setting later: captions_theme.default_base_scale = ProjectSettings.get_setting(_text_scaling)

However ProjectSettings.get_setting(_text_scaling) returns <null>.

Now comes the really weird part: I tried to reproduce it:

ProjectSettings.set_setting("bug/reports/number", 1)
ProjectSettings.add_property_info({
"name": "bug/report/number",
"type": TYPE_INT,
})
ProjectSettings.set_initial_value("bug/reports/number", 1)

var number:int = ProjectSettings.get_setting("bug/reports/number")

... but it works fine.

It also works fine, when I manually set a value other than the default in the Project Settings Editor.

Steps to reproduce

  1. git clone git@github.com:betalars/godot-closed-captions.git
  2. git lfs install, git pull again
  3. open Project
  4. (just to be sure) disable and enable the closed captions plugin.
    • [edit:] the settings are now being displayed in the advanced Project Settings tab
  5. run testing.tscn
  6. observe the bug
  7. set the text-scaling to something other than 1
  8. observe the bug disappearing

Minimal reproduction project (MRP)

see above

KoBeWi commented 6 months ago

_enable_plugin() only runs when you enable the plugin in the editor, not on every launch. You should put your code in _enter_tree().

betalars commented 6 months ago

_enable_plugin() only runs when you enable the plugin in the editor, not on every launch. You should put your code in _enter_tree().

Yeah, but the settings should only be available in the settings editor when the plugin is enabled. Because I have to remove the settings in _disable_plugin, I think the most logical thing is to enable them in _enable_plugin, even if that means I have to keep enablening and disabling the plugin during development, but that is not going to be an issue for users.

And even so, it still does not explain the behavior observed. When I run in to this issue, the settings are being displayed in the Project Settings Editor. Yet I still get a null value, that I cannot explain. (Also: see Steps to reproduce)

KoBeWi commented 6 months ago

Yeah, but the settings should only be available in the settings editor when the plugin is enabled. Because I have to remove the settings in _disable_plugin, I think the most logical thing is to enable them in _enable_plugin, even if that means I have to keep enablening and disabling the plugin during development, but that is not going to be an issue for users.

But _enter_tree() is called only when the plugin is enabled. You need to call add_property_info() and set_initial_value() in every editor session, otherwise they are undefined.

(Also: see Steps to reproduce)

Your project does not run due to errors. I made a new one using the information you provided: Settings.zip Turns out this is not really a bug, your project setting just does not exist in project.godot. It does not exist, because you use set_initial_value(), so your setting has default value, so it's not saved. If you intend to use setting at runtime you should either also define it at runtime or don't provide default value, so it's always overriden.

Maybe it could be clarified in the docs.

betalars commented 6 months ago

Your project does not run due to errors. Ah yes, git lfs. Sorry forgot to mention this.

I was suspecting something along those lines, because Godot has a habit of only storing things that are different from their default.

However ... even tho there is a workaround, and thanks for mentioning that, I would still argue this is a bug.

But I can write a bit of doc.

Just thinking where to put it. :thinking:

Possible places to look for this: 1 .Making Plugins

  1. the project settings page altough I am not sure where exactly to put it there: it kind of covers multiple methods, so I am leaning towards explaining it at the top of the page, but I am not sure if this is a good spot.
  2. A dedicated new tutorial page explaining all of this and linking it in the settings page.
  3. If this actually is a bug and will be patched: A disclaimer probably at the project settings page below the method descriptions.
betalars commented 6 months ago

But I suppose when all the things in godot always set their default values explicitly on ready, it is consistent to have this implementation as it is, even tho it is really counter intuitive when you don't work on the engine core.

Yeah ... I will write a doc.

Will put it as a comment here first so the info can be verified.

KerekesDavid commented 3 months ago

Damn, I also just ran into this, took me an hour to finally decide it must be a bug and search the issues. I'd vote for bug, it's very counterintuitive to not have a setting available after I just called set_setting().

At the least this should be a big warning on the ProjectSettings page.

I'm not quite sure of a neat workaround for my case. I wanted an autoload and the plugin itself neatly separated, so I have two files. First the plugin itself sets up the environment:

# plugin.gd
@tool
extends EditorPlugin

const AUTOLOAD_NAME = "PycoLog"
const SERVER_URL := "http://127.0.0.1:8000"

func _enter_tree():
    var setting_name := &"addons/pycolithics/server_url"
    if not ProjectSettings.has_setting(setting_name):
        ProjectSettings.set_setting(setting_name, SERVER_URL)
    ProjectSettings.set_initial_value(setting_name, SERVER_URL)
    ProjectSettings.set_as_basic(setting_name, true)

    add_autoload_singleton(AUTOLOAD_NAME, "res://addons/pycolythics-godot/pyco_log.gd")

func _exit_tree():
    remove_autoload_singleton(AUTOLOAD_NAME)
...

And then the autoload just reads the values when it loads:

# pyco_log.gd
extends Node

var url:String

func _ready() -> void:
    self.url = ProjectSettings.get_setting_with_override(&"addons/pycolithics/server_url")
...

So in this case I think I'll have to hard-code the default value in multiple places, which is not ideal.

berarma commented 1 week ago

This is the weirdest thing I've seen Godot do. Even putting it in the docs won't make up for the weirdness. I can't understand the reason behind. Why return null when even the settings editor knows it's not empty?

ProjectSettings knows the setting has a default value but it won't tell. This forces us to hard-code the default value every time we need to read the setting. We can't even get the initial value from code.

betalars commented 1 week ago

@berarma so if i am correct about it is: the reason is that only once a setting is changed is it being saved into the project settings file. The defaults will just be stored in memory and then forgotten once the editor is closed. Hence you need to refresh godots memory every time you load the AddOn.

I can see the technical reason behind it, but it is really counter-intuitive.

FYI: I am still planning on writing that AddOn doc thing, but am really busy with other stuff. May only happen next year. ._.

berarma commented 1 week ago

@berarma so if i am correct about it is: the reason is that only once a setting is changed is it being saved into the project settings file. The defaults will just be stored in memory and then forgotten once the editor is closed. Hence you need to refresh godots memory every time you load the AddOn.

I can see the technical reason behind it, but it is really counter-intuitive.

FYI: I am still planning on writing that AddOn doc thing, but am really busy with other stuff. May only happen next year. ._.

Not exactly. The settings windows will show the default value, so it's not forgotten, it's just the ProjectSettings object that will ignore default values.