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

Spread should be in degree (see documentation, and flag_disable_z case) #14330

Merged
merged 1 commit into from
Dec 16, 2017
Merged

Spread should be in degree (see documentation, and flag_disable_z case) #14330

merged 1 commit into from
Dec 16, 2017

Conversation

matrem
Copy link
Contributor

@matrem matrem commented Dec 6, 2017

I think particle spread should be taken as degree.
From these points it is :

So I think this line should be fixed :

code += " float angle1 = rand_from_seed(alt_seed)*spread*3.1416;\n";

@matrem matrem changed the title Spread should be degree (see documentation, and flag_disable_z case) Spread should be in degree (see documentation, and flag_disable_z case) Dec 6, 2017
@akien-mga akien-mga added this to the 3.0 milestone Dec 6, 2017
@reduz
Copy link
Member

reduz commented Dec 9, 2017

can you check this again? I did a few changes to particles recently. Also i may be converting many values to radians wrong from what i see in the code, so checking all usages of 3.14 may be good

@matrem
Copy link
Contributor Author

matrem commented Dec 9, 2017

Ok I can look into this, perhaps I'll ask you some things about the code to understand it :p
Should I fix this also :
#13488
usage of mix in particles.cpp seems a little buggy (random from 1 is a little strange)?

@matrem
Copy link
Contributor Author

matrem commented Dec 15, 2017

  • factorized a bit pi usage, and random function between -1,1.
  • fixed spread
  • fixed flatness parameter, it was not implemented (I hope I understand it correctly)
  • it seems pi usage is ok, but as it's mentioned in 3D particles cannot rotate #10182 base angle / angular velocity and other things are not well implemented in 3d particles.

Also I think these :

@akien-mga akien-mga merged commit 1c18943 into godotengine:master Dec 16, 2017
@reduz
Copy link
Member

reduz commented Dec 16, 2017

Seems good

Regarding the suggestions, I think randomness works more or less OK I think for most cases, any alternative I can think of makes it more complicated for the common use cases. Lifetime cant be randomized due to how the shader works, or more likely I suppose It can (can be made shorter I guess) but it wont affect the amount of particles computed simultaneously.

@matrem
Copy link
Contributor Author

matrem commented Dec 16, 2017

Ok personally I can't understand the current randomness behaviour (for example the scale) :

  • You set the scale to 5
  • Then you set the randomness to full (1)
    The scale will be chosen between 1 and 5. Where does the 1 comes from? I never say that my random value should get in the [1, 5] range.
    -If you set to middle (0.5), the scale will be chosen in [3,5], ok... why 3?

It seems to mee, that it is really much more understandable and practical to define randomness with a mean value and a variation (absolute or relative) around this mean value.
For example 5 and 2 get me the [3, 7] range.

But perhaps there is something I didn't catch.

@matrem matrem deleted the particles_spread branch December 16, 2017 15:37
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.

3 participants