-
Notifications
You must be signed in to change notification settings - Fork 24
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
Animation #47
base: master
Are you sure you want to change the base?
Animation #47
Conversation
Bezier Easing allows you to define any Easing Function possible by defining 4 Cubic Bezier points this implementation is based on and ported from how browsers implent the CSS cubic-bezier easing function added a cache to avoid recomputing the same easing function everytime
This Component Lets the Animation Component Listen to Clicks & Hover Events Multiple Animations Can Listen to the same MouseListener
This Component Drives Animations, it has alot of features. see it here => https://streamable.com/696gos by default this Component Supports: 8 Types of Animation, Custom Easing Functions 4 Distinct Triggers [OnClick, OnHoverEnter, OnHoverExit, OnDestroy], delay, repeat & repeatDelay This Component also Allows Adding Animations Afterwards via RPC Calls
Added Integration for the Animation, RectMask2D & Mask Components, also added option to attach a sub canvas to panels to optimize animations & reduce redraws the idea is that something that gets constantly animated will dirty the canvas every frame, it wont dirty parent canvases though, putting heavily animated components into their own canvas reduces redraws for the entire UI which may improve performance
see next commit
abstracting receivers into an interface so i can the MouseListener for other classes
i made some changes to embrace multiple animation components on the same object instead of restricting them, as they allow us to react to mouse events on different targets. the new code will attempt to re-use an existing animation component if theres no mouse target specified or an existing component with the same target allready exists here's a demo of what that allows us to do (https://streamable.com/b5efj7) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some general advice. The only thing I didn't include is you should be including a lot more comments so we can at least understand what each method does. (like what is binary subdivide?)
CommunityEntity.UI.BezierEasing.cs
Outdated
// this basically is the same process that Browsers use for their CSS easing, allowing you to define any possible easing value with 4 bezier points | ||
// had to port this from a javascript version | ||
|
||
public static Dictionary<string, BezierEasing> cache = new Dictionary<string, BezierEasing>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This dictionary should probably be a Dictionary<CustomStruct,BezierEasing> to avoid string allocations every time you access it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CommunityEntity.UI.BezierEasing.cs
Outdated
|
||
|
||
|
||
float A (float aA1, float aA2) { return 1.0f - 3.0f * aA2 + 3.0f * aA1; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 letter methods are unreadable, especially when there is no documentation provided... pretty much never do this 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i agree, in this case the three functions are a math equation broken up. i added a comment at the top but kept the variable names as changing them to "Part1/2/3" wouldnt have added any clarity that the comment doesnt allready adds
since i ported this from a js port of gecko's implementation i couldnt find a definition/description for what this formula is, otherwhise i would have changed the function names to include it
CommunityEntity.UI.BezierEasing.cs
Outdated
} | ||
|
||
// Returns dx/dt given t, x1, and x2, or dy/dt given t, y1, and y2. | ||
public float getSlope (float aT, float aA1, float aA2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This variable names are also very difficult to understand
CommunityEntity.UI.BezierEasing.cs
Outdated
return currentT; | ||
} | ||
|
||
public BezierEasing (float X1, float Y1, float X2, float Y2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method is fine because x & y are commonly understood
CommunityEntity.UI.BezierEasing.cs
Outdated
mY1 = Y1; | ||
mX2 = X2; | ||
mY2 = Y2; | ||
mSampleValues = new float[kSplineTableSize]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to reuse the float array rather than allocating a new one each time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CommunityEntity.UI.BezierEasing.cs
Outdated
} | ||
} | ||
|
||
void precompute() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In C# conventionally method names are PascalCase even if they are private
CommunityEntity.UI.MouseListener.cs
Outdated
var c = hObj.GetComponent<MouseListener>(); | ||
if(!c) c = hObj.AddComponent<MouseListener>(); | ||
|
||
c.onEnter += new Action(receiver.OnHoverEnter); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to do
c.onExit += reciever.OnHoverExit
rather than creating a new Action()
CommunityEntity.UI.MouseListener.cs
Outdated
} | ||
} | ||
|
||
public interface IMouseReceiver{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New classes / interface should be declared in a seperate .cs file
CommunityEntity.UI.BezierEasing.cs
Outdated
int currentSample = 1; | ||
int lastSample = kSplineTableSize - 1; | ||
|
||
for (; currentSample != lastSample && mSampleValues[currentSample] <= aX; ++currentSample) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure it should be
for (int currentSample = 1; currentSample < lastSample & mSampleValues[currentSample] <= aX; currentSample++ )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for taking the time to look through it, i really appreciate it! i've made some of the changes you proposed & replied to some of the comments, but feel free to let me know if i misunderstood or missed something, i also added a clarification to the BezierEasing file that it is a port from an existing js library (which is ported from gecko), while porting it my main goal was to convert it to C# Syntax which i was able to do without deep understanding of the underlying math/code so the naming & comments mostly stayed the same. i did dig deeper into it to get a better understanding of the methods used and wrote some comments to address it, but i couldn't find much info on the main Math equation. |
65d1072
to
9fad421
Compare
Refactor Code to support multiple Animations Changes to how animations are created, allows for adding multiple components if they have different mouseTargets
Adjusted RPC call behaviour to follow the same code that the AddUI code does, it will attempt to add animations to existing components if a matching mouseTarget exists
the last dropdown example does require the scrollview pull request, though it should still show up without it present
Changed Condition Naming to Trigger as it makes more sense, Changing the default "Generic" to "OnCreate" to get more consistent nameing Added Ability to Remove Properties, automatically remove "OnCreate" Properties after they are over Remove Properties if they fail due to bad arguements, log a message stating which type of animation failed Changes based on Jake's Review: - added a bunch of comments to methods - added clarification to the BezierEasing class that its ported from a Javascript version of Gecko's CSS implementation - added a BezierPoints Struct which is just a collection of the 4 points to be used in the cache lookup, reducing string allocations - Seperated IMouseReceiver Interface into own File - Removed uneccesary new Action() calls
changed condition to trigger in example files
Moved Animation initialization to own function, RPC and AddUI case statement now mostly share the same init code
this change removes alot of redundant code by switching to a dynamic vector & using an action to apply the values instead of having dedicated functions for each action before this, all animations used 1 or more floats as inputs, making use of Vector2, 3 & 4s or Color as needed. because of this they used the same code with minor tweaks causing a bunch of functions that did the same thing slightly differently. with this change all animations use this one system: - the property gets an AnimationValue that holds the from & to values as a Dynamic Vector (a List<float> wrapper with some added functionality) - in the switch statement evaluating the animation's Type, the AnimationValue's apply action & initial value gets set depending on the type - the InterpolateValue Enumerator gets called which receives the AnimationValue, a duration and easing descriptor - this function applies the from value immediately if it exists, then interpolates over the duration & sets a final value at the end - all it needs to apply the value is call the AnimationValue's apply function, which gets set in the type's case not only does this remove the need for 12 similliar functions, it also creates easier extendability as it now only requires adding cases to the switch statement the interpolation logic will allways remain consistent, the DynamicVector class also means that any animations needing 5 or more floats wont need additional code for parsing & Lerping
b20287d
to
4eb4d60
Compare
Changing the DynamicVector & AnimationProperty to be structs instead of objects part of this came about thanks to a conversation i had with WhiteThunder, so he deserves a shoutout Fixed MoveTo & MoveToPX issues with the last commit the MoveTo & MoveToPX had a faulty apply function that was applying the lerped value directly, instead of using their difference fixed mssing using System.Text for the StringBuilder
move example Files to Folder
this is now handled by the Animation Component to add support for fading out via CanvasGroups
+ Cleanup
Fixed Animations not getting stopped properly, this was partially because of AnimationProperty being a struct, another issue was that coroutines were not getting set to null after stopping them.
fixed wrong type an AddComponent made both cases Update compliant
its possible that this wouldnt have been an issue once it was added to the client, but it was producing issues during client mod testing
overall cleanup, naming improvements & only things that need to be public are public the math is still undocumented magic, but thats what math just is preperation for BezierPoints to be used directly, defined some default curves for comparison use
instead of keeping the string and parsing it everytime we try to ease with a custom bezier function it now parses it once and re-uses it for the entire property's lifetime. we still keep the static easing functions as they have better performance, they are now compared to a static readonly Points struct that also happens to represent their bezier curves, if we need to add any functions that cannot be expressed with a single cubic bezier, an invalid curve can be used for comparison this should reduce the overhead of easing by removing the need to reparse the string every update, which also gets rid of the array & string allocations it produced
Removed extra Client RPC - the CUI update mechanism works just fine for adding new animations Added Reset json property and function - if set, stops and removes all properties on an animation before new properties are added switched to using CoroutineEx.waitForSeconds instead of allocating new ones all the time (i somehow missed the client having this and thought it was server only for some reason until now) optimized InterpolateValue to bypass if start and end value are equal, this is fairly common in setups like the second animation example where different animations are ran depending on which tab is clicked. in the case where an object doesnt need to move it will just wait for the duration instead of assigning the same value every frame moved CommunityEntity instance functions into respective classes as a static function, made additional functions static
Allows Toggling if a graphic should obstruct clicks or not among other things, this enables developers to have graphics as overlays. defaults to true, next commit enables animations to respect this behaviour again (they used to already)
this was removed in the May 17 commit where the animation component got refactored, though there weren't many situations previously where raycasting would be disabled for an element, this is now needed to support both draggables & toggling raycasting on graphics again
How is this still not implemented... It's beautiful. |
Adding an Extendable Animation Component
Summary
this Fork adds an Animation Component that allows you to animate various properties of a UI Panel. it currently supports:
AnimationFeatureShowcase.mp4
Why?
Removing the static feeling from UIs
UI heavy plugins often feel very static, once the UI is on the screen it stays idle until either the server or player does something
Creating Viability
Its already possible to create animated UIs with current features, as fadeIn and fadeOut give a degree of animation, but this comes at the cost of server strain, very limited possibilities & a difficult developer experience
Setting the foundation
The Component was designed without focusing on a single usecase, instead trying to be as versatile as possible to allow anyone to benefit from it, its intent is to let any developer make more pleasant UIs that are cleaner, more efficient & more interactable. the animation logic is generalized, so that adding new animation types can be easily added
The Breakdown
this fork introduces a few new Classes:
to use the animation system, a developer can add the component by using the following Json Representation
Animation Components can be be used with multiple Properties to create complex Animations
Usecase & Comparison
Animations are a great way to make your UIs more pleasing to look at & snappy. but what's more important is that they bring interactivity to your UIs. the ability to listen to hover, click & drag Events allows you to Create more intuitive UI that feels dynamic & responds to the user.
It allows you to create:
MultiAnimation.Example.mp4
without this PR, when presenting lists & content on multiple pages we have to utilize console commands to update the page for the player. this PR allows developers to send all the pages & content at once (within reason) & letting the animations take care of switching pages, opening dropdowns & modals. this reduces the requests needed & makes the UI feel snappier.
All of the code in this fork have been fully tested by recreating the CUI system in unity, meaning that they should mostly work out of the box, this fork will also include 2 json files: