GDevelopApp/GDevelop-examples

Linked Chain demo

Closed this issue · 7 comments

HardTranceFan commented

This example is a demonstration using the Physics2 revolute joint to link together a number of objects to create a chain like effect.

Checklist

  • I've followed all of the best practices.
  • My game has a proper name in the game properties.
  • My game package name behind with com.example..
  • My game has all events unfolded.
  • I've added myself as the author in the game properties.
  • I've included a file called "README.md" with a description in proper English, explaining what this example is doing.
  • I confirm that this game and all of its resources can be integrated to this GitHub repository, distributed and MIT licensed.
  • I've cleaned unused resources.

Game folder

ChainPhysicsDemo-revolute.zip

Bouh commented

Hello @HardTranceFan
Thank you for this project! Here what I've seen during my review:

  • An image is broken in Animations of the chainLink1 object, same for chainLink2:
    image

  • README content can be shorter : This example is a demonstration using the Physics 2.0 and the revolute joint action to link together a number of objects to create a dynamic chain with a realistic physic.
    We don't add requirement in this readme, it should be explain in the example itselft in a comment for example.

  • To optimize a bit the physic because it could be costly in a larger example with tons of chains, you can check the Can sleep box in Physics 2.0 behavior for all object using the physics. It will stop the simulation of the physics when it need, (when the movement is over).

  • On objects text, add a period or not on both.

  • Our variables start by an underscore which is not required at all, I think you can remove this underscore, we strongly recommand the PascalCase syntax.

  • Objects must use PascalCase too.

  • Ressources are in an assets folder

  • Comment This action puts a revolute joint between chainLink2 and the next chainLink1. If the next chainLink1 doesn't exits, then _isStillProcessing remains false. is nice. Can you add in this comment why we want _isStillProcessing false, please, to remind people what is for.

  • Comment just below Once all the chain links have **bee** joined, bee ? Typo?

  • _jointId variable is a scene variable used for mouse, I think we should rename it to be more explicative like MouseJointId

  • On last condition you use Escape key is pressed I recommand to choose is released, because when you stay on escape key it restart the scene, and in this new scene it detect the escape key pressed, and it loop for ever while you press escape. Compared to is released where the event is detected once, when you release the key, and there is no more loop.

If you can check cases of my checklist please do, it give to everyone an overview on the remaning paint points.
Also very important to let us know when you update your zip file!

Cool use of the while loop with stillProcessing variable!

Overall this example is well made and documented, really a great job done here!
Before another review from someone else I think you can try to apply these first feedback, and then someone else will give a new look after.

HardTranceFan commented

Hi @Bouh,

I'm going through the check list, and I'm not sure what you mean with resources are in asset folder. Should they not be in the folder, and moved one level up instead?

Jeroen

Bouh commented

Hi @Bouh,

I'm going through the check list, and I'm not sure what you mean with resources are in asset folder. Should they not be in the folder, and moved one level up instead?

Jeroen

On contrary, everything is ok with the asset folder, there is nothing to do. (I try to reuse a genric checklist for faster review on examples)

Bouh commented

Hi @HardTranceFan Do you need more time or help?
Let us know ;) !

HardTranceFan commented

Hi @Bouh, no, I was waiting for your reply. For some reason it hadn't come up a week after I sent my response.

I'm looking to slightly modify the example, only adding a rope segment sprite for each chain link, so the images can be switched from chain to rope.

I'll post the update within an day or so.

  • ❤️1
HardTranceFan commented

Hi @Bouh,

I've updated the project as per checklist. I've also added an extra rope animation for the link, along with a text object that acts as a button to switch between linked chain and rope segments.

Cheers
Jeroen

ChainPhysicsDemo-revolute.zip

Bouh commented

Hi @Bouh,

I've updated the project as per checklist. I've also added an extra rope animation for the link, along with a text object that acts as a button to switch between linked chain and rope segments.

Cheers Jeroen

ChainPhysicsDemo-revolute.zip

Thank you, I merged your example in #294 .
I just made one edit to remove the unused scene variable.

Again thanks!