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

Switch to serde-wasm-bindgen over serde-serialize feature #8080

Merged
merged 1 commit into from
Sep 24, 2022

Conversation

Aaron1011
Copy link
Member

The serde-serialize feature is deprecated
(rustwasm/wasm-bindgen#3031).

This solves a cyclic dependency error with aHash

Copy link
Contributor

@relrelb relrelb left a comment

Choose a reason for hiding this comment

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

Can you please prefix the commit message with web:?

The `serde-serialize` feature is deprecated
(rustwasm/wasm-bindgen#3031).

This solves a cyclic dependency error with aHash
@relrelb relrelb merged commit 1922156 into ruffle-rs:master Sep 24, 2022
@Toad06
Copy link
Member

Toad06 commented Sep 25, 2022

Not sure if this is related to this PR but the user's configuration is completely ignored in the current nightly build (2022-09-25), both self-hosted and extension versions are affected.

For instance, if you set the log level to "Info" using the extension, this won't trace anything but the errors which is the default (note: there are some traces but they come from the JS file which is not affected by the config). wmode="transparent" also won't work, etc.

The issue only seems to happen when at least one of the option is not specified on the Javascript's object that holds the config.

@relrelb
Copy link
Contributor

relrelb commented Sep 25, 2022

Not sure if this is related to this PR but the user's configuration is completely ignored in the current nightly build (2022-09-25), both self-hosted and extension versions are affected.

For instance, if you set the log level to "Info" using the extension, this won't trace anything but the errors which is the default (note: there are some traces but they come from the JS file which is not affected by the config). wmode="transparent" also won't work, etc.

The issue only seems to happen when at least one of the option is not specified on the Javascript's object that holds the config.

Wow, thank you! That's quite an embarrassing regression... Anyway I tossed out a fix in #8093. I confirmed it to work on my end using the demo page, but you're more than welcome to double-check the other components (JS API, extension etc.).
I think we definitelty need to cover this with tests, to avoid future regressions.

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