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

Lambda expressions in GDScript #12170

Closed
wants to merge 4 commits into from
Closed

Conversation

dbsGen
Copy link
Contributor

@dbsGen dbsGen commented Oct 17, 2017

No description provided.

dbsGen added 2 commits March 5, 2017 22:35
Squashed version of godotengine#2704.

Edit by Rémi Verschelde: Squashed all changes in one commit and fixed some
indentation issues. Also applied clang-format to match the new master branch.
@ghost
Copy link

ghost commented Oct 17, 2017

pretty_print copyright headers are outdated (without contributors / year 2016 / outdated website link)?

@groud
Copy link
Member

groud commented Oct 17, 2017

This is not a good solution. I think there are a lot of good ideas in this PR but you should create different PR for the several enhancements you are doing.
From what I can read, this PR:

  • Adds a LEN operator to gdscript
  • Adds inverse_lerp and range_lerp to gdscript
  • Replaces OP_EXTENDS by OP_IS
  • Implements Lambda expressions.

@dbsGen
Copy link
Contributor Author

dbsGen commented Oct 17, 2017

@Noshyaar I missed this, and it is fixed.
@groud This brach(PR) is only for the lambda expression, you can write Lambda expressions like this:

  extends Node

  func _ready():
    var val = "1"
    var prin = func():
      print("s" + val);
    prin();
    hello(func():
      print("woqu")
    ,3)
  func hello(f, z):
    f()

I just rebase the source code and make the Lambda expressions could run on the newest gdscript.

@27thLiz
Copy link
Contributor

27thLiz commented Oct 17, 2017

The lambda branch is waay old (based on 2.1, that's why there are all these unrelated changes @groud mentioned).
Please rebase this on the master branch.

@groud
Copy link
Member

groud commented Oct 17, 2017

The lambda branch is waay old (based on 2.1, that's why there are all these unrelated changes @groud mentioned).

Oh ok, I didn't check the branch this PR was supposed to be merged.

@dbsGen
Copy link
Contributor Author

dbsGen commented Oct 17, 2017

@Hinsbart Already rebased.

@groud
Copy link
Member

groud commented Oct 17, 2017

@Hinsbart Already rebased.

You need to rebase it on master (not the lambda branch), and probably open a new PR asking for merging into master. Edit: this second part is not needed anymore, Hinsbart changed the destination branch :)

@27thLiz 27thLiz changed the base branch from lambda to master October 17, 2017 12:26
@27thLiz
Copy link
Contributor

27thLiz commented Oct 17, 2017

Edit: this second part is not needed anymore, Hinsbart changed the destination branch :)

Yes I changed the base branch to master, but it seems like it wasn't rebased to master correctly.
A look at OP's branch confirms this, it's 3 commits ahead of godotengine/lambda. (should be godotengine/master).

@groud
Copy link
Member

groud commented Oct 17, 2017

Yes I changed the base branch to master, but it seems like it wasn't rebased to master correctly.

Yes, the first part of my message "You need to rebase it on master" is still valid. ^^

@akien-mga
Copy link
Member

akien-mga commented Oct 17, 2017

To properly update your lambda branch, you need to do:

git checkout lambda
git reset --hard upstream/lambda   # assuming "upstream" is your godotengine/godot remote
git checkout -b lambda2    # better work in a new branch as we're rewriting history
git pull --rebase upstream/master    # then have fun fixing the many merge conflicts that will happen
git push origin lambda2

@LeonardMeagher2
Copy link
Contributor

will this allow:

func something():
    pass

func hello(f):
    f()

func _ready():
    hello(something)

?

or will I have to do

var something = func():
    pass

@dbsGen
Copy link
Contributor Author

dbsGen commented Oct 18, 2017

@LeonardMeagher2 Yes, It is allowed.

@dbsGen
Copy link
Contributor Author

dbsGen commented Oct 18, 2017

@akien-mga I am confused. Is this branch ok?(https://github.com/dbsGen/godot/tree/lambda2) And should I open a new PR?

@27thLiz
Copy link
Contributor

27thLiz commented Oct 18, 2017

@dbsGen No, unfortunately it's still not ok. It seems you rebased master on top of the lambda branch, not the other way round.

@dbsGen
Copy link
Contributor Author

dbsGen commented Oct 18, 2017

@Hinsbart How about this branch https://github.com/dbsGen/godot/tree/lambda3 ?

@dbsGen
Copy link
Contributor Author

dbsGen commented Oct 19, 2017

@Hinsbart I tried many, is this ok? (https://github.com/dbsGen/godot/tree/l2 )

@akien-mga akien-mga modified the milestones: 3.0, 3.1 Oct 20, 2017
@akien-mga
Copy link
Member

For the record, there is no plan to add lambdas to GDScript for 3.0 as this stage of development. On the other hand, there are many improvements planned for GDScript in 3.1, and discussing and implementing lambda support might be part of them.

@akien-mga akien-mga changed the title lambda for godot 3.0 Lambda expressions in GDScript Nov 10, 2017
@groud groud mentioned this pull request Jan 26, 2018
@ghost
Copy link

ghost commented Feb 6, 2018

> No description provided.

what's the use case for this? why would a game developer need to utilize lambda expressions, to help make their game?

@vnen
Copy link
Member

vnen commented Feb 6, 2018

@girng I think you lost a bit of history because this is a Godot 3.0 remake of an old PR (see #2704). And there's a lot of places where lambdas could be useful and make the code cleaner.

@ghost
Copy link

ghost commented Feb 6, 2018

@vnen thx, i personally havn't used them but i can see the need now, with that code sample in 2704. does make some code look nicer

@aaronfranke
Copy link
Member

@dbsGen You will need to learn how to use Git in order to submit code to Godot. Here's how I'd do it:

  1. Download and install GitKraken, then sign in.

  2. Clone your repository and check out your branch.

  3. Add Godot as an upstream remote, use the link https://github.com/godotengine/godot and the name upstream, then fetch it.

  4. Drag and drop your branch onto Godot's master branch to rebase it. You will have to resolve any conflicts.

  5. Force push back to GitHub.

@akien-mga
Copy link
Member

First of all, a huge thanks @dbsGen for your work on this feature, and the several iterations it went through as Godot moved on and the feature proposal didn't get merged.

Lambda expressions are a popular feature request for GDScript and definitely something we want to add, but we want do things right™ and our implementation ideas have matured a lot over the years, for this and related features.

The current plan is outlined in godotengine/godot-roadmap#23, and is lead mainly by @vnen who implemented optional typing recently and will go on to other GDScript improvements. Lambdas are currently planned for Godot 3.2.

As such I'll close this PR, since it won't be merged as is and the lambdas syntax will likely be reimplemented on top of the new features @vnen is working on. He will likely use your work as a reference, and you'll be very welcome to contribute to it when the time comes (or potentially do the implementation after discussing it with @vnen).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants