Swap arguments of ResourceSaver.save() #61647
Conversation
|
Another thing that could be considered would be adding something like One concern I have with the default value for path is that for a new Resource which hasn't been saved yet, the default output of calling No strong opinion either way, would be good to get @reduz's input on this too. |
Maybe the error message could just be clearer, to say that you either need to provide path or the Resource itself needs a path. |
KoBeWi
force-pushed
the
SaverResource
branch
2 times, most recently
from
b3d8cf3 to
f1d0043
Compare
|
The most natural for me has always been the path as first parameter. I don't mind that too much though. What I really think is bad is the path as an optional parameter with a default value. |
|
@reduz approved this during the PR meeting without a strong opinion either way. |
| @@ -174,7 +174,7 @@ void ResourceSaver::remove_resource_format_saver(Ref<ResourceFormatSaver> p_form | |||
| ResourceSaver *ResourceSaver::singleton = nullptr; | |||
|
|
|||
| void ResourceSaver::_bind_methods() { | |||
| ClassDB::bind_method(D_METHOD("save", "path", "resource", "flags"), &ResourceSaver::save, DEFVAL((uint32_t)FLAG_NONE)); | |||
| ClassDB::bind_method(D_METHOD("save", "resource", "path", "flags"), &ResourceSaver::save, DEFVAL(""), DEFVAL((uint32_t)FLAG_NONE)); | |||
There was a problem hiding this comment.
What was the decision on @neikeq's suggestion to not have a default value for the path?
There was a problem hiding this comment.
I think its expected that it has a default value, it makes it clear that if you already specified a path it will re-use it.
|
Needs a rebase, otherwise I think this can be merged now. |
|
Thanks! |
Whenever I use
ResourceSaver.save(), I intuitively try to put the Resource as first argument and path as second, only to discover that the actual order is reversed. So I reversed that order. IMO it makes sense to put resource first, as it's more important. This also allows making the path an optional argument and saving will default to the resource's own path.