GDevelop-extensions icon indicating copy to clipboard operation
GDevelop-extensions copied to clipboard

Draw shock wave effect

Open github-actions[bot] opened this issue 1 year ago • 15 comments

Description

Will allow you to draw shock wave effects.

How to use the extension

This behavior must be assigned to a shape painter object. The user will have state parameters (Start and End) like the particle emitter object, which are :

  • Type (Fill or line)
  • The color transition (Start / End).
  • The opacity transition (Start / End).
  • The transition of angle (Start / End).
  • The transition of the radius of the explosion (Start / End).
  • The transition of the contour size (Start / End).
  • The duration of the propagation.

The user can modify the values according to his needs.

You can modify the value of the object just after the creation of the object in the event sheet (all modifications are applied on the shape painter object created above the modification).

Checklist

  • [X] I've followed all of the best practices.
  • [X] I confirm that this extension can be integrated to this GitHub repository, distributed and MIT licensed.
  • [X] I am aware that the extension may be updated by anyone, and do not need my explicit consent to do so.

What tier of review do you aim for your extension?

Reviewed

Example file

DarwShockWaveEffect-Exemple.zip

Extension file

ShockWaveEffect.zip

github-actions[bot] avatar Sep 06 '23 09:09 github-actions[bot]

Very cool extension, nice to see it updated!

Do you think it could be useful to choose an easing per value?

Some suggestions:

  • I think it's more user-friendly to have the start and end properties next to each other (in the same group).
  • Width/Height and Color/Opacity properties will be in 2 columns if they are in the same group.
  • Cheating the property ordering is not a good idea because it will make your life difficult and the issue will be solved one day.
  • If possible it's better to use hidden properties rather than variables (but it's not necessary to change it).
  • normalize can be replaced by a division.
  • The event with the "trigger once" can probably be removed without any effect.
  • There is no need to check the time as the object is destroyed automatically.
  • Time change should happen after the drawing or the initial state will never be shown.
  • doStepPostEvent should be used instead of doStepPreEvent to avoid a 1-frame delay (more generally for anything rendering related).
  • The draw ellipse action is the same in both cases.
  • Are the floor necessary?
  • Actions, conditions and expressions can be generated from the properties. image

I have the impression that a particle emitter with an image of a star would be easier to use than the star behavior. What is your view on the matter?

D8H avatar Sep 07 '23 22:09 D8H

OK I've started the modification I've almost finished the ellipse

Alios5 avatar Sep 08 '23 01:09 Alios5

@D8H the floor is for pixel perfect animation

Alios5 avatar Sep 08 '23 17:09 Alios5

@Alios5 thanks for improving your already awesome extension. What is the status of this update? I'd love to get it into GDevelop!

tristanbob avatar Sep 24 '23 18:09 tristanbob

Please attach a new version of the example that uses the last extension update.

Edit: sorry, the bot did It. I'll check it soon.

D8H avatar Nov 22 '23 19:11 D8H

Some of my previous suggestions are unanswered:

  • I think it's more user-friendly to have the start and end properties next to each other (in the same group).
  • Cheating the property ordering is not a good idea because it will make your life difficult and the issue will be solved one day.
  • normalize can be replaced by a division.
  • There is no need to check the time as the object is destroyed automatically.
  • Time change should happen after the drawing or the initial state will never be shown.
  • doStepPostEvent should be used instead of doStepPreEvent to avoid a 1-frame delay (more generally for anything rendering related).
  • The draw ellipse action is the same in both cases. It can be moved outside of the conditions to avoid duplication.

Do you have some reservations or questions about them?

D8H avatar Nov 22 '23 22:11 D8H

@D8H yes regarding doStepPostEvent, it doesn't allow to recover properties like the width of the shape painter object for my gameplay , that's why I thought there was a bug , I'm going to add a checkbox for the user who wants to use it as pre or post event image

https://github.com/GDevelopApp/GDevelop-extensions/assets/89401184/689c6be9-1637-4b5d-b37f-34995fd1a512

Alios5 avatar Nov 23 '23 19:11 Alios5

I think it's more user-friendly to have the start and end properties next to each other (in the same group).

When I do this, the end fields are above the start fields.

Alios5 avatar Nov 23 '23 20:11 Alios5

There is no need to check the time as the object is destroyed automatically. Cheating the property ordering is not a good idea because it will make your life difficult and the issue will be solved one day. normalize can be replaced by a division.

Done

Alios5 avatar Nov 23 '23 20:11 Alios5

yes regarding doStepPostEvent, it doesn't allow to recover properties like the width of the shape painter object for my gameplay , that's why I thought there was a bug , I'm going to add a checkbox for the user who wants to use it as pre or post event

Doing the drawing in postEvent is important to let users change the state in events and have it drawn without 1-frame delay. Could it make sense to use a progress or a radius instead of the width? Otherwise, you could disable automatic clearing and do the clearing right before drawing a new frame. This way the object will always have a size.

I think it's more user-friendly to have the start and end properties next to each other (in the same group).

When I do this, the end fields are above the start fields.

There is the same issue in the 3D particle emitter, but GDevelop will keep the property order at some point.

D8H avatar Nov 24 '23 23:11 D8H

Okay, I'll test it.

Alios5 avatar Nov 25 '23 07:11 Alios5

Thank you for the changes.

Previous suggestions that weren't addressed:

  • Any enabled event should be uncollapsed.
  • I think it's more user-friendly to have the start and end properties next to each other (in the same group).
  • Cheating the property ordering is not a good idea because it will make your life difficult and the issue will be solved one day.
  • Time change should happen after the drawing or the initial state will never be shown.
  • doStepPostEvent should be used instead of doStepPreEvent to avoid a 1-frame delay (more generally for anything rendering related).
  • In the group "Draw", some actions are the same for "Line" and "Fill" they can be moved outside of the condition to avoid duplication.
  • Actions, conditions and expressions can be generated from the properties.
    • You may need to remove existing actions to be able to select it. image

New suggestions:

  • Layer time scale can be handled with this expression: TimeDelta() * LayerTimeScale(Object.Layer()).
  • "Enable fill" may be easier to understand than "Change type".
  • onCreated functions can be used instead of the trigger once.
  • Object.Behavior::PropertyTweenName() can now be written like this TweenName.

D8H avatar Apr 09 '24 22:04 D8H

Here's what it looks like when I put them all together image

Alios5 avatar Apr 10 '24 08:04 Alios5

What I mean is that it's better to have titles for color, shape... rather than start and end.

A bit like the particle emitter:

image

D8H avatar Apr 21 '24 15:04 D8H

Thanks, almost there!

  • Does Pound mean ponderation? It's not clear. It's the time from the start of the animation. Why not call it Time?
  • ActivateFillshould be renamed IsFilled with something like "Fill the shape" for the label.
    • You may have to adapt the sentences and labels of the action and condition as it's hard for the generator to make correct sentences for boolean properties.
  • You can duplicate the RgbMean from "Color conversion" extension into yours to avoid to deal with color channels directly.
  • TweenName should be renamed Easing ("Name of tween" may be confusing as it sounds like "Tween identifier") image
  • "Duration in second" can just be "duration" as there is already the unit on the property field.
  • For every label, there should be an upper-case letter only on the 1st word.
  • About the property groups
    • Rename "Animation scale" to "Timing"?
    • Rename "Angle" to "Rotation"?
    • Move the checkbox into "Outline"?
  • The behaviors should be renamed "EllipseShockWave" and "StarShockWave" because behavior should be nouns not verbs and this update is a breaking change to it's the right time to do it.

D8H avatar Apr 28 '24 12:04 D8H

I've made an few changes:

  • in names, labels, descriptions and sentences
    • mostly to fix some necessary upper-case
  • in the default property values
    • because users should have something that looks good by default

If you're ok with the changes, please update the PR with this version and it should be ready for a merge into the reviewed list.

DarwShockWaveEffect.zip

D8H avatar Apr 29 '24 23:04 D8H

I did a small test of the version with @D8H changes and everything seemed to work. I'm most happy that users can add the behavior and it works without changing anything. (The old version required them to select "fill" or "line".)

One suggestion: Instead of calling the star "branches", call them "points". I often hear people say "this is a 5-pointed star", or "this star has 5 points". I do not hear people refer to them as branches.

tristanbob avatar May 01 '24 00:05 tristanbob

One suggestion: Instead of calling the star "branches", call them "points". I often hear people say "this is a 5-pointed star", or "this star has 5 points". I do not hear people refer to them as branches.

Indeed, GDevelop draw action and Pixi call them points: https://pixijs.download/v4.8.0/docs/PIXI.Graphics.html#drawStar

I've made the changes: DarwShockWaveEffect.zip

I also remove the floor in the formulas to make the animation slightly smoother.

D8H avatar May 01 '24 09:05 D8H

Add a checkbox for pixel perfect, where the floor will be used for pixel perfect

Alios5 avatar May 01 '24 12:05 Alios5

Add a checkbox for pixel perfect, where the floor will be used for pixel perfect

What do you mean by "pixel perfect"? Circles points are not integers even if the radius is. Have you noticed any artifact that can be solved by rounding values?

If you disable the anti-aliasing, you will see sharp pixels even if the circle radius is not an integer:

image

D8H avatar May 01 '24 12:05 D8H

ok i tested it it's good as you make it

Alios5 avatar May 01 '24 18:05 Alios5

You forgot to hide the easing progress! for StarShockwave image

Alios5 avatar May 01 '24 18:05 Alios5

You forgot to hide the easing progress! for StarShockwave

Indeed, sorry about that.

D8H avatar May 02 '24 08:05 D8H

Errors were detected in this submission:

ℹ️ Ignoring validation for extension WithThreeJS.


❌ 1 Error found in extensions - please fix it before generating the registry.

github-actions[bot] avatar May 04 '24 11:05 github-actions[bot]

@Alios5 Thank you for the hard work and your patience. It should be in the reviewed extension list in a few hours.

D8H avatar May 04 '24 11:05 D8H