Skip to content

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Sep 27, 2022

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 :

  • 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

  • I've followed all of the best practices.
  • I confirm that this extension can be integrated to this GitHub repository, distributed and MIT licensed.
  • 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

DrawShockWaveEffectExemple.zip

Extension file

DrawShockWaveEffect.json.zip

@github-actions github-actions bot requested a review from a team as a code owner September 27, 2022 11:12
@github-actions github-actions bot added the ✨ New extension A new extension label Sep 27, 2022
@github-actions github-actions bot mentioned this pull request Sep 27, 2022
3 tasks
@D8H
Copy link
Contributor

D8H commented Sep 27, 2022

This is a following of:

@D8H D8H added the 🔍 Reviewed extension An extension that is to be reviewed in great detail before merging. label Sep 27, 2022
@D8H
Copy link
Contributor

D8H commented Sep 27, 2022

Thanks for submitting this update.

Some suggestions:

  • The __DrawEllipseShockWave__.CanDraw variable seems to be useless as it's always true.
  • What happens to the object when the animation is done?
    • Should there be a condition to check the animation ended?
    • Should there be a reset action to let user reuse the object for several animations?
    • Should object be deleted automatically as an option?
  • The Tween::Ease expression could be used to avoid to require the Tween behavior (but I guess it can stay the way it is).

If Someone does an object particle emitter, it will work very well with the extension.

@Alios5
Copy link
Contributor

Alios5 commented Sep 27, 2022

Thanks for submitting this update.

Some suggestions:

  • The __DrawEllipseShockWave__.CanDraw variable seems to be useless as it's always true.

  • What happens to the object when the animation is done?

    • Should there be a condition to check the animation ended?
    • Should there be a reset action to let user reuse the object for several animations?
    • Should object be deleted automatically as an option?
  • The Tween::Ease expression could be used to avoid to require the Tween behavior (but I guess it can stay the way it is).

If Someone does an object particle emitter, it will work very well with the extension.

In fact the variable stop the animation ( DrawEllipseShockWave.CanDraw = false ) of the object to apply the modification which we make in the sheet of events just after its creation before restarting the animation ( DrawEllipseShockWave.CanDraw = true ) it is what happens in the project example in the second group of sheet of event (it stop applies the modification of value and restarts the animation for each event)

@D8H
Copy link
Contributor

D8H commented Sep 27, 2022

The value is set to false then to true right after. It's the same as just setting it to true directly.
The "trigger once" should be removed too as it will prevent to change the value as a 2nd time.

image

@Alios5
Copy link
Contributor

Alios5 commented Sep 27, 2022

Yes indeed it works well without ,actually XD I just tested.

@github-actions github-actions bot force-pushed the extension/Alios5/602 branch from 2597609 to 150aa04 Compare September 27, 2022 12:47
@Alios5
Copy link
Contributor

Alios5 commented Sep 27, 2022

The value is set to false then to true right after. It's the same as just setting it to true directly. The "trigger once" should be removed too as it will prevent to change the value as a 2nd time.

image

The value is first set to false and then directly to true. This is the same as setting it directly to true. The "trigger once" should also be removed, as it prevents the value from being changed a second time. > > image

I updated it

@github-actions github-actions bot force-pushed the extension/Alios5/602 branch from 150aa04 to 19f2a33 Compare October 18, 2022 10:10
@github-actions github-actions bot force-pushed the extension/Alios5/602 branch from 19f2a33 to 95965d6 Compare November 29, 2022 20:20
@github-actions github-actions bot force-pushed the extension/Alios5/602 branch from 95965d6 to 64de29c Compare November 29, 2022 23:21
@tristanbob
Copy link
Contributor

@Alios5 I apologize for taking so long to review this wonderful extension. Our community is going to love using it!
In fact, while testing this extension I added it to the game I'm working on and it is absolutely gorgeous.

In the future, feel free to make updates to this extension when you see opportunities for improvements and bug fixes.

@tristanbob tristanbob merged commit 42ff24c into main Dec 24, 2022
@tristanbob tristanbob deleted the extension/Alios5/602 branch December 24, 2022 02:40
@D8H
Copy link
Contributor

D8H commented Dec 24, 2022

@tristanbob I think that @Alios5 was asking for a full review but it was merged in the community list.
Did you do a full review?
Should we open another PR to move it to the reviewed list?

@tristanbob
Copy link
Contributor

@tristanbob I think that @Alios5 was asking for a full review but it was merged in the community list. Did you do a full review? Should we open another PR to move it to the reviewed list?

I did not perform a full review. I saw the extension was placed in the community folder, so that is how I treated it.

We could do a full review, but for now, I'm happy it's available for users in the community section.

@D8H
Copy link
Contributor

D8H commented Dec 25, 2022

I did not perform a full review. I saw the extension was placed in the community folder, so that is how I treated it.

We could do a full review, but for now, I'm happy it's available for users in the community section.

Good call.

@Alios5 feel free to open a new PR to ask for a full review. You'll have to increment the version to have one change.

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

Labels

✨ New extension A new extension 🔍 Reviewed extension An extension that is to be reviewed in great detail before merging.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants