@monkeychops said: Smartest buffoon on this forum then :P You have made the best particle emitter, the best gui system and the best paging system... all of which I consider absolutely essential to the success of my future AAA title :)
I’d like to argue, but alas I can’t. He’s got a point. 
EDIT: And I might add: In what, 2 months? Ha. Buffoon my ass.
@nehon and anyone else who wants to play around with it
The source has been dumped out to:
Here are the outstanding issues:
Keep in mind… this will more than likely be cleaned up, changed appropriately by the powers that be to make sure you are getting awesomeness.
Now, for some basic usage as it stands right now:
Creating influencers:
[java]
ColorInfluencer ci = new ColorInfluencer(); // yep, that’s it
ci.setStartColor(1,1,1,1); // or some ColorRGBA, anyways… you get the idea
[/java]
Creating an emitter: (choices… there is an alternate constructor that allows you to pass in the influencers you would like loaded… see java doc info)
[java]
// Load an emitter shape (mesh)
Node hemiphereEmitter = (Node)assetManager.loadModel(“Models/SphereEmitter.j3o”);
Mesh hemiphereEmitterMesh = ((Geometry)hemiphereEmitter.getChild(0)).getMesh();
/**
* Creates a new instance of the Emitter class including influencers.
* @param name The name of the emitter (used as the output Node name containing the ParticleDataMesh)
* @param assetManager The application’s asset manager
* @param shape The Mesh to use as the particle emitter shape
* @param type The particle type (point, triangle, etc)
* @param maxParticles The maximum number of particles handled by the emitter
* @param preloadAllInfluencers loads all influencers if true, else loads core influencers (Gravity, Size, Color, Rotation and Impulse)
*/
Emitter fire = new Emitter(“fire”, assetManager, hemisphereEmitterMesh, ParticleDataMesh.Type.Triangle, 80, false);
fire.setSprite(“Textures/smoketrail.png”, 85, 256); // set texture sprite info
fire.setEmissionsPerSecond(20); // set emissions per second
fire.setParticlesPerEmission(2); // Set particles emitted per emission
// fire.setUseStaticParticles(true); // Optional setting for static particles that follow animated mesh
fire.setForceMinMax(9.2f,15.3f); // Set initial force params
fire.setLifeMinMax(.45f,1.45f); // Set particle life duration ounds
fire.setBillboardMode(BillboardMode.Velocity_Z_Up); // Set billboarding option
fire.setEnabled(true); // start emitter
rootNode.addControl(fire); // this is all that is needed if the node contains the mesh that will be used by the emitter other… you need to add the emitter node to the scene separately. The mesh param of the construstor should probably be removed, as it can be set after the fact.
rootNode.attachChild(fire.getNode());
[/java]
Configuring preloaded influencers:
[java]
fire.getInfluencer(ColorInfluencer.class).setStartColor(1,1,1,1); // You get the idea
[/java]
Sorry a little more info…
There really needs to be a restructure of the constructor before this goes out, with the addition of default mesh shapes. Though, I believe it’s important for people to be able to use/give feedback while this happens.
Because I wasn’t sure how this was going to handled, it is in a bit of a confused state at the moment. You have to add a mesh in the constructor, but you likely override what you added along the way. Sorry for this in advance… please don’t hate me!
@wezrule said: Has this been tried on the Android phones which fail with the current Particle Emitter?
The particle mesh class is much cleaner now, though, @nehon pointed out something that will still make things slow on android. I have not had a chance to update this yet… but it is not a huge change. This emitter should work well on android once the change happens–though, honestly, I’m not sure how buffer manipulation is handled there, and the current seems to run faster on desktops then the alternative… I wish I knew more about this… maybe some day! Thankfully we don’t have to rely on my knowledge to make this happen correctly)
One final request =)
If you would be so kind, once you find a permanent home for this… if you would allow me to continue to help work on it/maintain it… that would be great. If not, I totally understand! Just let me know… and if yes, where I should be pulling the source from.
@t0neg0d said: If you would allow me to continue to help work on it/maintain it... that would be great.Deal!
@t0neg0d said: I have =) I remember the thread where it was recommended. I have gone through it once in full and multiple time in sections...Things stick with me when I have a chance to use them in practice. (plus, I don’t mind looking like an idiot so I can learn… thus the stupid questions).
ok cool :), it was just something that popped in my head, as I was reading the More Effective C++ book and found that really useful (i’m sure I will forget it all later ^^). I really need to finish the Java one (I never got past page 70 or so, along with many others I bought/downloaded ^^). Yep experience is everything
Hi, I’ve not had time to be super-thorough but I’ve done a quick code review.
Overall, it looks very good 
I’ve seen commercial java projects with waaaaaay worse code.
A correct package needs to be defined obviously, I’m guessing the current one is a placeholder.
Emitter.java
addInfluencer javadoc - “ParticleData Influencer” should be “ParticleInfluencer”.
getInfleuncer - no javadoc
Remove the mapping influencers, you don’t need it - just remove the second s from influencerss and use that. To implement getInfluencer(Class x) just do a for loop over the list and return the first match.
In reality this will be faster than using the hash map even though its less “correct” as the overheads of maintaining the map will be higher than for the arraylist and you are only dealing with small numbers of influencers.
All Influencers, same two points: should have a constructor that sets the params (i.e. start/end colour etc).
rather than update always doing if (enabled) the thing calling update should call isEnabled and only call update if it is.
ImpulseInfluencer setChance/getChance javadoc says “successfully affecting”. That’s a bit confusing, I think you can lose the successfully bit.
PreferredDestinationInfluencer - what happens if I want to influence particles towards 0? Would be better to have preferred as null to do nothing, all other values mean their usual.
PreferredDestinationInfluencer, PreferredDirectionInfluencer - you seem to have got fed up with writing javadoc by the time you got to these ones 
RotationInfluencer -why do you have both a boolean to store direction and a float (inside a v3f) to store speed for angular velocity? Just use the float and if it goes negative that’s the reverse direction.
i.e. in the p.rotationSpeed.set use: FastMath.nextRandomFloat()speedFactor.x2 - speedFactor.x
and then ditch all the booleans and checks for them.
SpriteInfluencer - Rather than storing interval/duration/etc as mappings in the particle you can ditch the mapping and just store in all particles a float spawnTime and in the particle mesh a float meshTime. When new particles are spawned set their spawn time = to mesh time. Each update meshTime += tpf. The SpriteInfluencer update can then just look at ((meshTime - spawnTime) / fixedDuration and calculate the sprite from that. Even if you don’t make that (larger) change all of the strings used as keys should be defined in static final String FRAME_INTERVAL_KEY = “frameInterval” rather than repeated scattered through the file. I’d also recommend calling them SpriteInfluencer.FrameInterval to set a pattern that will avoid name collisions in future. This will remove all need for the Map data = new HashMap();
ParticleData.java
Consider removing data if not needed. At the very least lazy initialize it rather than creating a mapping for every single particle data whether it needs it or not.
Remove the booleans for rotation direction and just use negative velocity.
initialize() does not call data.clear()
ParticleData(X)Mesh.java
These all seem to be lacking javadoc and its not clear at a glance what they are for/how to use them/etc.
Where is the code that does the billboarding? I didn’t see it here on my look through.
@zarch said: Hi, I've not had time to be super-thorough but I've done a quick code review.Overall, it looks very good
I’ve seen commercial java projects with waaaaaay worse code.
A correct package needs to be defined obviously, I’m guessing the current one is a placeholder.
First, let me say… I think you’re being a bit kind here =) I know there are definately things that need to change but the idea was osrt of morphing as it went. Originally, I was trying to use the existing code with minor edits… this ended up being far more trouble than starting from scratch and cut/parte the things that could remain in place.
@zarch said: Emitter.javaaddInfluencer javadoc - “ParticleData Influencer” should be “ParticleInfluencer”.
getInfleuncer - no javadoc
Remove the mapping influencers, you don’t need it - just remove the second s from influencerss and use that. To implement getInfluencer(Class x) just do a for loop over the list and return the first match.
In reality this will be faster than using the hash map even though its less “correct” as the overheads of maintaining the map will be higher than for the arraylist and you are only dealing with small numbers of influencers.All Influencers, same two points: should have a constructor that sets the params (i.e. start/end colour etc).
rather than update always doing if (enabled) the thing calling update should call isEnabled and only call update if it is.
Wouldn’t a single call with a boolean check be more efficient than two separate calls? (this is a very serious question, as I do not know the answer)
I agree about the mapping, I just didn’t have time to update this after the conversation concerning it and putting it out for others to try/comment on/update/etc. The billboarding options took precedence.
@zarch said: ImpulseInfluencer setChance/getChance javadoc says "successfully affecting". That's a bit confusing, I think you can lose the successfully bit.
/agree
@zarch said: PreferredDestinationInfluencer - what happens if I want to influence particles towards 0? Would be better to have preferred as null to do nothing, all other values mean their usual.
/agree
@zarch said: PreferredDestinationInfluencer, PreferredDirectionInfluencer - you seem to have got fed up with writing javadoc by the time you got to these ones :D
Lol… I actually considered not adding these… all the javadoc info was added after completion. This was an oversight on my part when I decided to leave them in place and let others help make them into what they should be. PreferredDirection works well… the other? Not so much =)
@zarch said: RotationInfluencer -why do you have both a boolean to store direction and a float (inside a v3f) to store speed for angular velocity? Just use the float and if it goes negative that's the reverse direction. i.e. in the p.rotationSpeed.set use: FastMath.nextRandomFloat()*speedFactor.x*2 - speedFactor.x and then ditch all the booleans and checks for them.
Um… I don’t have a reason for it… The rotation influencer was written in like 5 minutes. I just noticed the original emitter only allowed for rotation along the z-axis and only in one direction. This was initial brain-dump knowing that all would have to be optomized at some point
@zarch said: SpriteInfluencer - Rather than storing interval/duration/etc as mappings in the particle you can ditch the mapping and just store in all particles a float spawnTime and in the particle mesh a float meshTime. When new particles are spawned set their spawn time = to mesh time. Each update meshTime += tpf. The SpriteInfluencer update can then just look at ((meshTime - spawnTime) / fixedDuration and calculate the sprite from that. Even if you don't make that (larger) change all of the strings used as keys should be defined in static final String FRAME_INTERVAL_KEY = "frameInterval" rather than repeated scattered through the file. I'd also recommend calling them SpriteInfluencer.FrameInterval to set a pattern that will avoid name collisions in future. This will remove all need for the Map data = new HashMap();
This very well might not need to store any data, honestly. Blend, life or startlife… (or some combination of these probably already contains the needed data for recalc’ing this at a cheap cost that the data get/set. I think I did it this way to test the data hashmap, as at some point, this will be necessary when writing some influencer.
@zarch said: ParticleData.javaConsider removing data if not needed. At the very least lazy initialize it rather than creating a mapping for every single particle data whether it needs it or not.
/agree
@zarch said: Remove the booleans for rotation direction and just use negative velocity. initialize() does not call data.clear()
/agree
@zarch said: ParticleData(X)Mesh.javaThese all seem to be lacking javadoc and its not clear at a glance what they are for/how to use them/etc.
These have the same javadocs that the originals do har har! Actually… why there is an interface for this is semi-confusing as the are only 2 choices you’ll ever use… Point or Triangle. This really is carry-over from the original.
@zarch said: Where is the code that does the billboarding? I didn't see it here on my look through.
This is in ParticleDataTriMesh.java as the billboarding needs to be recalulated per frame and point based particles are always facing the camera.
Ahh, ok.
Well then another recommendation as I mentioned earlier: move the logic from the switch statement into the enum.
[java]
public static enum BillboardMode {
/**
* Facing direction follows the velocity as it changes
*/
Velocity {
void applyBillboard(Vector3f left, Vector3f top, Vector3f dir) {
up.set(p.velocity).crossLocal(Vector3f.UNIT_Y).normalizeLocal();
left.set(p.velocity).crossLocal(up).normalizeLocal();
dir.set(p.velocity);
}
}
[/java]
Then the billboarding code just becomes billboardMode.applyBillboard(left, top, dir).
You will need a few more parameters probably, like passing in the particle data or something.
@t0neg0d said: Wouldn't a single call with a boolean check be more efficient than two separate calls? (this is a very serious question, as I do not know the answer)
The isEnabled() will very likely be inlined by hotspot (even more likely when it’s final). At any rate, there is no reason to have setEnabled()/isEnabled() as a standard part of the interface if the thing must do its own enabled checking. AppStates used to be this way too until I fixed them.
So, either call isEnabled() in the loop before calling update() or just ditch setEnabled()/isEnabled() completely. Having isEnabled() has precedence, though. However, if speed is really the issue then I’d ditch the enabled flag completely. The influencers can always be added or removed to enable/disable them.
P.S.: Either way we are talking about teeny tiny nanoseconds in a sea of microseconds.
@zarch said: Ahh, ok.Well then another recommendation as I mentioned earlier: move the logic from the switch statement into the enum.
[java]
public static enum BillboardMode {
/**
* Facing direction follows the velocity as it changes
*/
Velocity {
void applyBillboard(Vector3f left, Vector3f top, Vector3f dir) {
up.set(p.velocity).crossLocal(Vector3f.UNIT_Y).normalizeLocal();
left.set(p.velocity).crossLocal(up).normalizeLocal();
dir.set(p.velocity);
}}
[/java]Then the billboarding code just becomes billboardMode.applyBillboard(left, top, dir).
You will need a few more parameters probably, like passing in the particle data or something.
This needs to happen at a fairly specific time in the update of the buffers. Since I didn’t know that this was even possible with an enum… how will this work from the mesh class? And is it worth it considering that the only place this will ever be used is in the tri mesh class?
EDIT: forget the first part of the question… I see how it works. Just the second still applies.
@pspeed said: The isEnabled() will very likely be inlined by hotspot (even more likely when it's final). At any rate, there is no reason to have setEnabled()/isEnabled() as a standard part of the interface if the thing must do its own enabled checking. AppStates used to be this way too until I fixed them.So, either call isEnabled() in the loop before calling update() or just ditch setEnabled()/isEnabled() completely. Having isEnabled() has precedence, though. However, if speed is really the issue then I’d ditch the enabled flag completely. The influencers can always be added or removed to enable/disable them.
P.S.: Either way we are talking about teeny tiny nanoseconds in a sea of microseconds.
Makes sense to me! As soon as this reaches it’s new home, I’ll see about getting this stuff (along with other things) fixed.
Actually I was just looking for a good example of this enum usage in google and couldn’t find any 
Lots of examples of how to do it wrong but very few getting it right.
Your enum decleration looks like this:
[java]
enum example {
foo,
bar;
}
[/java]
Most people are then familiar with:
[java]
enum example {
foo(“a”),
bar(“b”);
final String blah;
example(String blah) {
this.blah = blah
}
String getBlah() { return blah; }
}
[/java]
But you can go one step further. Every entry in the enum is actually a full java class (effectively it functions as though the enum defines an abstract class and then each entry is an anonymous inner class that inherits from the abstract one). This means you can define methods in the enum, and implement them in each entry
[java]
enum example {
foo(“a”) {
Object doSomething(Object ob) {
// Do something
}
},
bar(“b”){
Object doSomething(Object ob) {
// Do something
}
};
final String blah;
example(String blah) {
this.blah = blah
}
String getBlah() { return blah; }
}
abstract Object doSomething(Object ob);
[/java]
There are two main advantages to doing it this way.
The first is re-usability. If you do another mesh type then you can use the same billboarding logic.
The second is more subtle - but it gets rid of the switch statement and it means that if someone adds a new enum entry then they don’t have to search for all the switch statements and find and update them all and hopefully not miss any. Instead you can immediately see what logic needs implementing to add a new entry to the types of the enum.
If ever you are switching on an enum you should stop and think whether the functionality could be embedded into the enum itself. Sometimes it doesn’t make sense to do it, but a lot of the time it does.
On the subject of enabled I’d be seriously tempted to remove it completely, just have people only add the influencers they need and remove them if they don’t need them any more. But then I don’t like the constructor that creates “default” influencers for people anyway
It’s just a source of confusion. If people want the influencer then they are setting the stats - so they might as well just create it themselves and this way they know what they are creating.
@zarch said: On the subject of enabled I'd be seriously tempted to remove it completely, just have people only add the influencers they need and remove them if they don't need them any more. But then I don't like the constructor that creates "default" influencers for people anyway :)It’s just a source of confusion. If people want the influencer then they are setting the stats - so they might as well just create it themselves and this way they know what they are creating.
I agree with this… no matter which side of the emitter you are manipulating, the # of lines of code are the same… and having the user add them in the constructor, ensures that they know exactly what the emitter can/will do.
The only slight down side is, a bigger difference in usage between the old emitter and the new one.
And THANKS!!! for the info on enums. I am betting I read this at a point, and it didn’t stick. I’m going to mill this over before trying to implement it into something, as I 'd hate to use it for the wrong reasons =)
In the emitter case, it could be done, but honestly, there will never be another particle mesh class added. There are points… and there are quads… and only quads use billboarding, so it may be overkill here. I personally think the ParticleDataMesh interface should be swapped out for a primitive that handles commonality between the two types. Well… unless someone thinks 3d particles is a good idea… but then billboarding isn’t needed
@t0neg0d I’m sorry I’ll be short on time this week end, I won’t be able to work a lot on this.
I peeked at the code and I agree with @zarch in every point.
On the API side, I would add that I’m not fond of the fact that you have to add the control to the rootNode and attach some node to it. This is not intuitive, and it denotes from the rest of JME API.
Intuitively, if I wanted to make a model emit particle, I would add the control to this model. The control would use its mesh as emitter shape.
This is mostly blender’s approach.
But maybe I’m overseeing things.
About the read/write things, don’t worry I’ll help you on this. Everything has to be savable if you want it to be saved in a j3o. It’s kind of vital because we want to be able to tweak those emitter in the SDK.
For now, let’s keep this in your repo, would you mind giving me commit access? I have to take full grasp on the system to be able to know how it will fit into JME.
Great work so far!
@t0neg0d said: And THANKS!!! for the info on enums. I am betting I read this at a point, and it didn't stick. I'm going to mill this over before trying to implement it into something, as I 'd hate to use it for the wrong reasons =)
Regarding enums - it is bit of personal preference. I never put any ‘business logic’ into enums. Same example as zarch was giving - what if you need different implementation of this code depending on type of mesh? Are you going to put processTriangle, processPoint, processQuad, processXYZ all in some enum? From one side, if you are sure that logic will be exactly the same, you have a saving (but same you can achieve by sharing this bit of code outside of enum). On the other hand, if implementations are different, you are coupling enum to every use of it, which sometimes might require putting embarrasing private details into the code inside enum.
@abies said: Regarding enums - it is bit of personal preference. I never put any 'business logic' into enums. Same example as zarch was giving - what if you need _different_ implementation of this code depending on type of mesh? Are you going to put processTriangle, processPoint, processQuad, processXYZ all in some enum? From one side, if you are sure that logic will be exactly the same, you have a saving (but same you can achieve by sharing this bit of code outside of enum). On the other hand, if implementations are different, you are coupling enum to every use of it, which sometimes might require putting embarrasing private details into the code inside enum.
For what it’s worth, I 100% agree with this.
Custom methods on enums are good for providing additional information about the type, ie: immutable concepts. Display names, bitmasks, whatever. Not so good for business logic.
a) switch/case blocks are not that expensive.
b) if it really feels like a good idea to put business logic into an enum then it isn’t an enum, it’s a strategy pattern.