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

Arbitrary Effects System #537

Merged
merged 7 commits into from Apr 5, 2018

Conversation

Projects
None yet
3 participants
@clusterfack
Member

clusterfack commented Mar 12, 2018

This a system to make temporary clientside effects without the creation and syncing of full entities.

You can define how you want the effect to look, move, and change over time by specifying a number of variables in the effect creation message. After creating one with the prerequisite variables you can pass it to the server effect entity system and it will send it to clients, and the client effect entity system will render them.

clusterfack added some commits Mar 12, 2018

Adds an arbitrary effects system
By creating a message defining things such as position, spritename, rotation, size, and delta values, you can create a temporary effect on the client. Just create a new message, define all the values upon it, pass it to the serverside effects system to send to the clients.
@Acruid

Other than some minor issues, looks good!

@@ -62,6 +62,7 @@ private static void Main()
{
RegisterIoC();
RegisterComponents();
IoCManager.Resolve<IEntitySystemManager>().Initialize();

This comment has been minimized.

@Acruid

Acruid Mar 12, 2018

Contributor

This should go somewhere inside SS14.Client.GameController with the rest of the initialization calls.

This comment has been minimized.

@clusterfack
namespace SS14.Shared.GameObjects.EntitySystemMessages
{
[Serializable]
public class EffectSystemMessage : EntitySystemMessage

This comment has been minimized.

@Acruid

Acruid Mar 12, 2018

Contributor

This is a large packet to be sending to all 200 clients for every visual effect created. Is there any way to make it smaller? Do we really need Velocity, RadialVelocity, and TangentialVelocity sent separately?

This comment has been minimized.

@clusterfack

clusterfack Mar 12, 2018

Member

Sent separately as opposed to what? Do you save space putting them all in a vector or something?

This comment has been minimized.

@Acruid

Acruid Mar 13, 2018

Contributor

I was hoping some of these values could be derived client side to reduce the size of the message, but I don't think that will be possible since I realized we can have particles spiraling outwards while moving linearly through the world.

int compare = y.DeathTime.CompareTo(x.DeathTime);
////If the comparer returns 0 sortedset will believe it is a duplicate and return 0, so return 1 instead
//if (compare == 0 && !x.Equals(y))

This comment has been minimized.

@Acruid

Acruid Mar 12, 2018

Contributor

Commented out code needs to be removed. The doc comment summary for this class also refers to powernet priority?

This comment has been minimized.

@clusterfack

clusterfack Mar 12, 2018

Member

When I used the priority comparer in powernets having it return 0 would cause it to be considered a duplicate and removed so I was on the fence here about whether thats also necessary for this priority queue or not

effect.Deathtime = effect.Age + TimeSpan.FromSeconds(4);
//Age the effect through a single update to the previous update tick of the effect system
//effect.Update((float)((lasttimeprocessed - effect.Age).TotalSeconds));

This comment has been minimized.

@Acruid

Acruid Mar 12, 2018

Contributor

No commented out code.

This comment has been minimized.

@clusterfack

clusterfack Mar 12, 2018

Member

This is how its supposed to work, if the timing system worked the code a line above it would be removed

/// <summary>
/// Effect's current color
/// </summary>
public Vector4 Color = new Vector4(255,255,255,255);

This comment has been minimized.

@PJB3005

PJB3005 Mar 17, 2018

Member

Why is this being stored as a Vector4 instead of a Color?

This comment has been minimized.

@clusterfack

clusterfack Mar 18, 2018

Member

Because it can store wildly abnormal values

/// <summary>
/// Effect's spin about its center in radians
/// </summary>
public float Rotation = 0f;

This comment has been minimized.

@PJB3005

PJB3005 Mar 17, 2018

Member

Use Angle maybe?

This comment has been minimized.

@clusterfack

clusterfack Mar 18, 2018

Member

That's just bikeshedding my dude

This comment has been minimized.

@PJB3005

PJB3005 Mar 18, 2018

Member

How is that bikeshedding? We have a standardized type for storing angles, so use it. You're not gonna store a vector as a tuple (float, float) either are you?

This comment has been minimized.

@clusterfack

clusterfack Apr 2, 2018

Member

I looked at this and its fairly pointless because its meant to be replicated on the client, so storing it in Angle form on the client as its a struct is silly since it gets continuously remade in an update() function from the system

clusterfack added some commits Apr 2, 2018

@clusterfack clusterfack referenced this pull request Apr 2, 2018

Merged

Adds weapons #48

@PJB3005 PJB3005 merged commit cfd34e4 into space-wizards:master Apr 5, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@wafflebot wafflebot bot removed the W: Review label Apr 5, 2018

@clusterfack clusterfack deleted the clusterfack:effectsystem branch Apr 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment