-
Notifications
You must be signed in to change notification settings - Fork 34
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
Bloxy Version 6 #287
base: v5
Are you sure you want to change the base?
Bloxy Version 6 #287
Conversation
…`convertObjectToValue`
Is there a reason why you didn't choose to use |
connect (): Promise<void> { | ||
return new Promise((resolve, reject) => { | ||
const connectSocket = (retries = 0): void => { | ||
this.socket = new SignalR.client("wss://realtime.roblox.com/notifications", ["usernotificationhub"], 3, true); |
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.
Consider replacing usage of signalr-client
(deprecated) with https://www.npmjs.com/package/signalr
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.
Do you perhaps know if Roblox uses ASP.NET or ASP.NET Core? Since Microsoft offers a signalr client as well (https://www.npmjs.com/package/@microsoft/signalr) which targets ASP.NET Core (and I'm not entirely sure if it also works with ASP.NET SignalR servers)
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.
Do you perhaps know if Roblox uses ASP.NET or ASP.NET Core? Since Microsoft offers a signalr client as well (npmjs.com/package/@microsoft/signalr) which targets ASP.NET Core (and I'm not entirely sure if it also works with ASP.NET SignalR servers)
Fairly certain @microsoft/signalr
only works with ASP.NET Core SignalR servers, and signalr
only works with Normal ASP.NET SignalR servers. Both signalr
and @microsoft/signalr
are published by the same person on NPM -- wouldn't make much sense to publish a redundant package.
Roblox uses ASP.NET
on realtime.roblox.com
, judging by the headers. So signalr
is the proper package to use.
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.
Roblox uses both SignalR for .NET Core and .NET Framework. But for the likes of Messaging Service (Message Router Service) and Real-Time notifications, which are both part of the BEDEV1 Framework (Primarily .NET Framework, becoming obsolete), they use Microsoft’s Owin integration of SignalR
Co-authored-by: Guido de Jong <[email protected]>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Guido de Jong <[email protected]>
compiled typescript output should stay in gitignore since it's not source files and can just be recompiled |
This allows to install the branch with npm and will only stay for a bit |
After some digging, I finally found the cause of the error where X-CSRF token refreshing doesn't always correctly work: The status message handler lowercases the received status message before comparing it to the set allowed/disallowed status messages. The set token validation error status message includes uppercase letters so this fails: https://github.com/Visualizememe/bloxy/blob/ea5281d6e99013bec5a57cc886bb85fa5ea861c4/src/controllers/rest/request/prepare.ts#L50 A good fix would be lowercasing Although not really relevant for this PR, it seems like a good fix to include here. |
Also what's the timeline on this? |
Yeah, bloxyjs/core is the new repository, since Martini unfortunately still did not manage to transfer the repository |
This PR (and branch) updates bloxy to version 6.
No information yet to this other than it will fix a lot of problems.
To-Do:
APIs:
Critical Changes
CaptchaAPI
was removedAdsAPI
was removedTradesAPI
was removed