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

Update: Node, Connection, Poru #102

Merged
merged 8 commits into from
Mar 29, 2024
Merged

Update: Node, Connection, Poru #102

merged 8 commits into from
Mar 29, 2024

Conversation

Joniii11
Copy link
Contributor

Node.ts:

- Update `<Node>.connect` to be asynchronus for establishing a connection to the node.
- Added to customize the Client Name to allow for other libraries besides Lavalink (*caugh caugh NodeLink caugh caugh*)

Connection.ts:

- Fix type `IVoiceServer` to make endpoint optional (refer to discord docs)

Poru.ts:

- Added 3 new interfaces to represent the Packets from discord (refer to discord docs)
- Update types from `<Poru>.send` function
- Update types from `<Poru>.packetUpdate` to use the new types

Have a nice day o/

@Joniii11
Copy link
Contributor Author

Damn the CI is failing cause of building website

@UnschooledGamer UnschooledGamer requested a review from parasop March 28, 2024 11:23
@UnschooledGamer
Copy link
Collaborator

Damn the CI is failing cause of building website

Yes cause Paras isn't listening to me to setup the docs and now it's in backlog, & now I'm tired of asking him everyday.

@UnschooledGamer
Copy link
Collaborator

Node.ts:

- Update `<Node>.connect` to be asynchronus for establishing a connection to the node.
- Added to customize the Client Name to allow for other libraries besides Lavalink (*caugh caugh NodeLink caugh caugh*)

Connection.ts:

- Fix type `IVoiceServer` to make endpoint optional (refer to discord docs)

Poru.ts:

- Added 3 new interfaces to represent the Packets from discord (refer to discord docs)
- Update types from `<Poru>.send` function
- Update types from `<Poru>.packetUpdate` to use the new types

Have a nice day o/

Node.ts:

- Update `<Node>.connect` to be asynchronus for establishing a connection to the node.
- Added to customize the Client Name to allow for other libraries besides Lavalink (*caugh caugh NodeLink caugh caugh*)

Connection.ts:

- Fix type `IVoiceServer` to make endpoint optional (refer to discord docs)

Poru.ts:

- Added 3 new interfaces to represent the Packets from discord (refer to discord docs)
- Update types from `<Poru>.send` function
- Update types from `<Poru>.packetUpdate` to use the new types

Have a nice day o/

I don't think custom client Name should be allowed, as the Client Name header should contain the client's(library name) name atleast

@Joniii11
Copy link
Contributor Author

then it should be formatted correctly like Poru/5.0.0 or sum. But some libraries allow it tho to set like the name of the discordbot

@UnschooledGamer
Copy link
Collaborator

then it should be formatted correctly like Poru/5.0.0 or sum. But some libraries allow it tho to set like the name of the discordbot

Yeah, some libraries do but it's not an good practice although Lavalink hasn't enforced it. And probably NodeLink enforces it I don't remember, have to ask Pedroo.

@Joniii11
Copy link
Contributor Author

NodeLink enforces it. I was in contact with Pedroo. That's why i added it to test if poru also works with NodeLink

@UnschooledGamer
Copy link
Collaborator

UnschooledGamer commented Mar 28, 2024

NodeLink enforces it. I was in contact with Pedroo. That's why i added it to test if poru also works with NodeLink

Poru will work nicely with it as it's based on the Lavalink API and will support most features of it. For others Poru will require some custom implementations.

@Joniii11
Copy link
Contributor Author

Currently it does not work cause of the enforcement of the name/version

@Joniii11
Copy link
Contributor Author

I pushed some small fixes also. It's more efficient and takes less space up :)

@parasop parasop merged commit a1586ea into parasop:v5 Mar 29, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants