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

Add Physics Tests project #473

Merged
merged 1 commit into from
Jun 19, 2020
Merged

Conversation

pouleyKetchoupp
Copy link
Contributor

New project to provide a test framework for the 3D physics engines, based on part 1 of this proposal:
godotengine/godot-proposals#667

I'm planning to add more tests in the coming weeks, but I'm providing this first version in order to start getting feedback.

@pouleyKetchoupp pouleyKetchoupp force-pushed the physics-tests branch 2 times, most recently from 984f9a5 to c7eb049 Compare May 31, 2020 09:52
Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm glad to see that you're working on physics, it really needs a lot of fixing. This is a first-pass review, I haven't looked into the project very much in depth yet.

Comment on lines 25 to 30
timer.paused = false

func is_timer_canceled() -> bool:
return timer.paused

func on_timer_done():
Copy link
Member

@aaronfranke aaronfranke May 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These scripts do not follow the GDScript style guide, there should be two lines between functions. Please read the whole style guide, there are some other issues that I can see: https://docs.godotengine.org/en/latest/getting_started/scripting/gdscript/gdscript_styleguide.html

"_edit_use_anchors_": false
}

[node name="Tests_Menu" type="MenuButton" parent="."]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Node names should be PascalCase with no underscore to match the convention of Godot's built-in names.

transform = Transform( 50, 0, 0, 0, 1, 0, 0, 0, 50, 0, 0, 0 )
shape = SubResource( 4 )

[node name="StaticBody_Head" type="StaticBody" parent="."]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The head and plane should be in their own scene, probably together.

class_name Test

var timer : Timer
var timer_started : bool = false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akien-mga says he doesn't want static typing used in the demos, so for now the policy I've been using is "no static typing unless it significantly improves readability".


Language: GDScript

Renderer: GLES 3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since rendering doesn't affect physics, the project should probably be converted to GLES 2.

@pouleyKetchoupp
Copy link
Contributor Author

Thanks @aaronfranke for your review! I'll look into the GDScript style guide and fix the different things you pointed out.

@pouleyKetchoupp
Copy link
Contributor Author

pouleyKetchoupp commented Jun 19, 2020

I've just pushed an update from the review:

  • Switched to GLES2
  • Godot head and ground plane in a separate scene
  • Style fixes in GDScript, scene nodes and project files

Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! I can think of a few things to add, but it's great as-is.

@pouleyKetchoupp
Copy link
Contributor Author

Thanks! I'm interested to hear more details about the things you would like to add. I've got my own todo list for the next tests I want to implement, but I would gladly add more things if they can be useful.

@aaronfranke aaronfranke merged commit d3317cc into godotengine:master Jun 19, 2020
@aaronfranke
Copy link
Member

aaronfranke commented Jun 19, 2020

Mostly more complex scenarios, some are test cases for known existing physics bugs.

  • Compound objects colliding with compound objects.

  • Tests for 2D physics would be nice too.

  • There's a bug where characters on a flat surface will slightly bounce up and down, it's visible in the Voxel demo and the TPS demo. The problem is most visible when you are colliding with more than one StaticBody (such as on a chunk border in the Voxel demo).

  • There's a bug where adding colliders is extremely slow. In the Voxel demo, if you disable VSync, comment out line 186 in chunk.gd, and set gravity to 0 on line 8 in player.gd so that you don't fall forever, you can observe that the game runs vastly faster by not generating colliders.

For the last point, here is a minimal script which showcases the problem. If placed on a StaticBody by itself with nothing else in the scene (no cameras or anything), for me it starts at over 300 FPS, gradually slows down to about 30 FPS, then once it's done adding colliders I get over 9000 FPS. If you change this to _ready and for i in range(amount): then the game is frozen for over 5 seconds before the first frame is finished.

extends StaticBody

var amount = 1000

func _process(delta):
	amount -= 1
	if amount > 0:
		var c = CollisionShape.new()
		c.shape = BoxShape.new()
		add_child(c)

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.

3 participants