Tested! This is great. And in general I feel this is a great approach – particle effects done this way look completely native to CGE, and are configurable in editor.
As a bonus, this proves that you can implement new CGE components outside of CGE codebase too, and they integrate nicely, look as proper part of editor. So I’m also happy with the editor improvements this has caused
TCastleSceneCore is not ideal. It exposes properties like
Spatial, code can also access ancestor
TCastleSceneCore.URL that is probably confusing on the emitter. I would propose to remake it as
TCastleTransform descendant – attaching a patch, it seems rather straightforward.
Unless you have future plans that justify that
TCastle3DParticleEmitterGPU should remain descending from
emitter_descend_from_castletransform.patch (1.7 KB)
I’d expose more properties on “Basic” tab, of both the emitter and effect classes, by overriding
PropertySections. At least the
TCastle3DParticleEmitterGPU.Effect. Like this:
function TCastle3DParticleEmitterGPU.PropertySections(const PropertyName: String): TPropertySections;
case PropertyName of
Result := [psBasic];
Result := inherited PropertySections(PropertyName);
This is the basic way to “emphasize” most important properties that user’s may want to tweak to create a particle system.
I’d consider renaming
TCastle3DParticleEmitterGPU to something more general, to avoid breaking compatibility later. Like
GPU name parts can be dropped – as you say it is for 2D too, and the “GPU” is an implementation detail (that ideally should be hidden from users, and have a fallback rendering without transform feedback; though I know that implementing this would be a beast).
A code convention note – at least in CGE code we generally do not write
Self.Xxx, we write just
Xxx (probably with some justified exceptions).
It’s not a problem for me when this is a component outside of CGE codebase, I understand it’s a matter of code style and you may want to just prefer writing
Self. explicitly. There are languages where writing
this is a convention (or necessity) and it makes sense. So I’m just noting it here – I will very much want to merge your work at some point into “CGE core” to have particle effects available out-of-the-box in CGE, and then this code style will be important, to be consistent with rest of CGE.
You mention in REDME “The editor source code is licensed under GNU v2 due to some of its code is borrowed from view3dscene.” – which parts? If they are some small parts, that I’m the sole author/copyright owner, then I will possibly be 100% OK with relicensing these bits for you on MIT, and make the licensing of cge-3d-particle-emitter simpler.
I see you depend on user defining bounding box explicitly. This is visible in
demos/gallery/ where only one emitter shows bbox. Hm, and that’s actually OK.
From what I see, we have here some CGE bug. Because even when you have a bounding box non-empty (on the “Box” emitter in
demos/gallery/), the emitter is still not pickable by CGE editor “Select Scene” (it should actually allow to select any
TCastleTransform that is the leaf of tree in
Viewport.Items, so it should highlight/select emitter too when you mouse over/click) or “Move Transform” tools. From a first glance, seems like a problem on CGE side, I’ll look into it soon.
I see you have alternative ways of assigning the effect – by assigning
Effect, or by setting
URL. I would say to simplify it, and force the user to explicitly create
TCastle3DParticleEffect instance in all cases, and assign it to emitter
Effect explicitly. This is more consistent, and it is then clear what is the memory management.
E.g. now calling
TCastle3DParticleEmitterGPU.LoadEffect(const AURL: String) multiple times on a long-living
TCastle3DParticleEmitterGPU is not optimal. The application will not have memory leaks, but it will internally create more and more effects that are owned by
TCastle3DParticleEmitterGPU, while actually only the last one is used and accessible.
Maybe you want to have 2 effect descendants, one that allows to customize all values , and another one that allows to load particle effect from various formats, without customizing the values (thus it would only publish a
Or maybe just drop loading of the effects from special file formats, and depend entirely on designing / serializing particle effects by CGE.
Or maybe support only
.castle-component format to define a particle effect, by having alternative effect class like
TCastleParticleEffectDesign similar to
TCastleTransformDesign that would refer to another
.castle-component file, and require that as root it contains
In any case, keep the simple rule: to have emitter work, one has to assign
TCastle3DParticleEmitterGPU.Effect. There is no shortcut
TCastle3DParticleEmitterGPU.URL. This would simplify IMHO a bit the relation effects<->emitters in code.