Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove GD.load() in C# in favor of ResourceLoader.load() #263

Open
Torguen opened this issue Nov 28, 2019 · 5 comments
Open

Remove GD.load() in C# in favor of ResourceLoader.load() #263

Torguen opened this issue Nov 28, 2019 · 5 comments

Comments

@Torguen
Copy link

Torguen commented Nov 28, 2019

Describe the project you are working on:
Just learning

Describe the problem or limitation you are having in your project:
The problem is that there is GD.load and ResourceLoader.load and the two methods do the same.

Describe how this feature / enhancement will help you overcome this problem or limitation:
Eliminating the most appropriate (I think only ResourceLoader should remain) will avoid confusion among newbies.

Show a mock up screenshots/video or a flow diagram explaining how your proposal will work:

Describe implementation detail for your proposal (in code), if possible:

If this enhancement will not be used often, can it be worked around with a few lines of script?:

Is there a reason why this should be core and not an add-on in the asset library?:

@aaronfranke
Copy link
Member

Makes sense, but it would be inconsistent with GDScript. As per the docs:

GDScript has a simplified @GDScript.load built-in method which can be used in most situations, leaving the use of ResourceLoader for more advanced scenarios.

Does anyone know what's supposed to make it simpler? Anything aside from not needing to write ResourceLoader.?

I think it might make sense to remove this from GDScript too, and just require people to write ResourceLoader.load(). This way it would free up the load name in case a user wanted to make their own method and call it load, and would make users more aware of ResourceLoader.

This appears to be the only method in GD.cs that just wraps another method of the same name.

@willnationsdev
Copy link
Contributor

I personally prefer having the load() method in GDScript. It 1) makes the code more readable and concise and 2) mimics the brevity of preload() for which there is no ResourceLoader equivalent.

@neikeq
Copy link

neikeq commented Dec 5, 2019

You can also static import Godot.GD so you can write Load instead of GD.Load.

@Shadowblitz16
Copy link

I honestly think Resources should be empty classes.
it makes custom game api's much easier to write and customize the way you want them.
they shouldn't be exposing any godot related members except to the devs of the game

@Xrayez
Copy link
Contributor

Xrayez commented Sep 30, 2020

I think it might make sense to remove this from GDScript too, and just require people to write ResourceLoader.load(). This way it would free up the load name in case a user wanted to make their own method and call it load, and would make users more aware of ResourceLoader.

Are all GDScript functions available in C# as well? If not, then I'm not sure whether consistency would be a winning argument for removing load from both languages. GDScript was created as an easy-to-use language. I cannot say the same thing about C#, which is used for anything nowadays it seems, not just game development, so the verbosity in C# is likely justified.

Regarding documentation, the reference to ResourceLoader should be added to the load method description 😕:

godot-gdscript-load

EDIT: added in godotengine/godot#42439.

And to be honest, there are quite a bunch of existing methods in GDScript which reserve those general keywords already, like seed(), so may be another issue in and of itself.

So, if that's a problem, I'd just remove GD.load without touching GDScript. 😛

@Calinou Calinou changed the title Remove GD.load in C# Remove GD.load() in C# Oct 8, 2020
@Calinou Calinou changed the title Remove GD.load() in C# Remove GD.load() in C# in favor of ResourceLoader.load() Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Ready for Review
Development

No branches or pull requests

7 participants