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

New physics behavior #775

Merged
merged 26 commits into from
Dec 22, 2018
Merged

New physics behavior #775

merged 26 commits into from
Dec 22, 2018

Conversation

Lizard-13
Copy link
Contributor

@Lizard-13 Lizard-13 commented Nov 29, 2018

  • Previous functionality
  • Edge shape
  • Custom shape dimensions and offset
  • Kinematic type
  • Filtering layers and masks
  • Ability to apply forces and torques at any point (not only mass center)
  • Joints ID manager
  • Joints
    • Distance joint
    • Revolute joint (fixed point or between objects)
    • Prismatic joint
    • Pulley joint
    • Gear joint
    • Mouse joint
    • Wheel joint
    • Weld joint
    • Rope joint
    • Friction joint
    • Motor joint
  • Ability to remove joints
  • Conditions to check joint reaction force and torque (useful to make breakable joints)
  • Ability to modify any joint property on the fly
  • When object size is modified, recreate the shape only, not the entire object, or remake the joints in the new body
  • Expression to get body mass center
  • Condition to check colliding objects
  • Remake the icons and add new ones

Also simplified the world scaling, no more negative values and opposite angles here and there :)

EDIT: Forget to say that almost every position and angle value in joints settings are in world coordinates, so you can add joints using sprites points for example, without worrying about the object rotation.

@Lizard-13
Copy link
Contributor Author

Breakable joints ready
breakablejoint

@4ian
Copy link
Owner

4ian commented Nov 30, 2018

Incredible 😍

* Compare joints objects
* Get joints anchor points
* Read/Modify distance, revolute and prismatic joints settings
* Read/modify pulley, gear, mouse, wheel, weld, rope, friction and motor joints
@zatsme
Copy link
Contributor

zatsme commented Nov 30, 2018

Loving this, but you made a copypasta error I think 😄

In physikruntimebehaviour.js

The Wheel Joint Definition....

Line 1686

gdjs.PhysikRuntimeBehavior.prototype.addWheelJoint = function(x1, y1, other, x2, y2, axisAngle, frequency, dampingRatio, enableMotor, motorSpeed, maxMotorForce, collideConnected, variable){

and line 1711

jointDef.set_maxMotorForce(maxMotorForce);

Both should be maxMotorTorque

Apart from that I got the Wheels working fine 👍

@zatsme
Copy link
Contributor

zatsme commented Dec 1, 2018

bikes

Cool 😃

@4ian
Copy link
Owner

4ian commented Dec 1, 2018

That's really cool! We should definitely have both the demo that you made as example! :)

@Lizard-13
Copy link
Contributor Author

Fixed!, thanks, surely I've a lot of these, there are tons of settings :D
I'll finish the other things and start testing every setting

@zatsme
Copy link
Contributor

zatsme commented Dec 1, 2018

One other thing, It took a few seconds for me to find the Mass setting.... It is labelled Density, which is fine if you know this stuff but for beginners I think either Mass (Density) or vice versa, or just Mass 🤔

@4ian
Copy link
Owner

4ian commented Dec 1, 2018

I guess mass is computed by multiplying the density and the surface of the object. Might be useful to put something like Density (will be used to compute the mass of the object)

@zatsme
Copy link
Contributor

zatsme commented Dec 1, 2018

Ok, I'm used to setting mass explicitly, but I like the way box2d does it 😁. This is going to be super powerful 😊

@Lizard-13
Copy link
Contributor Author

The old setting was used as the density too I think, this way is more clear, because set mass = 2 is not the same that set density = 2. I've tried to port Box2D as is, but I've made some modifications for easiness (global position for joint anchors and forces applying point).

So... If you people think it's better to put the mass, I can use that mass and calculate the shape area to get the density in the background :)

@zatsme
Copy link
Contributor

zatsme commented Dec 1, 2018

No, it's good, we'll make it clear in the doc 😁

And improve response on object size modifications
@Lizard-13
Copy link
Contributor Author

Now, on object size change, instead destroying and recreating the body the fixture is recreated, this saves the joints and I guess it's a bit faster.
You can use custom shape dimensions and offset, to achieve things like this:
shapessize
It's confusing until you can see the real shapes.
There's also an action to set the shape scale, it works when using custom shape dimensions or offsets, so you can scale it with the object.

@Lizard-13
Copy link
Contributor Author

Ok, it should be almost free of bugs, tested every setting, just missed some default values on joint creation
itsalive

And here are the test files
The testbed with all the joints:
PhysikJointsTest.zip
All the joints with editable settings:
PhysikJointsSettingsTest.zip
And the boring one where you can change the custom shapes scale:
Uploading PhysikObjectSizeTest.zip…

@zatsme
Copy link
Contributor

zatsme commented Dec 2, 2018

Looking very good 😊.

Just 1 question right now, how are you supposed to use the edge shape, and when might we get polygons? 😋

@Lizard-13
Copy link
Contributor Author

The edge shape is almost the same than a box, but it's mean to be static and is a bit faster, so it's better suited for level grounds.

Polygons could be already added, I did some code in a custom test file, so I know it works and can already put the working code in GD... But to be useful we need a polygon editor.
@4ian Can we create custom editors for behaviors settings?, right now the editor is generated automatically, so I can't make a better layers/masks selector or make it to preview the shape or a polygon editor. Surely it would take me a lot of time to finish something like that, but I world do it :)

@zatsme
Copy link
Contributor

zatsme commented Dec 3, 2018

Can't you get the polygon data from the sprite polygon data. We can already edit that, although it's a little clumsy interface as added dots all go to 0,0 right now.

I should probably have said points or vertices 🤣

@Lizard-13
Copy link
Contributor Author

Although it's the most common use case, the object to which you attach the behavior is not always a sprite.
It would be super dirty, but I could enable the Polygon shape, and in the behavior code check if the owner object is a sprite, if it's, get the polygon from the collision shape from the sprite first animation and frame. Don't like it a lot.

@4ian
Copy link
Owner

4ian commented Dec 3, 2018

@4ian Can we create custom editors for behaviors settings?, right now the editor is generated automatically, so I can't make a better layers/masks selector or make it to preview the shape or a polygon editor. Surely it would take me a lot of time to finish something like that, but I world do it :)

So for now the editor for behaviors (BehaviorsEditor/index.js) is reading the properties of the behavior and then using PropertiesEditor to generate a form from these properties. Precisely a schema is generated from the properties (using PropertiesMapToSchema.js) and sent to `PropertiesEditor.

  • PropertiesEditor support displaying a button (search for _renderButton in PropertiesEditor/index.js) if the field of the schema has "onClick".
  • We could add a new property or a special kind of property in PropertiesMapToSchema to handle buttons opening dialogs.
  • The difficulty is to actually open the dialog: how to register/find the dialog to open? I guess we would need to "register" somewhere a list of dialog that can be opened from PropertiesEditor (much like ParameterRenderingService, which given a type of parameter returns the class to be used to do the rendering). We would have a PropertyEditorService that would be, given an editor name, open a dialog with this editor - and would have a method to register a new editor.
  • The special property I mentioned before would be having the name of the editor stored, maybe in the properties "extraInfo".

Finally, the question is: where to write this new editor/dialog? For now, the answer is: directly inside newIDE. Ideally the extension would be able to bring their own editors as part of the extension source code. But this is difficult because the "compilation" of newIDE is done only for files in newIDE/app/src/*.

You can see that for example, for now, all objects editors are in newIDE/app/src/ObjectEditor/Editors (and there is a ObjectsEditorService for giving access to them).

So the answer would be: we need to create a PropertyEditorService in PropertiesEditor folder (and a subfolder "Editor" containing the custom editors) (like it's done for ObjectsEditorService.js).
(Later if we are able to compile .js file in extensions for the IDE, we will move all the files in the **/Editors/* folders to their own extensions).

@4ian
Copy link
Owner

4ian commented Dec 3, 2018

It would be super dirty, but I could enable the Polygon shape, and in the behavior code check if the owner object is a sprite, if it's, get the polygon from the collision shape from the sprite first animation and frame. Don't like it a lot.

Yes, it would be dirty. Ideally behaviors should 100% stay separated from the underlying object having them. We could provide way in the IDE to copy the collisions mask to physics polygon, but that's more a convenient option than what should be the default :/

@Lizard-13
Copy link
Contributor Author

What I can do right now is enable a new string label in the behavior, here the user can enter the points in a list (as in the old IDE, but in pixels, no meters). Then try to parse it in the code.
It's ugly, but all the functionality will be there for the time it gets a nice editor :D

@Lizard-13
Copy link
Contributor Author

Totally forgot the collision condition, now it works as the normal collision condition, i.e. both objects used in the condition are picked.
Also added the option to (don't) allow to sleep, and a condition to check if an object is sleeping, useful for these balance games.
collideandsleep

@Lizard-13
Copy link
Contributor Author

@4ian Will start doing the polygon, merge it when you want, if I don't add the polygon now will do it for the editor, I'll rewrite the wiki page too!

@Lizard-13
Copy link
Contributor Author

@zatsme The worst scenario is having two independent Box2D worlds around, objects from one world won't collide with the objects from the other, not so bad.
About the GD forces, what do you think @4ian ?

@4ian
Copy link
Owner

4ian commented Dec 20, 2018

@zatsme That's a very true concern!
I think people will be confused and will use both - so we should actively avoid this.

I think I'll work on something to warn about potential conflicting uses. Maybe gray out a behavior if another one is already used in the game (i.e: if Physics is used, gray out Physics2 and vice versa - including actions/conditions/expressions). If you still try to use them, you'll get a warning "You're trying to use XXX, but the game is already using YYY. Are you sure you want to use both? This could result in unexpected/conflicting behaviors of objects on the scene".

This could be extended to the use of forces with objects having a behavior like Physics/Physics2.

More generally, there are a few things that I'd like to warn users against (making weird hitboxes with platformer engine, using sprite of different sizes in platformer engine, launching music in a loop, and using image that are too large). All of these are not related, but could be really useful.

@Lizard-13
Copy link
Contributor Author

Lizard-13 commented Dec 20, 2018

This could be extended to the use of forces with objects having a behavior like Physics/Physics2.

Very handy!

@4ian
Copy link
Owner

4ian commented Dec 20, 2018

Pretty handy!

Yes, as much as I'd like to merge this now and have the new behavior and demo integrated, we should be a bit cautious - especially because GDevelop was criticized in the past for being confusing on multiple parts... It's also on us to make sure that things are easy to use properly, and hard to use improperly :)

And if we can avoid/reduce drastically the number of users asking the questions on the forum/discord, that's great :)
For example, in the same vein, if a game is not using any physics behavior, we should gray out Physics (the old one) and if the user clicks on it, he should get a message "You're about to use Physics, but Physics2 is recommended for new game as more complete and powerful. Are you sure you want to use it?". (We could even hide it!)
=> Instead of getting users asking for months "should I use physics or physics2", we'll get everybody to use the proper one :)

@zatsme
Copy link
Contributor

zatsme commented Dec 20, 2018

Would it be an idea to have an extension/behaviour type Id?

Then when a behaviour/extension is used for the first time the ide can Check if another of the same type is already used and warn.

If physics was Id Phy1 and physics 2 was Phy2 then you can warn that a type of Phy was already in use and it's also a later/newer version.

That way if we ever get to a point where GD5 is being extended by many devs like construct and game maker then the parser can warn against issues automatically as long as the extension type is present.

🤔

@4ian
Copy link
Owner

4ian commented Dec 20, 2018

Yes, that's what the internal name of the extensions are for :)
In this case, the extension name is Physics2 (and the name displayed to user is "Physics Engine 2.0" - even though the extension are never shown directly in GDevelop 5 for now).
The behavior name is Physics2Behavior (see here) (and the name displayed to user is "Physics Engine 2.0 (beta)").

What could be done here is a quick helper in newIDE codebase that gives the extension used by a game (it's already done in update-examples-information-from-resources-examples.js in computeUsedExtensions, we should extract it in a separate file to use it in the IDE), then take the name of another extension (like "Physics2") and return a warning (or nothing if it's fine to use).
This would be using a list of "incompatibilities" (that could grow if more extensions are added in the future - even if ideally, extensions should behave properly).

This could be refined to events to detect usage of incompatibles actions/conditons on some objects (we should have all the tool needed to list the actions/conditions used on a specific object).

@4ian
Copy link
Owner

4ian commented Dec 20, 2018

For warning users about forces on object that have the physics behavior, that is surely a bit simpler that what I describe:

In the selector of instructions (internally, the word "instruction" represents a condition or an action), if the user choose a condition or action for an object, check the behavior of this object, and send it to the class/helper for incompatibilities.

I've added a task for it here: https://trello.com/c/xq91to0I/218-warn-the-user-against-using-incompatible-actions-conditions-like-using-force-on-an-object-that-have-the-physics-behavior

Here is the roadmap with all the "productivity" warnings that we could add: https://trello.com/b/qf0lM7k8/gdevelop-roadmap?menu=filter&filter=label:Productivity%20friendly%2Favoid%20common%20errors

If you have other ideas, let me know and I can add them here :)

@Lizard-13
Copy link
Contributor Author

Lizard-13 commented Dec 20, 2018

Should the polygon origin be at the center or top-left of the object?
With "center" I mean the drawable/visual center, not the center point but the default value of the center point.

@4ian
Copy link
Owner

4ian commented Dec 20, 2018

Should the polygon origin be at the center or top-left of the object?

What are the impacts/implications of both solutions?

@Lizard-13
Copy link
Contributor Author

No idea, thinking about it... On object size change (and if shape scale is not changed), if the origin is at center it will stay centered (small offset in all the directions), but if the origin is at top-left all the shape offset will be at the bottom-right.

@zatsme
Copy link
Contributor

zatsme commented Dec 20, 2018

Can't we define it as we do with sprites?

@Lizard-13
Copy link
Contributor Author

A choice Polygon Origin: TopLeft/Center ?, could be, don't know.

@4ian
Copy link
Owner

4ian commented Dec 20, 2018

If we can, we should make this choice for the user no?

Can't we define it as we do with sprites?

I guess the way polygons are defined in Box2D is a bit different than hitboxes (there can be a single polygon for example). Also, polygon can't/should not change for every frame - this would be a nightmare in terms of performance.

I'm almost thinking of adding a big warning against making different hitboxes for every frame btw.

@Lizard-13
Copy link
Contributor Author

Lizard-13 commented Dec 20, 2018

If we can, we should make this choice for the user no?

I'm checking out GD4 (oof haha), and the polygon editor indeed let the user choose object center and origin (and for some reason top-left option is commented out in the sources)

Also, polygon can't/should not change for every frame - this would be a nightmare in terms of performance.

For sure, what I'm doing right now is storing the vertices in a memory buffer, they aren't recomputed on shape size change, but you've to pass them to the body again in a new shape, so every contact has to be recomputed.
But I'm just re-setting the vertices, and is what Box2D does in the background when you create a box shape, so the speed should be similar.

@Lizard-13
Copy link
Contributor Author

Lizard-13 commented Dec 20, 2018

Good and bad news, good ones: I get it to work, and have proofs :)
scale1
scale2
The smaller and darker shape is the expected size at 0.5 shape scale, as you can see the triangle scales correctly on center, and the green one scales on top-left.

The bad ones, @4ian , do you have any idea why the parser converts an empty array into a JSON empty object, and non-empty arrays break the parser, skipping next properties?
For example I add the default property vertices: [], around here: https://github.com/4ian/GDevelop/pull/775/files#diff-44c774e96a649c5ec44318372413a95dR277
and even edit the game.json file to add the vertices array manually, but that doesn't matter, on preview/export GD replaces them with broken objects. vertices: [] is coverted to "vertices": {}, and convert vertices: [1,2,3] into "vertices": {"": {"3": "shape"}} or something like that, mixing the next property and skipping the others.
The raw JSON content (https://github.com/4ian/GDevelop.js/blob/234ecd8e03c3aa7ab62095907bcad9e767878908/Bindings/BehaviorJsImplementation.h#L31) is correctly saved.
I get it to work and take the screenshots editing the data.js file.

@Lizard-13
Copy link
Contributor Author

@4ian Merge it if you want, the conversation is getting long and it seems it will take ma a while to get what's going on, for a feature that won't be available anyway :)

@4ian
Copy link
Owner

4ian commented Dec 20, 2018

So the serializer in C++ is not properly handling these arrays it seems. I can help to take a look at it.
We'll merge this in the next days, and I'll put this hidden under a flag in the preferences (like events functions) that I'll remove when we have help ready and a good way to show to the user that they should use this and not the old behavior anymore :)

This way you can continue working on the polygons on another PR :)

@4ian
Copy link
Owner

4ian commented Dec 20, 2018

In short it's because the Serializer was originally created to be compatible with XML and JSON. XML does not have arrays but multiple children with the same name.
The SerializerElements that are created by the gd::Serializer don't contain the information that this thing is an array (because XML don't have this). So I made a method ConsiderAsArrayOf that you can call on SerializerElement, to be able to iterate on the children (not useful for JSON, but was useful for XML). It's manually called by the methods that serialize/unserialize. As all we're doing here is parsing and writing back JSON, without calling ConsiderAsArrayOf, the notion that this thing was an array is lost.

I think we should be able to have a "isArray" boolean stored in the SerializerElement when reading the JSON. Then when writing back the SerializerElement, it would be properly outputted as an array.
(This would be an extra property in SerializerElement, the property arrayOf, set by ConsiderAsArrayOf, would still be there).

I'll take a look!

@4ian
Copy link
Owner

4ian commented Dec 21, 2018

@Lizard-13 Fix was actually quite simple. Made the fix in 238b6a2 and added tests for this.

I've updated libGD.js, remove it from public/libGD.js and relaunch npm start/yarn start (did not bumped the version as it's a bugfix).

Let me know if you still have an issue!

Merge changes from GDevelop master into the branch
@Lizard-13
Copy link
Contributor Author

I'm a bit lost with this one, I've cherry picked your commits into my ligGD.js, but still failing to read the array. the SerializerElement passed here:
https://github.com/4ian/GDevelop.js/blob/be3e0b8d63fecfb4a4aa90e01e8a9b4f2dbd966c/Bindings/BehaviorJsImplementation.cpp#L83
Is not considering the "vertices" child as an array, so I guess the problem comes from the project unserialization. Will continue the debug later and start the pull request for the polygon and editor after you merge it, to ensure there are no further modifications and conflicts.
*Maybe I'm doing something wrong, but I guess opening the game file should read the array correctly no matter the behavior code.

@4ian
Copy link
Owner

4ian commented Dec 22, 2018

If you have merged master into your branch, there is no need to cherry pick anything right?
You have 238b6a2 in your commits right?

@Lizard-13
Copy link
Contributor Author

Hey, you're right, but I did it before merging master, not sure why 😅
Yeah, I've the modifications and recompiled libGD.js myself (so I can debug it around), the tests you've added seems pretty solid, so I must be doing a mistake somewhere... but as said before, lets continue it in a new pull request, it's going off topic for the current one :)

@4ian
Copy link
Owner

4ian commented Dec 22, 2018

Let's merge this now! I'll hide the extension for the next version and add a preference to display it once we have help and a way to make sure users can't misuse the two behaviors ;)

@4ian 4ian merged commit 2d314ff into 4ian:master Dec 22, 2018
@4ian
Copy link
Owner

4ian commented Dec 22, 2018

Huge huge thanks for your totally awesome work on this. 💯🎉 This is a huuuge feature! I have no doubt that lots of game will use it soon - and the demos are super impressive. 😎

It's a great example of GDevelop being able to handle complex games with advanced physics! 🚀
Thanks again and keep up the good job!

@Lizard-13 Lizard-13 deleted the new-physics-behavior branch December 22, 2018 23:24
@jjhesk
Copy link

jjhesk commented Mar 15, 2022

Does the polygon physics work now on GD5? Can show me some example?
@4ian

@4ian
Copy link
Owner

4ian commented Mar 15, 2022

Please ask this on the forum or on the Discord :)

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

Successfully merging this pull request may close these issues.

5 participants