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

Add update_item_with_callback and get_auth_ticket_for_web_api method. Change get_session_ticket to get_session_ticket_with_steam_id. #125

Merged
merged 10 commits into from
Dec 30, 2023

Conversation

LZQCN
Copy link
Contributor

@LZQCN LZQCN commented Dec 19, 2023

I believe that although these modifications are functional, they may not necessarily be correct and require further inspection. In particular, I believe that the change from 'get_session_ticket' to 'get_session_ticket_with_steam_id' could potentially cause compatibility issues.

src/api/auth.rs Outdated Show resolved Hide resolved
@ceifa
Copy link
Owner

ceifa commented Dec 21, 2023

What do you think about the progress callback being an optional parameter of the original updateItem?

Added optional `progress_callback` and `progress_callback_interval_ms` parameter.
@LZQCN
Copy link
Contributor Author

LZQCN commented Dec 23, 2023

What do you think about the progress callback being an optional parameter of the original updateItem?

New commit changed the progress callback to optional and added a new optional parameter to control the interval of update callback

@ceifa
Copy link
Owner

ceifa commented Dec 24, 2023

Can we merge the two logics of UGC submit? Maybe something like:

impl UgcUpdate {
    pub fn submit(
        self,
        mut update: UpdateHandle<ClientManager>,
        callback: impl FnOnce(Result<(PublishedFileId, bool), steamworks::SteamError>)
            + Send
            + 'static,
    ) -> steamworks::UpdateWatchHandle<ClientManager> {
        if let Some(title) = self.title {
            update = update.title(title.as_str());
        }

        if let Some(description) = self.description {
            update = update.description(description.as_str());
        }

        if let Some(preview_path) = self.preview_path {
            update = update.preview_path(Path::new(&preview_path));
        }

        if let Some(tags) = self.tags {
            update = update.tags(tags);
        }

        if let Some(content_path) = self.content_path {
            update = update.content_path(Path::new(&content_path));
        }

        if let Some(visibility) = self.visibility {
            update = update.visibility(visibility.into());
        }

        let change_note = self.change_note.as_deref();
        update.submit(change_note, callback)
    }
}

#[napi(
ts_arg_type = "(data: { itemId: bigint; needsToAcceptAgreement: boolean }) => void"
)]
success_callback: napi::JsFunction,
Copy link
Owner

@ceifa ceifa Dec 24, 2023

Choose a reason for hiding this comment

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

Do we really need to receive a JsFunction and transform it to threadsafe on these callbacks? Maybe it can be handled automatically by napi when receiving a FnOnce? https://napi.rs/docs/concepts/function

Copy link
Contributor Author

@LZQCN LZQCN Dec 26, 2023

Choose a reason for hiding this comment

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

Yes, Using a JsFunction and converting it to thread-safe is necessary, and this is related to the following code:
https://github.com/Noxime/steamworks-rs/blob/master/src/ugc.rs

    pub fn submit<F>(self, change_note: Option<&str>, cb: F) -> UpdateWatchHandle<Manager>
    where
        F: FnOnce(Result<(PublishedFileId, bool), SteamError>) + 'static + Send,
    {
        use std::ptr;
        unsafe {
            let change_note = change_note.and_then(|v| CString::new(v).ok());
            let note = change_note.as_ref().map_or(ptr::null(), |v| v.as_ptr());
            let api_call = sys::SteamAPI_ISteamUGC_SubmitItemUpdate(self.ugc, self.handle, note);
            register_call_result::<sys::SubmitItemUpdateResult_t, _, _>(
                &self.inner,
                api_call,
                CALLBACK_BASE_ID + 4,
                move |v, io_error| {
                    cb(if io_error {
                        Err(SteamError::IOFailure)
                    } else if v.m_eResult != sys::EResult::k_EResultOK {
                        Err(v.m_eResult.into())
                    } else {
                        Ok((
                            PublishedFileId(v.m_nPublishedFileId),
                            v.m_bUserNeedsToAcceptWorkshopLegalAgreement,
                        ))
                    })
                },
            );
        }
        UpdateWatchHandle {
            ugc: self.ugc,
            _inner: self.inner,
            handle: self.handle,
        }
    }

@LZQCN
Copy link
Contributor Author

LZQCN commented Dec 26, 2023

Can we merge the two logics of UGC submit? Maybe something like:

impl UgcUpdate {
    pub fn submit(
        self,
        mut update: UpdateHandle<ClientManager>,
        callback: impl FnOnce(Result<(PublishedFileId, bool), steamworks::SteamError>)
            + Send
            + 'static,
    ) -> steamworks::UpdateWatchHandle<ClientManager> {
        if let Some(title) = self.title {
            update = update.title(title.as_str());
        }

        if let Some(description) = self.description {
            update = update.description(description.as_str());
        }

        if let Some(preview_path) = self.preview_path {
            update = update.preview_path(Path::new(&preview_path));
        }

        if let Some(tags) = self.tags {
            update = update.tags(tags);
        }

        if let Some(content_path) = self.content_path {
            update = update.content_path(Path::new(&content_path));
        }

        if let Some(visibility) = self.visibility {
            update = update.visibility(visibility.into());
        }

        let change_note = self.change_note.as_deref();
        update.submit(change_note, callback)
    }
}

OK, in the new commit, I've completed the merge of the two UGC submit logics into a single method as suggested.
7ae76ca

@ceifa ceifa merged commit 5635d3f into ceifa:main Dec 30, 2023
3 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.

2 participants