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

Clarify that some ParticleProcessMaterial properties have effect over the lifetime #83606

Merged
merged 1 commit into from
Feb 18, 2024

Conversation

k0T0z
Copy link
Contributor

@k0T0z k0T0z commented Oct 19, 2023

Fixes #83603

@k0T0z k0T0z requested a review from a team as a code owner October 19, 2023 09:38
@akien-mga
Copy link
Member

Why remove the additional documentation about how it works with a CurveXYZTexture?

@k0T0z
Copy link
Contributor Author

k0T0z commented Oct 19, 2023

@akien-mga Hmmm, I forgot I am working on the scale, right, I will fix it

@k0T0z k0T0z closed this Oct 19, 2023
@k0T0z k0T0z force-pushed the enhance-process-material-doc branch from b52f4ec to 8ac05d3 Compare October 19, 2023 09:50
@k0T0z k0T0z reopened this Oct 19, 2023
@ArtyIF
Copy link

ArtyIF commented Oct 19, 2023

I think it needs to apply to other properties like damping as well, but it needs to be double-checked first

@YuriSizov YuriSizov changed the title Enhance ParticleProcessMaterial.xml documentation Clarify what ParticleProcessMaterial.scale_curve does Nov 13, 2023
@YuriSizov YuriSizov modified the milestones: 4.2, 4.3 Nov 13, 2023
@YuriSizov YuriSizov changed the title Clarify what ParticleProcessMaterial.scale_curve does Clarify that some ParticleProcessMaterial properties have effect over the lifetime Nov 13, 2023
@Mickeon
Copy link
Contributor

Mickeon commented Jan 28, 2024

This PR should probably be updated to include more properties other than scale_curve. If it can't, I mean... It is good by itself but it could be more meaningful.

@k0T0z
Copy link
Contributor Author

k0T0z commented Jan 28, 2024

Hmmmm, sure I mean why not, is there any other properties needed, I see we need the damping as well, is there anything else I am not aware of?

@Mickeon
Copy link
Contributor

Mickeon commented Jan 28, 2024

I looked at the whole list and it seems like the following properties seem to forget to mention the "over its lifetime" part.

  • anim_offset_curve
  • anim_speed_curve
  • damping_curve
  • hue_variation_curve
  • linear_accel_curve
  • orbit_velocity_curve
  • radial_accel_curve
  • tangential_accel_curve

color_initial_ramp may seem to be part of them at first, but it is initial, so I am not sure if it counts here.

Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

The PR itself is completely fine. As I said before, I just wish it tackled more properties (which can be done later).

I had no "qualification" to approve it previously, but I can now.

@k0T0z
Copy link
Contributor Author

k0T0z commented Feb 18, 2024

I can add them some time anyway. Btw congrats for being able to approve now 🎉🎉🎉

@akien-mga akien-mga merged commit 0734ee6 into godotengine:master Feb 18, 2024
@akien-mga
Copy link
Member

Thanks!

@k0T0z k0T0z deleted the enhance-process-material-doc branch February 19, 2024 12:24
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.

Change the tooltip for scale_curve in ParticleProcessMaterial to better indicate that it scales over lifetime
5 participants