Skip to content

Conversation

Ark2000
Copy link
Owner

@Ark2000 Ark2000 commented Sep 10, 2024

  • Use godot resource instead of str_to_var(), so you can edit default config (default_panku_config.tres) in godot editor.
  • Implement a delay save mechanism so we can save everything when set_value and to better adapt something like slider value realtime change saving.

Copy link
Collaborator

@worron worron left a comment

Choose a reason for hiding this comment

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

Seems like some fixes required.

Comment on lines +66 to +75
static func _get_key(keys:Array[String]) -> String:
return ".".join(keys)

static func get_value(keys:Array[String], default:Variant = null) -> Variant:
var key := _get_key(keys)
var cfg := _get_config()
return cfg.get(key, default)

static func set_value(key:String, val:Variant) -> void:
var config = _get_config(USER_CONFIG_FILE_PATH)
config[key] = val
set_config(config)
static func set_value(keys:Array[String], val:Variant) -> void:
var key := _get_key(keys)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This cause many errors for me, since every PankuConfig.set_value/PankuConfig.get_value I can see have String as first argument, not array. I miss the point why key array and join used here.

Seems it works fine with key-value args

static func get_value(key: String, default:Variant = null) -> Variant:
	var cfg := _get_config()
	return cfg.get(key, default)

static func set_value(key: String, val:Variant) -> void:
	var cfg := _get_config()
	cfg[key] = val
	save_config_with_delay()

Comment on lines +9 to +12
"engine_tools": {
"time_scale": "1"
},
"engine_tools.time_scale": "1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is not it some kind of duplication? Which dictionary structure was originally planned: nested or plain with joined keys?


const INITIAL_DEFAULT_CONFIG_FILE_PATH = "res://addons/panku_console/default_panku_config.cfg"
const USER_CONFIG_FILE_PATH = "user://panku_config.cfg"
const INITIAL_DEFAULT_CONFIG_FILE_PATH = "res://addons/panku_console/default_panku_config.tres"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe we need to change file extension hint here too.

Also, do we need to keep default_panku_config.cfg file in project or may be we can delete it within this pull request?

@Ark2000
Copy link
Owner Author

Ark2000 commented Sep 11, 2024

a new pr has been upload #195

@Ark2000 Ark2000 closed this Sep 11, 2024
@Ark2000 Ark2000 deleted the dev branch September 11, 2024 15:05
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.

2 participants