VectorEffects #2577
VectorEffects #2577
Conversation
yaRnMcDonuts
added
the
Feature Request / Proposal
|
In order to add the NoiseVectorEffect, I also will have to add a procedural noise library to the engine. The best choice for a fully-fledged noise library in java would appear to be the JNoise library, however I believe that would be overkill for basic noise functions that are needed for this PR, so instead I am choosing to implement the java FastNoiseLite library into the engine. This is a very light weight single class noise library that should be more suited to include in the engine than something like JNoise. Let me know if anyone disagrees or prefers a different noise library, otherwise I will move forward with this plan to include FastNoiseLite.java in the engine's math package. I also will update this PR to include javadoc before merging. |
Add FastNoiseLite (MIT license) from https://github.com/Auburn/FastNoiseLite/tree/master
|
🖼️ Screenshot tests have failed. The purpose of these tests is to ensure that changes introduced in this PR don't break visual features. They are visual unit tests. 📄 Where to find the report:
✅ If you did mean to change things: ✨ If you are creating entirely new tests: Note; it is very important that the committed reference images are created on the build pipeline, locally created images are not reliable. Similarly tests will fail locally but you can look at the report to check they are "visually similar". See https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-screenshot-tests/README.md for more information Contact @richardTingle (aka richtea) for guidance if required |
|
I re-ran the jobs and this time the screenshot tests passed. I guess something must have bugged out during the first run. |
|
Yeah, that's decidedly weird. I can't see why it decided to post that (especially not 2 hours ago). Hopefully just something bugging out as you say |
|
I just need to upload the final example for the noise effect and then I'll merge this PR. I will wait atleast another day since no one else reviewed. However, I've done some thorough self-review, also using AI analysis tools to critique my code and the approach. And I'm very confident that this is the best way to handle these types of effects, as opposed to hard coding things like fading and flickering into the shaders or java classes for lights, emitters, and custom shader effects. |
|
|
||
|
|
||
| if(vectorEffect.isFinished()){ | ||
| getApplication().enqueue(() ->{ |
There was a problem hiding this comment.
why do you need to enqueue this?
There was a problem hiding this comment.
I ran into some issue while removing with an iterator when the effect got set as repeating, which is why I used enqueue as a hacky approach to ensure safe removal from the list on the next frame. I think it is possible for a vector effect to be flagged finished for a single frame when set as repeating and tirgger removal when it shouldn't, and by using enqueue it gives it time to get un-flagged as finished before the next frame (I know enqueue is typically used for thread safety, but it also works as a convenient way to solve issues that require waiting until the next frame without having to add a list to store things that need removed on the next frame). But I'll check again and see if I can solve this in a less hacky way if you prefer that.
There was a problem hiding this comment.
If that's the case, i think isFinished returning true when an effect is not finished is a problem, code can call that method at any time during the effect lifecycle, and it would cause very unexpected behavior.
| } | ||
|
|
||
| public void registerVectorEffect(VectorEffect vectorEffect){ | ||
| if(activeVectorEffects != null){ |
There was a problem hiding this comment.
activeVectorEffects cannot be null, since it is final
| else { | ||
| throw new IllegalStateException("VectorEffect supports only Vector2f, Vector3f, Vector4f, or ColorRGBA"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Better avoid doing internal clones like that, just pass a Vector4f out to getAsVector4 and set it.
Besides, sometime you call getAsVector4(i) and then you clone the returned value again
There was a problem hiding this comment.
ah yes i did leave some redundant cloning there in 2 places I see. I'll remove them and rewrite the method to take in a Vector4f store parameter instead of cloning internally.
jme3-effects/src/main/java/com/jme3/vectoreffect/VectorGroup.java
Outdated
Show resolved
Hide resolved
|
This is quickly growing beyond the scope of what I intended, and tests are all broken now after trying to take an approach that is not my preferred one in order to follow java best practices/conventions. I was unaware of the standards expected of such a system. My original system works for my game otherwise, and no one else seems quite as excited about the new feature as I was anways, so I see no reason to spend more time on trying to contribute this. |
Adds a simple but highly reusable AppState that runs effects directly on a valid Vector type object (being Vector2f, Vector3f, Vector4f, and ColorRGBA).
Useful for lights, particle effects, and essentially every Color or Vector based MaterialParamater that shaders and special effects rely on.
The most useful functionality of this system will be the ability to quickly deploy an
EaseVectorEffecton colors to smoothly fade in/out things like lights and special effects, as to avoid the sudden and unsightly impact of removing something from the scene instantaneously in one single frame. This type of smooth fading is a very important aspect of polishing a game. And while the code to fade out colors can be simple, it is something that is used over and over again for a variety of different effects in a project and can sometimes requires some not-so-simple extra bells and whistles (like a delay timer or chaining), thus a versatile system like this becomes necessary to handle this in an efficient way.In addition to smooth fading, this library also enables a variety of other compound loopable effects by using the SequencedVectorEffect class (such as flickering, pulsing, oscillating, etc), and I also will be adding a NoiseVectorEffect class soon as well.