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

feat: add sdk_version to HANDSHAKE message #262

Merged
merged 7 commits into from
Oct 1, 2024
Merged

Conversation

afgiel
Copy link
Collaborator

@afgiel afgiel commented Sep 25, 2024

No description provided.

src/Constants.ts Outdated
@@ -36,3 +36,5 @@ export const Permissions = Object.freeze({
CREATE_INSTANT_INVITE: BigFlagUtils.getFlag(0),
ADMINISTRATOR: BigFlagUtils.getFlag(3),
});

export const HANDSHAKE_SDK_VERSION_MINIUM_MOBILE_VERSION = '250.5';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dumb question, is it okay to use this string for both iOS and Android? I remember sometimes one had a <version>.0 and the other is just <version>

@@ -4,6 +4,7 @@
import {Opcodes} from '../Discord';
import {DiscordSDK, Events, Platform} from '../index';
import {DISPATCH} from '../schema/common';
import {version as sdkVersion} from '../../package.json';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

@@ -53,8 +54,9 @@ function buildConfig(format) {
declaration: true,
outDir: outDir,
}),
json(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this required for retrieving the sdk's version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

src/Discord.ts Outdated
Comment on lines 215 to 219
} catch {
return null;
}
}
return null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Might be cool if we could return UNKNOWN or some other constant, so we could determine if the version is absent because of an error or because the version is too old to attempt to provide it

Copy link
Contributor

@matthova matthova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, here we go sdk version

@afgiel afgiel merged commit 86a3a17 into main Oct 1, 2024
4 checks passed
@afgiel afgiel deleted the afgiel/sdk-version branch October 1, 2024 19:25
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.

4 participants