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

Removed ESIK_Result parameter from the delegate on SIK_JoinLobby_AsyncFunction #6

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

KryptoniteKS
Copy link
Contributor

Removed casting from an EChatRoomEnterResponse to an ESIK_Result and broadcasting that value, which would have unintended consequences if someone tried to use that value. In fact, the LobbyEnter_t callback doesn't even have an EResult property, so I removed it from the delegate altogether.

A good replacement would be to broadcast the Param.m_ulSteamIDLobby of the lobby, which would provide an easy way to see if the callback had a bad response (SteamId would be 0 in this case).

Remove the Steam Result enum from the delegate, as the callback does not actually have a Steam Result enum
Removed the Steam Result enum from the broadcasting of the delegate; we were casting from EChatRoomEnterResponse to EResult, which would give unintended consequences.
Also removed the use of ESIK_ChatRoomEnterResponse::None, as the Steam API would return ChatRoomEnterResponseError in the event an error occurred. This "None" could cause issues if a user tried to cast back to an EChatRoomEnterResponse.
@KryptoniteKS
Copy link
Contributor Author

KryptoniteKS commented Oct 30, 2024

Side note: I also removed the broadcasting of the ESIK_ChatRoomEnterResponse::None value and replaced it with ESIK_ChatRoomEnterResponse::ChatRoomEnterResponseError as this is the proper response the Steam API would return.

I understand if these changes aren't merged, as you may want to go a different direction with it. This was just something I caught while trying to utilize these functions and changed on my personal project.

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.

1 participant