Save subsystem: Change behavior and API to be more intuitive

Is your feature request related to a problem? Please describe. The current API for functions can cause a little bit of friction when using them. For example, when requesting an array of all slot names with get_slot_names() while taking advantage of Godot's type system. As it returns an Array, if you tried to cast it to an Array[String] it would cause an error and you would have to iterate over the array to append the names instead or cast each element when using it. As all elements will be guaranteed to be a string, it would make sense to change the signature to func get_slot_names() -> Array[String] instead.

Additionally, some API's can fail, but will only push an error to the output while the game will continue execution like normal. For example, when calling load("bogus save"), in this example "bogus save" does not exist and will push an error to the console before continuing on as if nothing happened. You could call has_slot(slot_name) and do a check before, but as this function gets called within the load() it seems a bit double that you would have to call it in order to control the flow of error handling at runtime. Currently, the load() function signature looks like func load(slot_name := "") -> void, instead of returning void (aka nothing) i think it should either return a bool signaling if the load operation succeeded or not, or it should return a status code in the form of an enum value in order to allow fine grained control where a function can fail because of multiple reasons.

such an enum could look like

enum ErrorCode {
    SUCCESS,
    SLOT_DOES_NOT_EXIST,
    ...
}

An example usage could look like:

var error := Dialogic.Save.load("save_b")

if error:
    # something went wrong, handle_error will set up the error response
    handle_error(error)
else:
    # there were no errors, tell the user everything went fine
    display_load_message()

In other cases, where the cause of the error is not directly related to dialogic's workings, it might make more sense to make use of the built in Error enum, as a lot of the IO related operation inside of the Save module's functions actually return a code if something goes wrong, so bubbling that up to the developer can be useful for if they want to go about handling each error separately.

Describe the solution you'd like With what I showed earlier, These are the changes I would suggest:

func save(slot_name := "", is_autosave := false, thumbnail_mode:=ThumbnailMode.TAKE_AND_STORE, slot_info := {})
- Changed to: func save(slot_name := "", is_autosave := false, thumbnail_mode:=ThumbnailMode.TAKE_AND_STORE, slot_info := {}) -> Error
- Note: If is_autosave is set to true and autosaves are disable in the settings, it will not actually save. As this is intentional, i think returning Error.SUCCESS seems fit.
func load(slot_name := "") -> void
- Changed to: func load(slot_name := "") -> Error
func save_file(slot_name:String, file_name:String, data:Variant) -> void
- Changed to: func save_file(slot_name:String, file_name:String, data:Variant) -> Error
func set_global_info(key:String, value:Variant) -> void
- Changed to: func set_global_info(key:String, value:Variant) -> Error
func get_slot_names() -> Array
- Changed to: func get_slot_names() -> Array[String]
func delete_slot(slot_name:String) -> void
- Changed to: func delete_slot(slot_name:String) -> Error
func add_empty_slot(slot_name: String) -> void
- Changed to: func add_empty_slot(slot_name: String) -> Error
func reset_slot(slot_name := "") -> Error
- Changed to: func reset_slot(slot_name := "") -> Error
- Note: You would return the call to save_file()
func set_slot_info(slot_name:String, info: Dictionary) -> void
- Changed to: func set_slot_info(slot_name:String, info: Dictionary) -> Error
func save_slot_thumbnail(slot_name:String) -> void
- Changed to: func save_slot_thumbnail(slot_name:String) -> Error

Describe alternatives you've considered Dealing with the status quo.

Additional context The current behavior makes a couple of assumptions that things will go right, which in most cases this will be fine. But in the cases where trouble does pop up, it might cause confusion. So making things more rigid could be useful.

Changing func get_slot_names() -> Array to func get_slot_names() -> Array[String] is technically a breaking change, but would only break in cases where the returned array would be appended by objects that aren't strings.

dialogic-godot / dialogic

Save subsystem: Change behavior and API to be more intuitive #2168