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

Issues in GDScript parser with assert() function and its arguments #39324

Closed
Arech opened this issue Jun 5, 2020 · 6 comments
Closed

Issues in GDScript parser with assert() function and its arguments #39324

Arech opened this issue Jun 5, 2020 · 6 comments

Comments

@Arech
Copy link

Arech commented Jun 5, 2020

Godot version: latest stable 3.2.1

OS/device including version: W7 x64, but it shouldn't matter probably

Issue description: put the following code in cbasen.gd

extends RigidBody2D

class_name CBaseN

func myfunc(arg:float)->void:
	assert(arg>0) #just a valid call
	assert(ar>0) # invalid call, but NO F#@%$ ISSUES WITH THIS LINE!
	#print("d=%d" % [int(ar>0)])  #THIS LINE WON'T WORK HOWEVER,
	#as well as any other possible function call with incorrect argument I tried

Try the following code for derived class

extends CBaseN

func _ready():
	myfunc(1) #emits error here however, despite no visible issues with cbasen.gd

I walked in gdscript parser under debugger while parsing code of the derived class. The parser indeed does know nothing about myfunc(), though it successfully parses cbasen.gd.


This issue cost me about half a work day to pinpoint.

It is soooo discouraging... Even more so after looking at the source code that completely lacks any sanity checks and state assertions...

@Calinou
Copy link
Member

Calinou commented Jun 5, 2020

It is soooo discouraging... Even more so after looking at the source code that completely lacks any sanity checks and state assertions...

GDScript is being rewritten from the ground up: #39093

@Calinou Calinou added this to the 4.0 milestone Jun 5, 2020
@Arech
Copy link
Author

Arech commented Jun 6, 2020

Yes, I know, it is being rewritten, I’ve read the news post on the site. And now I've skimmed through the code of new GDScript... And now I'm feeling much more worried than before, because… well, the old GDScript is at least somehow field tested by community (though, I’m puzzled that nobody ever encountered the bug I’ve described in this issue). The new code however is…totally new (that is not bad by itself). But I was unable to find any tests of it (we’re aren’t calling /main/tests/test_gdscript.cpp “the tests”, aren’t we?). And the code almost totally lacks debugging assertions, checks of internal state/arguments/parameters during code flow… Unfortunately, that’s coherent with the all other Godot’s codebase, but it’s a recipe for disaster(((

@Calinou
Copy link
Member

Calinou commented Jun 6, 2020

But I was unable to find any tests of it (we’re aren’t calling /main/tests/test_gdscript.cpp “the tests”, aren’t we?).

See https://github.com/godotengine/gdscript-tests.

@akien-mga
Copy link
Member

akien-mga commented Jun 6, 2020

Can't rest right now but I think this assert evaluation bug might be fixed in 3.2.2 beta 4.

@Arech
Copy link
Author

Arech commented Jun 6, 2020

See https://github.com/godotengine/gdscript-tests.

That is very good to hear, thank you!

@ThakeeNathees
Copy link
Contributor

this bug already fixed and cherrypicked : bd081df

in 3.2.2 beta 3 it's working as expected

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

No branches or pull requests

4 participants