Skip to content
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

Explicit definitions API overhaul #35

Closed
Vorlias opened this issue Mar 5, 2021 · 8 comments
Closed

Explicit definitions API overhaul #35

Vorlias opened this issue Mar 5, 2021 · 8 comments
Labels
Enhancement New feature or request
Milestone

Comments

@Vorlias
Copy link
Member

Vorlias commented Mar 5, 2021

Deprecate Net.Definitions.Event for Net.Definitions.ServerEvent, Net.Definitions.ClientEvent.

The difference here is

ServerEvent - An event fired by the server and received by a client.
ClientEvent - An event fired by the client and recieved by the server.

The typings are updated so

A ServerEvent grabbed on the server can only fire, and on the client can only recieve.
A ClientEvent on the server can only recieve and on the client can only fire.

This is only a typing difference, behaviorally for Luau it would not matter.

I'm not sure on the naming schemes here though. Are they too ambiguous? what would work better?

@Vorlias Vorlias added Enhancement New feature or request help wanted labels Mar 5, 2021
@Vorlias Vorlias added this to the 2.1 milestone Mar 5, 2021
@Vorlias
Copy link
Member Author

Vorlias commented Mar 5, 2021

I'm also contemplating breaking down AsyncFunction into Server/Client too for the same sort of type isolation. Function will stay the same as it only works one way anyway.

@OverHash
Copy link
Contributor

OverHash commented Mar 5, 2021

I support the motion to isolate the events like this, but am dubious of the naming. Adding Client* implies that the event is only to be used on the client. Maybe a different prefix should be found.

Additionally, how would you define a bidirectional remote? Or are you going with the idea to not allow that behaviour?

@Vorlias
Copy link
Member Author

Vorlias commented Mar 5, 2021

I support the motion to isolate the events like this, but am dubious of the naming. Adding Client* implies that the event is only to be used on the client. Maybe a different prefix should be found.

Additionally, how would you define a bidirectional remote? Or are you going with the idea to not allow that behaviour?

Do you have an example of a bidirectional remote that wouldn't be achievable with AsyncFunction ?

Also yeah the naming I'm still iffy about myself, I am wanting to find alternatives - could do things like ClientToServerEvent and ServerToClientEvent or something, but still don't know about that.

@OverHash
Copy link
Contributor

OverHash commented Mar 6, 2021

An example of a bidirectional event that I use in my game is when someone adds/removes an item from trading. The client will tell the server, and then the server will fire off to the other player.

It would be good to support these use cases.

Having verbose names like ClientToServerEvent is not a bad idea.

@Vorlias
Copy link
Member Author

Vorlias commented Mar 8, 2021

An example of a bidirectional event that I use in my game is when someone adds/removes an item from trading. The client will tell the server, and then the server will fire off to the other player.

It would be good to support these use cases.

Having verbose names like ClientToServerEvent is not a bad idea.

Could have BidirectionalEvent perhaps, since that's more descriptive for that?

@OverHash
Copy link
Contributor

OverHash commented Mar 9, 2021

Yep, thought that was a good name too.

@Vorlias
Copy link
Member Author

Vorlias commented Mar 20, 2021

Members added:
Net.Definitions.BidirectionalEvent
Net.Definitions.ServerToClientEvent
Net.Definitions.ClientToServerEvent
Net.Definitions.ServerAsyncFunction
Net.Definitions.ServerFunction
Net.Definitions.ClientAsyncFunction

Deprecated (and will be removed in 2.2):
Net.Definitions.Event
Net.Definitions.Function
Net.Definitions.AsyncFunction

@Vorlias Vorlias changed the title Definitions isolation Explicit definitions API overhaul Mar 20, 2021
@Vorlias
Copy link
Member Author

Vorlias commented Mar 20, 2021

I did think about prefixing them with Define but it might make those names too long.

@Vorlias Vorlias closed this as completed Mar 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants