search

playcanvas / engine

JavaScript game engine built on WebGL, WebGPU, WebXR and glTF
https://playcanvas.com
MIT License
9.57k stars 1.34k forks source link

Add the "emitter" property description for the particle system in the manual #2729

Open superluffa-kei opened 3 years ago

superluffa-kei commented 3 years ago

Hi,

Before reporting the issue #2704, I had checked the manual first. As there was no way to change a certain property of the particle, I reported the issue about adding a certain property(e.g. depth test). However, according to @yaustar, there is a way to modify the material of the emitter directly, which is undocumented.

If this "emitter" property is officially provided by the engine, can the team add it accordingly so that we can use it without feeling that it's undocumented API?

Thank you

superluffa-kei commented 3 years ago

(...And @yaustar, how did you find those undocumented properties? )

LeXXik commented 3 years ago

We usually do it by looking at the source code. If you find a case, when someone is using some undocumented property, it means this API is not public and you are advised not to use it, as it might change in the future without notice. Instead, you are encouraged to submit an issue, like the one you made here, to add the feature. The team can then evaluate whether the private API can be exposed, or some additinal functionality is needed.

As for the particle emitter, every Particle Emitter Component has an instance of a ParticleEmitter class: https://github.com/playcanvas/engine/blob/781de090904fc9d128067e5c2d3e519e46268ff1/src/framework/components/particle-system/component.js#L627

entity.particlesystem.emitter

When ParticleEmitter is instantiated, it creates an instance of Material internally: https://github.com/playcanvas/engine/blob/781de090904fc9d128067e5c2d3e519e46268ff1/src/scene/particle-system/particle-emitter.js#L691

@yaustar accessed that material property via

entity.particlesystem.emitter.material
willeastcott commented 2 years ago

The ParticleEmitter class is currently private and I don't think we want to make it public. It's really supposed to be the inner workings of the ParticleSystemComponent (handling the rendering).

I think a better solution might be to expose ParticleSystemComponent#material as a getter (which internally would do return this.emitter.material;. Does anybody have any thoughts on this? @mvaligursky perhaps? @yaustar?

yaustar commented 2 years ago

I guess it depends on possible use cases for doing so. Exposing does give developers a lot of flexibility though and is something we should consistently do across the components like text elements etc

mvaligursky commented 2 years ago

I agree we should not expose the ParticleEmitter class - it's internal implementation, and I already see the future where for WebGPU we will have completely different implementation based on compute shaders. What is needed to be exposed should really be exposed on the ParticleSystem.